Skip to content

Conversation

@lhutton1
Copy link
Contributor

@lhutton1 lhutton1 commented Feb 2, 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.

cc @mbaret @manupa-arm @ekalda @jacobbohlin @dchauhan-arm @NicolaLancellotti

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
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.

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.

@manupak
Copy link
Contributor

manupak commented Feb 3, 2022

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.

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

@masahi masahi merged commit a8741e2 into apache:main Feb 3, 2022
@lhutton1 lhutton1 deleted the fixing-layout-optimizer branch February 3, 2022 23:49
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
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