-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[microNPU] Fix layout assignment in layout optimizer pass #10143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
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. |
manupak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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
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
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