Skip to content

Conversation

@neildhar
Copy link
Contributor

@neildhar neildhar commented Oct 27, 2025

We currently always look at index 1 in the shape to determine if it is
an outer product. However, for 3D operands, we actually have to look at
index 2. This causes us to incorrectly crash when supplied a 3D operand
where the first dimension is 1.

Eliminate the isOuter check entirely, since my understanding is that:

  1. it is purely defensive, since we will just crash when isOuter is
    true and mmaLayout is non-null.
  2. it does not disqualify all the invalid K values, so it might be
    confusing.

@neildhar neildhar force-pushed the fix-dotop-lowering branch 3 times, most recently from c0d7dd3 to ecb7e9b Compare October 27, 2025 22:24
@neildhar neildhar marked this pull request as ready for review October 27, 2025 22:26
@neildhar neildhar requested a review from ptillet as a code owner October 27, 2025 22:26
@neildhar neildhar changed the title Fix and simplify DotOp lowering Fix handling of 3D DotOp with M=1 Oct 27, 2025
Copy link
Collaborator

@ThomasRaoux ThomasRaoux left a comment

Choose a reason for hiding this comment

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

LGTM

We currently always look at index 1 in the shape to determine if it is
an outer product. However, for 3D operands, we actually have to look at
index 2. This causes us to incorrectly crash when supplied a 3D operand
where the first dimension is 1.

Eliminate the isOuter check entirely, since my understanding is that:
1. it is purely defensive, since we will just crash when isOuter is
   true and mmaLayout is non-null.
2. it does not disqualify all the invalid K values, so it might be
   confusing.
@neildhar
Copy link
Contributor Author

neildhar commented Nov 4, 2025

@ThomasRaoux is this good to merge?

@ThomasRaoux ThomasRaoux merged commit d0ef9d3 into triton-lang:main Nov 4, 2025
9 checks passed
@neildhar neildhar deleted the fix-dotop-lowering branch November 4, 2025 18:11
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.

2 participants