Skip to content

Conversation

@ThomasRaoux
Copy link
Collaborator

Reverts #8292 as it causes functional regressions

@ThomasRaoux ThomasRaoux requested a review from ptillet as a code owner October 7, 2025 21:49
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines 805 to 812
for (Value v : slice) {
auto layoutIt = layout.find(v);
assert(layoutIt != layout.end());
// If we found a valid rematerialization for this value while constructing
// the slice, use that.
if (Value remat = existingRemats.lookup({v, layoutIt->second})) {
assert(getRematValue(v, layoutIt->second) == remat && "remat mismatch");
// If we already have a remat value for this value, use it.
if (Value remat = getRematValue(v, layoutIt->second)) {
mapping.map(v, remat);
valuesWithExistingRemat.insert(v);
continue;

Choose a reason for hiding this comment

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

P1 Badge Reuse remats without dominance validation

While rewriting a slice, the code now calls getRematValue directly to skip rematerializing a value whenever any remat is registered. The callback in getConvertBackwardSlice only filters remats that dominate the current use, but that information is no longer carried forward to the rewrite step after the existingRemats map was removed. As a result, rewriteSlice can map a value to a remat that lives in a different control path and does not dominate the rewritten convert, yielding invalid IR (uses before defs) or incorrect rematerialisation. The earlier implementation only reused remats recorded during slice construction, so the dominance constraint was preserved.

Useful? React with 👍 / 👎.

@ThomasRaoux ThomasRaoux merged commit 3a3b644 into main Oct 7, 2025
6 of 9 checks passed
@ThomasRaoux ThomasRaoux deleted the revert-8292-fix-layout-dominance branch October 7, 2025 22:17
yangshuxin pushed a commit to yangshuxin/triton that referenced this pull request Oct 9, 2025
Jokeren pushed a commit that referenced this pull request Oct 10, 2025
ita9naiwa pushed a commit to ita9naiwa/triton that referenced this pull request Oct 12, 2025
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