[microNPU] Fix layout assignment in layout optimizer pass#10143
Merged
Conversation
Fixes the layout optimizer incorrectly assigning layouts for graphs with more complex topologies than previously considered. Specifically, this commit now ensures that intermediate layouts match (e.g. parent output = child input) and that all consumers are taken into account when altering the output layout - something not done previously due to an incorrect traversal order. Previously, the input layout was always altered if the producer was an NPU operation without regard to the output layout of that operation. Additionally, is was possible for the output layout to be incorrectly set due to a depth-first post-order of traversal of the graph, meaning it was possible for not all consumers to be taken into account when altering the layout. Now the `AnalyzeConsumers` pass is run before `LayoutOptimization` which determines a mapping from NPU operation to list of boolean values that represent whether or not each consumer is an NPU operation. Since this is completed before `LayoutOptimization`, all consumers are guaranteed to be taken into account when altering the output layout. In turn, the input layouts can correctly be determined by checking whether the output of the producer will be altered. Change-Id: I04e9605da65fa9f12801109dd50c5e3f08cbc73c
manupak
reviewed
Feb 3, 2022
Contributor
There was a problem hiding this comment.
Thanks @lhutton1 for the quick fix!
There is an another alternative which is to add an identity operator to bypassed branch which can do the layout transform. This will allow the usage of rolling buffer enabled cascading to the other full branch.
What Im wondering is whether that should be decided by the cascader
I would like to hear @mbaret s thoughts here.
Contributor
|
The above comment is for a future reference. This fix is important as currently it is fails the execution. Just keeping the thread for a follow up, if needed. |
mbs-octoml
pushed a commit
to mbs-octoml/mbs-tvm
that referenced
this pull request
Feb 5, 2022
Fixes the layout optimizer incorrectly assigning layouts for graphs with more complex topologies than previously considered. Specifically, this commit now ensures that intermediate layouts match (e.g. parent output = child input) and that all consumers are taken into account when altering the output layout - something not done previously due to an incorrect traversal order. Previously, the input layout was always altered if the producer was an NPU operation without regard to the output layout of that operation. Additionally, is was possible for the output layout to be incorrectly set due to a depth-first post-order of traversal of the graph, meaning it was possible for not all consumers to be taken into account when altering the layout. Now the `AnalyzeConsumers` pass is run before `LayoutOptimization` which determines a mapping from NPU operation to list of boolean values that represent whether or not each consumer is an NPU operation. Since this is completed before `LayoutOptimization`, all consumers are guaranteed to be taken into account when altering the output layout. In turn, the input layouts can correctly be determined by checking whether the output of the producer will be altered. Change-Id: I04e9605da65fa9f12801109dd50c5e3f08cbc73c
ylc
pushed a commit
to ylc/tvm
that referenced
this pull request
Feb 16, 2022
Fixes the layout optimizer incorrectly assigning layouts for graphs with more complex topologies than previously considered. Specifically, this commit now ensures that intermediate layouts match (e.g. parent output = child input) and that all consumers are taken into account when altering the output layout - something not done previously due to an incorrect traversal order. Previously, the input layout was always altered if the producer was an NPU operation without regard to the output layout of that operation. Additionally, is was possible for the output layout to be incorrectly set due to a depth-first post-order of traversal of the graph, meaning it was possible for not all consumers to be taken into account when altering the layout. Now the `AnalyzeConsumers` pass is run before `LayoutOptimization` which determines a mapping from NPU operation to list of boolean values that represent whether or not each consumer is an NPU operation. Since this is completed before `LayoutOptimization`, all consumers are guaranteed to be taken into account when altering the output layout. In turn, the input layouts can correctly be determined by checking whether the output of the producer will be altered. Change-Id: I04e9605da65fa9f12801109dd50c5e3f08cbc73c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the layout optimizer incorrectly assigning layouts for graphs with more complex topologies than previously considered. Specifically, this commit now ensures that intermediate layouts match (e.g. parent output = child input) and that all consumers are taken into account when altering the output layout - something not done previously due to an incorrect traversal order.
Previously, the input layout was always altered if the producer was an NPU operation without regard to the output layout of that operation. Additionally, is was possible for the output layout to be incorrectly set due to a depth-first post-order of traversal of the graph, meaning it was possible for not all consumers to be taken into account when altering the layout.
Now the
AnalyzeConsumerspass is run beforeLayoutOptimizationwhich determines a mapping from NPU operation to list of boolean values that represent whether or not each consumer is an NPU operation. Since this is completed beforeLayoutOptimization, all consumers are guaranteed to be taken into account when altering the output layout. In turn, the input layouts can correctly be determined by checking whether the output of the producer will be altered.cc @mbaret @manupa-arm @ekalda @jacobbohlin @dchauhan-arm @NicolaLancellotti