Skip to content
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

[TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse #5917

Merged
merged 5 commits into from
Jun 26, 2020

Conversation

merrymercy
Copy link
Member

@merrymercy merrymercy commented Jun 24, 2020

Add LegalizeInvalidAttach in Schedule::normalize to legalize the compute_at location if the target iterator of compute_at is split or fused.

  • Case 1: If the target of compute_at is split, we will move the compute_at location to the inner iterator.
  • Case 2: If the target of compute_at is fused, we will move the compute_at location to the newly fused iterator.
    Case 2 can only happen if the target of compute_at is the innermost operand of fuse operation.

Examples: the following two cases will crash the compiler before this fix, but they are legal after this fix.

    A = te.compute((10, 10), lambda i, j: 1.0, name='A')
    B = te.compute((10, 10), lambda i, j: A[i][j], name='B')

    # Case 1: Split an axis which is the target of a compute_at
    s = te.create_schedule([B.op])
    s[A].compute_at(s[B], B.op.axis[1])
    s[B].split(B.op.axis[1], 2)
    print(tvm.lower(s, [A, B], simple_mode=True))

    # Case 2: Fuse an axis which is the target of a compute_at
    s = te.create_schedule([B.op])
    s[A].compute_at(s[B], B.op.axis[1])
    s[B].fuse(B.op.axis[0], B.op.axis[1])
    print(tvm.lower(s, [A, B], simple_mode=True))

@merrymercy merrymercy changed the title Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse [TIR] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse Jun 24, 2020
@merrymercy merrymercy changed the title [TIR] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse Jun 24, 2020
@merrymercy
Copy link
Member Author

This is a support PR for #5883. cc @tqchen @comaniac @jcf94

@tqchen tqchen self-assigned this Jun 24, 2020
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Overall LGTM in terms of the functionality.

src/te/schedule/schedule_dataflow_rewrite.cc Show resolved Hide resolved
src/te/schedule/schedule_dataflow_rewrite.cc Show resolved Hide resolved
@merrymercy
Copy link
Member Author

@tqchen @comaniac comments are addressed

@tqchen
Copy link
Member

tqchen commented Jun 25, 2020

cc @vinx13 @Hzfengsy @spectrometerHBH please also help to take a look, we can proceed to merge after reviews by a few more eyes

Copy link
Member

@vinx13 vinx13 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@@ -196,6 +196,7 @@ tvm_t.*
.python_history
.pytest_cache
.local
cmake-build-debug
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we don’t need this line

Copy link
Member Author

@merrymercy merrymercy Jun 26, 2020

Choose a reason for hiding this comment

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

It is produced by my Jetbrain Clion

@merrymercy merrymercy merged commit 96bf271 into apache:master Jun 26, 2020
@merrymercy merrymercy deleted the pr-legalize-invalid-attach branch June 26, 2020 05:52
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 30, 2020
…ter split or fuse (apache#5917)

* Add LegalizeInvalidAttach

* lint & typo

* lint & typo

* address comment

* fix lint
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Jul 2, 2020
…ter split or fuse (apache#5917)

* Add LegalizeInvalidAttach

* lint & typo

* lint & typo

* address comment

* fix lint
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.

6 participants