Skip to content

Conversation

jinhongyii
Copy link
Contributor

This PR is part of the TensorIR upstreaming effort (#7527) , which adds 2 schedule primitives:

  • fuse
  • split

@jinhongyii
Copy link
Contributor Author

@tqchen @jcf94 @comaniac Welcome to review

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. Just some style comments.

Copy link
Contributor

@jcf94 jcf94 left a comment

Choose a reason for hiding this comment

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

Thanks! @jinhongyii

TVM_TRY_REWRITE_IF(floormod(x + y * c1, c2), floormod(x, c2),
c2.Eval()->value > 0 && c1.Eval()->value % c2.Eval()->value == 0);

TVM_TRY_REWRITE_IF(floormod(x * c1, x * c2), x * floormod(c1, c2), c2.Eval()->value != 0);
Copy link
Member

Choose a reason for hiding this comment

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

Please send this as separate PR with regression testcases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These lines are added to support symbolic split/fuse in some simple cases, but it still can't handle situations where there are 2 or more symbolic vars. Do you think we should support this simple situation for now by merging it in a new PR or we put it aside and support the complete symbolic usage after we improve symbolic divisible check in iter_affine_map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tqchen @junrushao1994 what's your opinion on this?

@junrushao
Copy link
Member

CC @spectrometerHBH please take a look at the iter-affine-map part

@jinhongyii
Copy link
Contributor Author

I have addressed all the comments. Please have another look.

@jinhongyii
Copy link
Contributor Author

Comments are addressed. Please have another round of review!

@jinhongyii jinhongyii requested a review from junrushao July 19, 2021 16:43
@junrushao
Copy link
Member

Thanks for your patience! I don't have other comments

@jinhongyii
Copy link
Contributor Author

jinhongyii commented Jul 20, 2021

Sorry for bothering you. I don't know what happened and I don't know how to cancel it.

@jinhongyii
Copy link
Contributor Author

@junrushao1994 I've addressed your new comments

@jinhongyii jinhongyii requested a review from junrushao July 20, 2021 14:20
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.

Thanks for the tireless effort! LGTM

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.

LGTM

@tqchen tqchen merged commit 2df0854 into apache:main Jul 21, 2021
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
* Fuse&split (apache#408)



Co-authored-by: jinhongyi <323195289@qq.com>
Co-authored-by: Junru Shao <junrushao1994@gmail.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* Fuse&split (apache#408)



Co-authored-by: jinhongyi <323195289@qq.com>
Co-authored-by: Junru Shao <junrushao1994@gmail.com>
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.

7 participants