[Arith][GPU]Rewrite simplify fix for Vectorized Cooperative Fetching#5924
Merged
tqchen merged 7 commits intoapache:masterfrom Jun 26, 2020
Merged
[Arith][GPU]Rewrite simplify fix for Vectorized Cooperative Fetching#5924tqchen merged 7 commits intoapache:masterfrom
tqchen merged 7 commits intoapache:masterfrom
Conversation
7 tasks
Member
|
Thanks @jcf94 we should add a testcase to test_arith_rewrite_simplify, by constructing the case and
|
tqchen
requested changes
Jun 25, 2020
| return broadcast(floordiv(b1, c2), lanes).Eval(); | ||
| } | ||
| // If all indices can be guaranteed to settle inside a coeff range | ||
| if (c2val % bmod->coeff == 0 && bmod->base + (lanes.Eval() - 1) * c1val < bmod->coeff) { |
Member
There was a problem hiding this comment.
Please add a unit test in tests_arith_rewrite_simplify to cover this rule.
Contributor
Author
Commemts are all addressed.
|
Member
|
@jcf94 Did our old rule affect the correctness of common operators? |
Contributor
Author
Yes, with those rules several other UTs will fail. Our rules make it to be |
tqchen
approved these changes
Jun 26, 2020
Member
|
Thanks @jcf94 @merrymercy . this PR is now merged |
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
Jun 30, 2020
zhiics
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Jul 2, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pr is part of #5883 , fix for the rewrite_simplify error when doing vectorized cooperative fetching in some cases.
Code generated with bug is shown like this:
Which will finally lower to wrong CUDA C instructions.
This should be simplified to generate the correct RampNode:
Then main problems inside this expression are:
should be simplified to:
@merrymercy Our former simplify rules will cause some extra bug, I find a better way to fix this.
@tqchen For the UTs, I'm not sure if there's any better way to check if all of the inner AST blocks are RampNode, the current UTs I added can still pass even if the vectorize failed.
cc @minminsun @FrozenGene @yangjunpro