[mlir] fix infinite while in 1:N dialect conversion #123122
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Maksim Levental (makslevental) Changes#116524 introduced a "subtle" bug; I can't give a repro unless you're willing to build triton-lang/triton#5329 but here's a sketch: class ConvertFuncOp : public OpConversionPattern<tt::FuncOp> {
public:
using PointerCanonicalizationPattern::PointerCanonicalizationPattern;
LogicalResult
matchAndRewrite_(tt::FuncOp funcOp, OneToNOpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
...
// REPLACE tt.ptr WITH (tt.ptr, arith.constant 0) SAME tt.ptr!!!!
...
return success();
}
};Then in // mlir/lib/Transforms/Utils/DialectConversion.cpp
ConversionPatternRewriterImpl::remapValues(...) {
if (!currentTypeConverter) {
// The current pattern does not have a type converter. I.e., it does not
// distinguish between legal and illegal types. For each operand, simply
// pass through the most recently mapped values.
remapped.push_back(mapping.lookupOrDefault(operand));
continue;
}
}Then ConversionValueMapping::lookupOrDefault(...) {
...
do {
ValueVector next;
for (Value v : current) {
// ALWAYS FINDS tt.ptr AT INDEX 0
auto it = mapping.find({v});
if (it != mapping.end()) {
// ALWAYS REPLACES tt.ptr AT INDEX 0
// WITH (tt.ptr, arith.constant)
llvm::append_range(next, it->second);
} else {
next.push_back(v);
}
...
}
} while (true);result: The fix is to check whether we're "deepening" on the same Note, I can't add a test for this because the fail is an infinite loop. Full diff: https://github.com/llvm/llvm-project/pull/123122.diff 1 Files Affected:
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 403321d40d53c9..83d66dbe342d3f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -201,8 +201,9 @@ ConversionValueMapping::lookupOrDefault(Value from,
// If possible, Replace each value with (one or multiple) mapped values.
ValueVector next;
for (Value v : current) {
- auto it = mapping.find({v});
- if (it != mapping.end()) {
+ ValueVector vv{v};
+ auto it = mapping.find(vv);
+ if (it != mapping.end() && it->first != vv) {
llvm::append_range(next, it->second);
} else {
next.push_back(v);
|
Member
|
Did you close this PR on purpose? |
Member
// REPLACE tt.ptr WITH (tt.ptr, arith.constant 0) SAME tt.ptr!!!!Can you show the C++ code for that? There are two 2 ways to replace a value: |
makslevental
deleted the
users/makslevental/fix-1-n-dialect-conversion
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#116524 introduced a "subtle" bug; I can't give a repro unless you're willing to build triton-lang/triton#5329 but here's a sketch:
Then in
Then
result:
currentgrows without bounds, looks like(tt.ptr, arith.constant ,arith.constant, arith.constant....)andwhileloops forever.The fix is to check whether we're "deepening" on the same
Value.Note, I can't add a test for this because the fail is an infinite loop.