Skip to content

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

Merged
merged 1 commit into from
Apr 28, 2025

Conversation

manman-ren
Copy link
Collaborator

Prior to the fix, getAlloc will hit an infinite loop. This patch also handles the case where tmemDesc comes from argument of warp_specialize.

@manman-ren manman-ren requested a review from ptillet as a code owner April 27, 2025 04:17
@manman-ren manman-ren requested review from Mogball and removed request for ptillet April 27, 2025 04:17
Comment on lines 183 to 187
auto arg = cast<BlockArgument>(subOp.getSrc());
auto wsOp = op->getParentOfType<triton::gpu::WarpSpecializeOp>();
auto capture = wsOp.getExplicitCaptures()[arg.getArgNumber()];
Copy link
Collaborator

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()

Copy link
Collaborator

@Mogball Mogball left a 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()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (subOp.getSrc().getDefiningOp()) {

I don't think this is ever null.

Copy link
Collaborator Author

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?

Copy link
Collaborator

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

Copy link
Collaborator

@Mogball Mogball left a 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:
@manman-ren manman-ren merged commit 248fa4c into triton-lang:main Apr 28, 2025
8 checks passed
FindHao pushed a commit to FindHao/triton that referenced this pull request Apr 30, 2025
…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.
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.

3 participants