Skip to content

[revert](feat) revert Replace LLVM dialect with MLIR (#1038) - #1517

Open
245516766 wants to merge 1 commit into
triton-lang:main-devfrom
245516766:revert-dev-cwy
Open

[revert](feat) revert Replace LLVM dialect with MLIR (#1038)#1517
245516766 wants to merge 1 commit into
triton-lang:main-devfrom
245516766:revert-dev-cwy

Conversation

@245516766

Copy link
Copy Markdown
Contributor

This reverts commit daceb99.

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 12, 2026
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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

  • ✅ Successfully posted inline: 2 comment(s)

Comment on lines +192 to +194
Value getSSBufferPtr(bool isAIC, int groupIdx, int ptrSetIdx,
DenseMap<int, Value> &precomputedPtrs,
SmallVector<SmallVector<Value>> ssbufferPtrs);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[performance · high]
The ssbufferPtrs parameter was changed from ArrayRef<SmallVector<Value>> (a lightweight view type) to SmallVector<SmallVector<Value>> (pass-by-value), which now causes a deep copy of the entire nested SmallVector structure on every call. This function is called numerous times in loops (e.g., ~19 call sites in the .cpp file), making this a significant performance regression. The parameter should be passed by const reference (const SmallVector<SmallVector<Value>> &) or kept as ArrayRef<SmallVector<Value>> to avoid unnecessary copying.

Suggestion:

Suggested change
Value getSSBufferPtr(bool isAIC, int groupIdx, int ptrSetIdx,
DenseMap<int, Value> &precomputedPtrs,
SmallVector<SmallVector<Value>> ssbufferPtrs);
Value getSSBufferPtr(bool isAIC, int groupIdx, int ptrSetIdx,
DenseMap<int, Value> &precomputedPtrs,
const SmallVector<SmallVector<Value>> &ssbufferPtrs);

Comment on lines +140 to +141
Value foundValue = findResult.value().first;
Type dataType = findResult.value().second;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[maintainability · low]
Unused variable foundValue is extracted from findResult.value().first but never referenced afterwards. Only dataType (the second element of the pair) is actually used. This will likely trigger compiler warnings about unused variables and introduces unnecessary code.

Suggestion:

Suggested change
Value foundValue = findResult.value().first;
Type dataType = findResult.value().second;
Type dataType = findResult.value().second;

@245516766

Copy link
Copy Markdown
Contributor Author

/retry

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.

2 participants