[Mlir-commits] [mlir] [mlir][bytecode] Fix clang 21/22 crash in getChecked<T> for types with only inherited getChecked (PR #189172)
Mehdi Amini
llvmlistbot at llvm.org
Sat Mar 28 08:46:02 PDT 2026
https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/189172
## Summary
The generic `getChecked<T>` helper in `BytecodeImplementation.h` uses `llvm::is_detected` to probe whether `T::getChecked(emitError, params...)` is callable without a `MLIRContext*` argument. For types that only inherit the base `StorageUserBase::getChecked(emitError, MLIRContext*, Args...)` template (e.g. `ArrayAttr`, `FusedLoc`), this SFINAE probe triggers a clang 21/22 internal assertion (`isInitialized() && "querying uninitialized conversion"`) during overload resolution.
Fix the root cause by ensuring `getChecked<T>` is only called for types that have an explicit, non-template no-context `getChecked(emitError, params...)` overload:
- Change the default `cBuilder` in `BytecodeBase.td` from `getChecked<T>` to `get<T>` for both `DialectAttribute` and `DialectType`. Types that need graceful failure (verifyInvariants returning null instead of asserting) override `cBuilder` explicitly.
- In `BuiltinDialectBytecode.td`: switch `FusedLoc` and `DenseResourceElementsAttr` from `getChecked<>` to `get<>` (these types have no explicit no-context getChecked); add explicit `getChecked<>` builders for `ComplexType`, `MemRefType`, `RankedTensorType`, and `UnrankedTensorType` (which do have explicit no-context overloads and can fail gracefully on invalid element types).
- In `BytecodeDialectGen.cpp`: generate `get<T>(context)` instead of `getChecked<T>(emitError, context)` for the zero-argument case.
- Add a note to `BytecodeImplementation.h` documenting the clang 21/22 limitation for future callers.
- Add a test for `ComplexType` with unsupported element type to `invalid-type-remapping.mlir`.
Fixes #188249
## Test plan
- [ ] Existing bytecode tests pass: `mlir/test/Bytecode/`
- [ ] New test in `mlir/test/Bytecode/invalid/invalid-type-remapping.mlir` covers `ComplexType` with unsupported element type
- [ ] Verify generated `.inc` file uses `get<T>` for types with only inherited getChecked and `getChecked<T>` for types with explicit no-context overloads
🤖 Generated with [Claude Code](https://claude.com/claude-code)
>From 084c9276fc653612d02f6c5e54d9f080e74e97c4 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Fri, 27 Mar 2026 11:11:56 -0700
Subject: [PATCH 1/3] [MLIR][SPIRV] Fix getPushConstantValue to use correct
integer type for offset
`getPushConstantValue` was creating the offset constant with
`getI32IntegerAttr(offset)`, which produces an i32 attribute regardless
of the `integerType` parameter. When `integerType` is i64, this results
in invalid SPIRV where the constant attribute type (i32) does not match
the result type (i64).
Fix by using `getIntegerAttr(integerType, offset)` so the attribute type
matches the declared integer type.
Fixes #185149
Assisted-by: Claude Code
---
mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp b/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp
index 2c9e9c040d460..8769f0655759f 100644
--- a/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp
+++ b/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp
@@ -1320,8 +1320,8 @@ Value spirv::getPushConstantValue(Operation *op, unsigned elementCount,
loc, parent->getRegion(0).front(), elementCount, builder, integerType);
Value zeroOp = spirv::ConstantOp::getZero(integerType, loc, builder);
- Value offsetOp = spirv::ConstantOp::create(builder, loc, integerType,
- builder.getI32IntegerAttr(offset));
+ Value offsetOp = spirv::ConstantOp::create(
+ builder, loc, integerType, builder.getIntegerAttr(integerType, offset));
auto addrOp = spirv::AddressOfOp::create(builder, loc, varOp);
auto acOp = spirv::AccessChainOp::create(builder, loc, addrOp,
llvm::ArrayRef({zeroOp, offsetOp}));
>From 7c909b4f67068b9854b74f3dc9d7b14449f7f4fb Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Sat, 28 Mar 2026 07:49:50 -0700
Subject: [PATCH 2/3] [MLIR] Fix crash in remove-dead-values when branch is in
dead block
When a branch operation is in a statically unreachable (dead) block, its
forwarded operands are also dead. processBranchOp was propagating this
deadness to the successor block arguments unconditionally, even when those
block arguments are live because they are also forwarded from other reachable
predecessor branches.
This caused an assertion failure in processSimpleOp when an op had a dead
operand (the incorrectly-marked-dead block argument) but a live result.
Fix by checking, for each successor block argument that would be marked dead,
whether the liveness analysis considers it live. If so, do not add it to the
non-live set.
Fixes #182263
Assisted-by: Claude Code
---
mlir/lib/Transforms/RemoveDeadValues.cpp | 11 +++++++++
mlir/test/Transforms/remove-dead-values.mlir | 26 ++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index f0a210a2ededb..3edb9af671150 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -511,6 +511,17 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la,
// Do (2)
BitVector successorNonLive =
markLives(operandValues, nonLiveSet, la).flip();
+ // A block argument should not be considered dead if the liveness analysis
+ // determines it is live. This can happen when this branch is in a
+ // statically unreachable (dead) block: the forwarded operand appears dead
+ // because it is in the dead block, but the successor block argument may
+ // still be live because it is also forwarded from other live predecessor
+ // branches.
+ for (auto [index, blockArg] :
+ llvm::enumerate(successorBlock->getArguments())) {
+ if (successorNonLive[index] && hasLive({blockArg}, nonLiveSet, la))
+ successorNonLive.reset(index);
+ }
collectNonLiveValues(nonLiveSet, successorBlock->getArguments(),
successorNonLive);
diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index 64088ce15cd48..d2f5c4ab95574 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -868,3 +868,29 @@ module @func_with_non_call_users {
}
spirv.EntryPoint "GLCompute" @callee
}
+
+// -----
+
+// Regression test: verify that the pass does not crash when a branch op
+// forwards a value through intermediate block arguments to a join block.
+// Previously, processBranchOp incorrectly used forwarded operand liveness
+// to determine successor block argument liveness, causing incorrect marking
+// of live values as dead. (https://github.com/llvm/llvm-project/issues/182263)
+//
+// CHECK-LABEL: func.func @branch_forwarded_block_arg_liveness
+// CHECK-CANONICALIZE-LABEL: func.func @branch_forwarded_block_arg_liveness
+func.func @branch_forwarded_block_arg_liveness(%x: i64) -> i64 {
+ %c1 = arith.constant 1 : i64
+ %c2 = arith.constant 2 : i64
+ %cmp = arith.cmpi slt, %c1, %c2 : i64
+ cf.cond_br %cmp, ^bb1(%c1 : i64), ^bb2(%c1 : i64)
+^bb1(%a: i64):
+ cf.br ^bb3(%a : i64)
+^bb2(%b: i64):
+ cf.br ^bb3(%b : i64)
+^bb3(%arg0: i64):
+ // CHECK: arith.addi
+ // CHECK-CANONICALIZE: arith.addi
+ %final = arith.addi %c1, %arg0 : i64
+ func.return %final : i64
+}
>From d93d87198477d1ef0261c786b6c27264a05d4d67 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Sat, 28 Mar 2026 08:41:04 -0700
Subject: [PATCH 3/3] [mlir][bytecode] Fix clang 21/22 crash in getChecked<T>
for types with only inherited getChecked
The generic `getChecked<T>` helper in BytecodeImplementation.h uses
`llvm::is_detected` to probe whether `T::getChecked(emitError, params...)`
is callable without a `MLIRContext*` argument. For types that only inherit
the base `StorageUserBase::getChecked(emitError, MLIRContext*, Args...)`
template (e.g. ArrayAttr, FusedLoc), this SFINAE probe triggers a clang
21/22 internal assertion (`isInitialized() && "querying uninitialized
conversion"`) during overload resolution.
Fix the root cause by ensuring `getChecked<T>` is only called for types
that have an explicit, non-template no-context
`getChecked(emitError, params...)` overload:
- Change the default `cBuilder` in `BytecodeBase.td` from
`getChecked<T>` to `get<T>` for both `DialectAttribute` and
`DialectType`. Types that need graceful failure (verifyInvariants
returning null instead of asserting) override `cBuilder` explicitly.
- In `BuiltinDialectBytecode.td`: switch `FusedLoc` and
`DenseResourceElementsAttr` from `getChecked<>` to `get<>` (these
types have no explicit no-context getChecked); add explicit
`getChecked<>` builders for `ComplexType`, `MemRefType`,
`RankedTensorType`, and `UnrankedTensorType` (which do have explicit
no-context overloads and can fail gracefully on invalid element types).
- In `BytecodeDialectGen.cpp`: generate `get<T>(context)` instead of
`getChecked<T>(emitError, context)` for the zero-argument case.
- Add a note to `BytecodeImplementation.h` documenting the clang 21/22
limitation for future callers.
- Add a test for ComplexType with unsupported element type to
`invalid-type-remapping.mlir`.
Fixes #188249
Assisted-by: Claude Code
---
.../mlir/Bytecode/BytecodeImplementation.h | 8 +++++++
.../include/mlir/IR/BuiltinDialectBytecode.td | 22 +++++++++++++++----
mlir/include/mlir/IR/BytecodeBase.td | 16 ++++++++++++--
.../invalid/invalid-type-remapping.mlir | 12 ++++++++++
mlir/tools/mlir-tblgen/BytecodeDialectGen.cpp | 5 +----
5 files changed, 53 insertions(+), 10 deletions(-)
diff --git a/mlir/include/mlir/Bytecode/BytecodeImplementation.h b/mlir/include/mlir/Bytecode/BytecodeImplementation.h
index fe85908e476ff..8e0a2e2efa7e2 100644
--- a/mlir/include/mlir/Bytecode/BytecodeImplementation.h
+++ b/mlir/include/mlir/Bytecode/BytecodeImplementation.h
@@ -543,6 +543,14 @@ using has_get_checked_method = decltype(T::getChecked(std::declval<Ts>()...));
/// the base `StorageUserBase::getChecked` template (e.g. ArrayAttr), that
/// template instantiation requires a complete storage type which may not be
/// available in the bytecode reading TU.
+///
+/// NOTE: callers must ensure T has an explicit no-context
+/// `getChecked(emitError, params...)` overload (i.e. the overload set does not
+/// consist solely of inherited StorageUserBase templates). Calling this helper
+/// for types that only inherit the base template triggers a compiler bug in
+/// clang 21/22 (assertion `isInitialized()` in ImplicitConversionSequence)
+/// during SFINAE evaluation. The generated bytecode read functions use
+/// `get<T>` for such types instead of this helper.
template <typename T, typename... Ts>
auto getChecked(function_ref<InFlightDiagnostic()> emitError,
MLIRContext *context, Ts &&...params) {
diff --git a/mlir/include/mlir/IR/BuiltinDialectBytecode.td b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
index 8a1f3d5e5b2e0..d0b13a20a9040 100644
--- a/mlir/include/mlir/IR/BuiltinDialectBytecode.td
+++ b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
@@ -117,7 +117,7 @@ def FileLineColLoc : DialectAttribute<(attr
}
let cType = "FusedLoc",
- cBuilder = "cast<FusedLoc>(getChecked<FusedLoc>([&]() { return reader.emitError(); }, context, $_args))" in {
+ cBuilder = "cast<FusedLoc>(get<FusedLoc>(context, $_args))" in {
def FusedLoc : DialectAttribute<(attr
Array<Location>:$locations
)> {
@@ -144,7 +144,7 @@ def DenseResourceElementsAttr : DialectAttribute<(attr
ResourceHandle<"DenseResourceElementsHandle">:$rawHandle
)> {
// Note: order of serialization does not match order of builder.
- let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, type, *rawHandle)";
+ let cBuilder = "get<$_resultType>(context, type, *rawHandle)";
}
let cType = "RankedTensorType" in {
@@ -153,6 +153,9 @@ def RankedTensorType : DialectType<(type
Type:$elementType
)> {
let printerPredicate = "!$_val.getEncoding()";
+ // Use getChecked to return null (and emit a diagnostic) instead of asserting
+ // when the element type is invalid.
+ let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
}
def RankedTensorTypeWithEncoding : DialectType<(type
@@ -237,7 +240,11 @@ def Float128Type : DialectType<(type)>;
def ComplexType : DialectType<(type
Type:$elementType
-)>;
+)> {
+ // Use getChecked to return null (and emit a diagnostic) instead of asserting
+ // when the element type is not a valid complex element type.
+ let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
+}
def MemRefLayout: WithType<"MemRefLayoutAttrInterface", Attribute>;
@@ -248,6 +255,9 @@ def MemRefType : DialectType<(type
MemRefLayout:$layout
)> {
let printerPredicate = "!$_val.getMemorySpace()";
+ // Use getChecked to return null (and emit a diagnostic) instead of asserting
+ // when element type or layout is invalid.
+ let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
}
def MemRefTypeWithMemSpace : DialectType<(type
@@ -288,7 +298,11 @@ def UnrankedMemRefTypeWithMemSpace : DialectType<(type
def UnrankedTensorType : DialectType<(type
Type:$elementType
-)>;
+)> {
+ // Use getChecked to return null (and emit a diagnostic) instead of asserting
+ // when the element type is invalid.
+ let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
+}
let cType = "VectorType" in {
def VectorType : DialectType<(type
diff --git a/mlir/include/mlir/IR/BytecodeBase.td b/mlir/include/mlir/IR/BytecodeBase.td
index 184c81e6a5f7d..2e31c97f11f91 100644
--- a/mlir/include/mlir/IR/BytecodeBase.td
+++ b/mlir/include/mlir/IR/BytecodeBase.td
@@ -147,11 +147,23 @@ class DialectAttrOrType<dag d> {
class DialectAttribute<dag d> : DialectAttrOrType<d>, AttributeKind {
let cParser = "succeeded($_reader.readAttribute<$_resultType>($_var))";
- let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
+ // Use get<> by default. Types with an explicit no-context getChecked
+ // (verifyInvariants that should return null instead of asserting) should
+ // override cBuilder to call T::getChecked or getChecked<T> explicitly.
+ // Avoid using the generic getChecked<T> helper for types that only inherit
+ // StorageUserBase::getChecked, because the SFINAE probe in that helper
+ // triggers a clang 21/22 compiler crash for such types.
+ let cBuilder = "get<$_resultType>(context, $_args)";
}
class DialectType<dag d> : DialectAttrOrType<d>, TypeKind {
let cParser = "succeeded($_reader.readType<$_resultType>($_var))";
- let cBuilder = "getChecked<$_resultType>([&]() { return reader.emitError(); }, context, $_args)";
+ // Use get<> by default. Types with an explicit no-context getChecked
+ // (verifyInvariants that should return null instead of asserting) should
+ // override cBuilder to call T::getChecked or getChecked<T> explicitly.
+ // Avoid using the generic getChecked<T> helper for types that only inherit
+ // StorageUserBase::getChecked, because the SFINAE probe in that helper
+ // triggers a clang 21/22 compiler crash for such types.
+ let cBuilder = "get<$_resultType>(context, $_args)";
}
class DialectAttributes<string d> {
diff --git a/mlir/test/Bytecode/invalid/invalid-type-remapping.mlir b/mlir/test/Bytecode/invalid/invalid-type-remapping.mlir
index 44d0a4eb8bb4a..cfe6f79bc29d5 100644
--- a/mlir/test/Bytecode/invalid/invalid-type-remapping.mlir
+++ b/mlir/test/Bytecode/invalid/invalid-type-remapping.mlir
@@ -43,6 +43,18 @@ module {
// -----
+// CHECK: invalid element type for complex
+// CHECK: failed to read bytecode
+// ComplexType whose element type is replaced by one that is neither int nor
+// float — previously crashed with an assertion when using get<ComplexType>.
+module {
+ func.func @complex_unsupported_elem_type(%arg0: complex<i32>) {
+ return
+ }
+}
+
+// -----
+
// CHECK: DenseTypedElementsAttr element type must implement DenseElementTypeInterface, but got: '!test.i32'
// CHECK: failed to read bytecode
// DenseTypedElementsAttr whose element type is replaced by one that does not
diff --git a/mlir/tools/mlir-tblgen/BytecodeDialectGen.cpp b/mlir/tools/mlir-tblgen/BytecodeDialectGen.cpp
index dd178b5e5d232..d8004a663aaee 100644
--- a/mlir/tools/mlir-tblgen/BytecodeDialectGen.cpp
+++ b/mlir/tools/mlir-tblgen/BytecodeDialectGen.cpp
@@ -206,10 +206,7 @@ void Generator::emitParseHelper(StringRef kind, StringRef returnType,
auto funScope = ios.scope("{\n", "}");
if (args.empty()) {
- ios << formatv(
- "return getChecked<{0}>([&]() {{ return reader.emitError(); }, "
- "context);\n",
- returnType);
+ ios << formatv("return get<{0}>(context);\n", returnType);
return;
}
More information about the Mlir-commits
mailing list