-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix TensorMemoryAllocation to correctly implement getAlloc #6614
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
Conversation
auto arg = cast<BlockArgument>(subOp.getSrc()); | ||
auto wsOp = op->getParentOfType<triton::gpu::WarpSpecializeOp>(); | ||
auto capture = wsOp.getExplicitCaptures()[arg.getArgNumber()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to check or assert that the arg belongs to wsOp. Best would be to get wsOp from arg.getOwner()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
while (isa<triton::gpu::MemDescSubviewOp>(op)) { | ||
op = op->getResult(0).getDefiningOp(); | ||
while (auto subOp = dyn_cast<triton::gpu::MemDescSubviewOp>(op)) { | ||
if (subOp.getSrc().getDefiningOp()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (subOp.getSrc().getDefiningOp()) { |
I don't think this is ever null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is null for the lit test case where it comes from arguments of warp_specialize. Is the lit test case valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry you're right. I think I had a brain fart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
…ng#6614) Prior to the fix, getAlloc will hit an infinite loop. This patch also handles the case where tmemDesc comes from argument of warp_specialize.
Prior to the fix, getAlloc will hit an infinite loop. This patch also handles the case where tmemDesc comes from argument of warp_specialize.