Skip to content
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

Enable tests for argmin/max and fix some bugs #1537

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

peterbell10
Copy link
Contributor

argmin/argmax is currently only tested in 1d and when we enable the tests for 2d it reveals a few bugs.

`argmin`/`argmax` is currently only tested in 1d and when we enable the tests
for 2d it reveals a few bugs.
@peterbell10 peterbell10 requested a review from ptillet as a code owner April 17, 2023 18:48
@@ -137,11 +137,12 @@ class SimplifyReduceCvt : public mlir::RewritePattern {
op->getLoc(), newTy, newOperands[i]);
}

rewriter.setInsertionPoint(reduce);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we're matching on the convert_layout op, the rewriter inserts directly after that op and potentially before the rest of the arguments are ready.

auto newReduce = rewriter.create<triton::ReduceOp>(
op->getLoc(), newOperands, reduce.getAxis());
auto &newCombineOp = newReduce.getCombineOp();
rewriter.inlineRegionBefore(reduce.getCombineOp(), newCombineOp,
newCombineOp.end());
rewriter.cloneRegionBefore(reduce.getCombineOp(), newCombineOp,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlining is destructive, so if the rewrite produces an invalid output this would break the original ReduceOp's region.

@ptillet ptillet merged commit a3c3e5a into triton-lang:main Apr 18, 2023
pingzhuu pushed a commit to siliconflow/triton that referenced this pull request Apr 2, 2024
…ton-lang#1537)

`argmin`/`argmax` is currently only tested in 1d and when we enable the
tests for 2d it reveals a few bugs.
ZzEeKkAa pushed a commit to ZzEeKkAa/triton that referenced this pull request Aug 5, 2024
…fset/emitIndices of dot layout. (triton-lang#1537)

Support repCluster in emitOffset/emitIndices utilis for dot layout.

---------

Signed-off-by: Tiotto, Ettore <[email protected]>
Co-authored-by: Tiotto, Ettore <[email protected]>
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