Skip to content

Fix infercorrect layout in Layoutrewrite and improve naming. - #12007

Merged
masahi merged 5 commits into
apache:mainfrom
lazycal:Fix-layoutrewriter
Jul 6, 2022
Merged

Fix infercorrect layout in Layoutrewrite and improve naming.#12007
masahi merged 5 commits into
apache:mainfrom
lazycal:Fix-layoutrewriter

Conversation

@lazycal

@lazycal lazycal commented Jul 4, 2022

Copy link
Copy Markdown
Contributor

This model

def @main(%x: Tensor[(1, 1, 1, 1), float32] /* ty=Tensor[(1, 1, 1, 1), float32] */) -> Tensor[(1, 1, 1), float32] {
  %0 = image.resize2d(%x, size=[2, 4], roi=[0f, 0f, 0f, 0f], rounding_method="") /* ty=Tensor[(1, 1, 2, 4), float32] */;
  %1 = mean(%0, axis=[3]) /* ty=Tensor[(1, 1, 2), float32] */;
  image.resize1d(%1, size=[1], roi=[0f, 0f], rounding_method="") /* ty=Tensor[(1, 1, 1), float32] */
}

fails with error Check failed: (tir::BijectiveLayout(new_src_layout, dst_layout).defined()) is false: Cannot insert layout transform because there are inconvertible layouts: NCW v.s. NCH. This should be a bug introduced in #10118. This line 7805d6f#diff-c3bb97b7e813244d6dae39925cc10bb9d50e8b7a280556b423068b3705f9f114R363 fixes it.

In short, there should be no layout rewriting here since no conv etc. are used. But when the pass is checking resize1d, the previous code do not pass consistent layouts for the argument old_in_layouts for the two calls of ResizeInferCorrectLayout (the first time NCH from old_in

InferCorrectLayouts(ref_call, Array<Layout>(nullptr), old_in, types);
, second time NCW from old_in_2
std::tie(infer_out, success) = InferCorrectLayouts(new_call, new_in_tmp, old_in2, types);
), misleading the optimizer into thinking that the layout has changed and hence an ill-formed layout_transform is inserted.

This PR also improves the naming based on suggestions from @masahi in #10118, as well as adds several comments and logging statements to help people understand. (Hope this helps you understand that change better too @masahi :-)).

@lazycal
lazycal force-pushed the Fix-layoutrewriter branch from 8248426 to fa8f026 Compare July 4, 2022 19:26
@lazycal
lazycal marked this pull request as ready for review July 5, 2022 04:17
@lazycal lazycal changed the title [Draft not ready for review] Fix infercorrect layout in layoutrewrite. Fix infercorrect layout in Layoutrewrite and improve naming. Jul 5, 2022
@lazycal

lazycal commented Jul 5, 2022

Copy link
Copy Markdown
Contributor Author

To reproduce the problem, run the following code:

import tvm
from tvm import relay
from tvm.relay.transform import InferType


@tvm.instrument.pass_instrument
class PrintIR:
    """Print the name of the pass, the IR, only before passes execute."""

    def __init__(self, print_mod=True, show_meta_data=False) -> None:
        self.pass_cnt = 0
        self.print_mod = print_mod
        self.show_meta_data = show_meta_data

    def run_before_pass(self, mod, info):
        with tvm.transform.PassContext(instruments=[]):
            global prev_mod
            if self.print_mod:
                print(relay.transform.InferType()(mod))
            print('>' * 40, f'Running Pass#{self.pass_cnt}:', info)

        self.pass_cnt += 1


x = relay.var("x", shape=(1, 1, 1, 1))
y = relay.image.resize2d(x, (2, 4))
z = relay.mean(y, axis=3)
a = relay.image.resize1d(z, (1,))
func = relay.Function((x,), a)
mod = tvm.IRModule.from_expr(func)
print(InferType()(mod))

with tvm.transform.PassContext(opt_level=4, instruments=[PrintIR()]):
    relay.create_executor("graph", mod).evaluate()
print('pass')

@masahi
masahi merged commit 111169c into apache:main Jul 6, 2022
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
…12007)

* Fix infercorrect layout in layoutrewrite.

* Compatibility issue.

* Fix lint.

* Better naming and detailed comments.

* Add unittest.
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
…12007)

* Fix infercorrect layout in layoutrewrite.

* Compatibility issue.

* Fix lint.

* Better naming and detailed comments.

* Add unittest.
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
…12007)

* Fix infercorrect layout in layoutrewrite.

* Compatibility issue.

* Fix lint.

* Better naming and detailed comments.

* Add unittest.
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.

2 participants