Skip to content

Revert "[ControlFlowOpt](feat) decompose pointers across control flow (#1344)" - #1497

Open
kcanamgal wants to merge 1 commit into
triton-lang:mainfrom
kcanamgal:0811-revert-1344
Open

Revert "[ControlFlowOpt](feat) decompose pointers across control flow (#1344)"#1497
kcanamgal wants to merge 1 commit into
triton-lang:mainfrom
kcanamgal:0811-revert-1344

Conversation

@kcanamgal

@kcanamgal kcanamgal commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

This reverts commit 6dab8f4.

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because FILL THIS IN.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

@github-actions github-actions Bot added compiler Changes to C/C++ compiler backend (lib/, include/) python Changes to Python runtime or bindings ascend-backend Changes to the Ascend NPU backend labels Aug 11, 2026
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📝 In summary (no line info): 1 comment(s)

[bug · critical]

📄 third_party/ascend/include/Utils/Utils.h

⚠️ GitHub could not post this as an inline comment: No line information provided

Copy-paste bug: The RHS branch erroneously operates on lhsValue instead of rhsValue. When rhs is a non-constant Value (not an integer attribute), the code converts lhsValue again instead of converting rhsValue, and the assertion checks the wrong variable. This means rhsValue may remain a non-Index type (e.g., i32), causing potential type mismatch errors in the subsequent arith::AddIOp operation.

Affected functions: addOpFoldResult, subOpFoldResult, mulOpFoldResult, divOpFoldResult, remOpFoldResult, minOpFoldResult (all have the same bug, only maxOpFoldResult is correct).

💡 Suggested Change

Before:

  auto rhsValue = dyn_cast<Value>(rhs);
  if (rhsInt) {
    rhsValue = createConstIndexValueOp(loc, b, rhsInt.value());
  } else {
    lhsValue = convertToIndexIfNeeded(lhsValue, loc, b);
    assert(isa<IndexType>(lhsValue.getType()));
  }

After:

  auto rhsValue = dyn_cast<Value>(rhs);
  if (rhsInt) {
    rhsValue = createConstIndexValueOp(loc, b, rhsInt.value());
  } else {
    rhsValue = convertToIndexIfNeeded(rhsValue, loc, b);
    assert(isa<IndexType>(rhsValue.getType()));
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ascend-backend Changes to the Ascend NPU backend compiler Changes to C/C++ compiler backend (lib/, include/) python Changes to Python runtime or bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant