[AMD] re-enable canonicalize pointers pass #5329
Draft
+1,113
−742
Conversation
This file contains 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
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
from
9390a46 to
bc6ba64
Compare
giuseros
reviewed
Dec 4, 2024
| Value tensorPtr = createTensorPointer(fatPtr, curLoc); | ||
| auto newForOp = | ||
| replaceForOpWithNewSignature(rewriter, forOp, {basePtr, offset}); | ||
| size_t operandNum = curOperand->getOperandNumber(); |
There was a problem hiding this comment.
Could we have this into a function? Like rewriteForOpInPlace?
| operandNum + 1 - forOp.getNumControlOperands(); | ||
|
|
||
| SmallVector<Value> newInitArgs( | ||
| forOp.getInitArgs().take_front(numLeadingInitsIncludingCurOp)); |
There was a problem hiding this comment.
I would remove the currentOp. It was there before exactly because we were pushing back the new operands. We can now get rid of it (and make the IR way simpler).
|
|
||
| newForOp.getBody()->insertArgument(numLeadingInitsIncludingCurOp + 1, | ||
| offset.getType(), offset.getLoc()); | ||
| newForOp.getBody()->insertArgument(numLeadingInitsIncludingCurOp + 1, |
There was a problem hiding this comment.
Shouldn't be numLeadingInitsIncludingCurOp + 2
There was a problem hiding this comment.
No because you insert offset and then base behind it
| rewriteOpMap[forOp] = newForOp; | ||
|
|
||
| newForOp->setOperand(operandNum, tensorPtr); | ||
| { |
There was a problem hiding this comment.
Basically, if you remove the current operand from the the loop arguments, you won't need this anymore. This will be removed by canonicalization anyway, but it's here only because we have to pass "something" as the current operand.
| Value tensorPtr = createTensorPointer(fatPtr, curLoc); | ||
| newForOp->setOperand(operandNum, tensorPtr); | ||
| } | ||
|
|
||
| OpOperand *forOperand = &newForOp->getOpOperand(operandNum); | ||
| // This is making sure we propagate the visit from the forOp result |
7 tasks
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
2 times, most recently
from
cca951b to
2ca0a71
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
from
2ca0a71 to
ce211fd
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
from
d1f4d6a to
e4b27bd
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
from
1773949 to
5091eef
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
2 times, most recently
from
32d4271 to
9ed96db
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
6 times, most recently
from
f308df9 to
1b44114
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
3 times, most recently
from
1b6bd72 to
05c6fc1
Compare
makslevental
force-pushed
the
makslevental/re-enable-canonicalize-pointers
branch
from
05c6fc1 to
55c3dd2
Compare
giuseros
reviewed
Dec 18, 2024
There was a problem hiding this comment.
As a warmup comment, could you add a test to check the issue we were having? Something on the line of:
for (%ptrA,%ptrB){
yield %ptrB, %ptrA
}
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.
Disabled here #4966 (comment). Will explain soon.