Skip to content

Conversation

@lhutton1
Copy link
Contributor

@lhutton1 lhutton1 commented Dec 22, 2021

It looks like the LayoutOptimizer pass was accidentally removed. Probably due to a race condition when merging PR's. Re-enabling this feature.

cc @ekalda @manupa-arm @mbaret

It looks like in apache#9597 the LayoutOptimizer pass was accidentally removed.
Probably due to a race condition in PR's. Re-enabling this feature.

Change-Id: I4fc16a440f90277c5fcd887715166332af052c6b
Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to run the LUT pass before the layout optimization pass since the LUT pass removes some of the identity operators (which the layout pass currently skips IIRC), so in that case we could transform more layouts between transformation-eligible operators?

@lhutton1
Copy link
Contributor Author

Yes I think that would make more sense!

Change-Id: I6e7a22f46660029bbf4be3deb2be929cecf5d365
Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done for spotting that bug/git fail, LGTM! Fingers crossed all the tests pass when both of these passes are enabled 😅

@manupak
Copy link
Contributor

manupak commented Dec 23, 2021

Oops.... I also think we need to have at least a single integration test that requires the this pass being run -- maybe in a follow up?

Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@manupak manupak merged commit 85e27c1 into apache:main Dec 23, 2021
@manupak
Copy link
Contributor

manupak commented Dec 23, 2021

Thanks all! Lets add a test to make sure it doesn't get removed silently...

@lhutton1
Copy link
Contributor Author

Yes that's sensible, will do!

@lhutton1 lhutton1 deleted the enable-layout-optimizer branch December 23, 2021 10:43
lhutton1 added a commit to lhutton1/tvm that referenced this pull request Jan 4, 2022
…line

Follow up to apache#9793. In checking this it was found that both the layout
and LUT optimizer passes were silently not running. This is fixed by
converting each pass to a module pass, rather than a function pass. Tests
have been added to prevent this happening in the future.

Change-Id: I5145c6f02eeb0daea3cdba56198e0804ec32f351
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
* [microNPU] Re-enable LayoutOptimizer pass

It looks like in apache#9597 the LayoutOptimizer pass was accidentally removed.
Probably due to a race condition in PR's. Re-enabling this feature.

Change-Id: I4fc16a440f90277c5fcd887715166332af052c6b

* change pass ordering

Change-Id: I6e7a22f46660029bbf4be3deb2be929cecf5d365

Co-authored-by: lukhut01 (generated by with_the_same_user script) <lukhut01@e127400.cambridge.arm.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* [microNPU] Re-enable LayoutOptimizer pass

It looks like in apache#9597 the LayoutOptimizer pass was accidentally removed.
Probably due to a race condition in PR's. Re-enabling this feature.

Change-Id: I4fc16a440f90277c5fcd887715166332af052c6b

* change pass ordering

Change-Id: I6e7a22f46660029bbf4be3deb2be929cecf5d365

Co-authored-by: lukhut01 (generated by with_the_same_user script) <lukhut01@e127400.cambridge.arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants