[Mlir-commits] [mlir] [mlir][ArithToSPIRV] Fix handling of unsigned integers in `convertIntegerAttr` (PR #204937)
Chirag Wattamwar
llvmlistbot at llvm.org
Sat Jun 20 21:55:25 PDT 2026
https://github.com/ChiragSW updated https://github.com/llvm/llvm-project/pull/204937
>From 5752d6bd6cf6240d6495102deef2208516f8cb99 Mon Sep 17 00:00:00 2001
From: Chirag <wattamwarchiraggg at gmail.com>
Date: Sat, 20 Jun 2026 22:30:43 +0530
Subject: [PATCH 1/2] [Fix][mlir](convert-tensor-to-spirv) mlir-opt crashes at
BuiltinAttributes.cpp:366 with assertion (getType().isIndex() ||
getType().isSignlessInteger()) && 'must be signless integer' failed.
---
mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp | 14 ++++++++++----
.../TensorToSPIRV/tensor-ops-to-spirv.mlir | 7 +++++++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
index 9a6d330db72fe..ff524b5360670 100644
--- a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
+++ b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
@@ -53,18 +53,24 @@ static BoolAttr convertBoolAttr(Attribute srcAttr, Builder builder) {
/// Returns null attribute if conversion fails.
static IntegerAttr convertIntegerAttr(IntegerAttr srcAttr, IntegerType dstType,
Builder builder) {
+ unsigned dstWidth = dstType.getWidth();
+ APInt srcValue = srcAttr.getValue();
+ bool isUnsigned = srcAttr.getType().isUnsignedInteger();
+ APInt dstValue = isUnsigned ? srcValue.zextOrTrunc(dstWidth)
+ : srcValue.sextOrTrunc(dstWidth);
+
// If the source number uses less active bits than the target bitwidth, then
// it should be safe to convert.
- if (srcAttr.getValue().isIntN(dstType.getWidth()))
- return builder.getIntegerAttr(dstType, srcAttr.getInt());
+ if (srcValue.isIntN(dstWidth))
+ return builder.getIntegerAttr(dstType, dstValue);
// XXX: Try again by interpreting the source number as a signed value.
// Although integers in the standard dialect are signless, they can represent
// a signed number. It's the operation decides how to interpret. This is
// dangerous, but it seems there is no good way of handling this if we still
// want to change the bitwidth. Emit a message at least.
- if (srcAttr.getValue().isSignedIntN(dstType.getWidth())) {
- auto dstAttr = builder.getIntegerAttr(dstType, srcAttr.getInt());
+ if (!isUnsigned && srcValue.isSignedIntN(dstWidth)) {
+ auto dstAttr = builder.getIntegerAttr(dstType, dstValue);
LLVM_DEBUG(llvm::dbgs() << "attribute '" << srcAttr << "' converted to '"
<< dstAttr << "' for type '" << dstType << "'\n");
return dstAttr;
diff --git a/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir b/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
index 65c6e0587129e..53ee699024068 100644
--- a/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
+++ b/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
@@ -72,6 +72,13 @@ func.func @tensor_2d() -> () {
return
}
+// CHECK-LABEL: func @tensor_unsigned_int
+// CHECK-NEXT: spirv.Constant dense<[10, 200]> : tensor<2xui32> : !spirv.array<2 x ui32>
+func.func @tensor_unsigned_int() -> () {
+ %x = arith.constant dense<[10, 200]> : tensor<2xui8>
+ return
+}
+
// We do not handle zero-element tensors yet. Just make we do not crash on them.
// CHECK-LABEL: func @tensor_2d_empty
// CHECK-NEXT: arith.constant dense<>
>From 668ac9acde4109910e5ddd58fe73873ffaac0691 Mon Sep 17 00:00:00 2001
From: Chirag <wattamwarchiraggg at gmail.com>
Date: Sun, 21 Jun 2026 10:24:07 +0530
Subject: [PATCH 2/2] reverted old changes, now moved fix to arith.constant
---
mlir/include/mlir/Dialect/Arith/IR/ArithOps.td | 8 ++------
.../lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp | 14 ++++----------
mlir/lib/Dialect/Arith/IR/ArithOps.cpp | 4 ++--
.../TensorToSPIRV/tensor-ops-to-spirv.mlir | 7 -------
mlir/test/Dialect/Arith/invalid.mlir | 16 ++++++++++++++++
5 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td b/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
index 1f8b07aed3f0d..423948c8734af 100644
--- a/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
+++ b/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
@@ -244,12 +244,8 @@ def Arith_ConstantOp : Op<Arith_Dialect, "constant",
}];
let arguments = (ins TypedAttrInterface:$value);
- // TODO: Disallow arith.constant to return anything other than a signless
- // integer or float like. Downstream users of Arith should only be
- // working with signless integers, floats, or vectors/tensors thereof.
- // However, it is necessary to allow arith.constant to return vectors/tensors
- // of strings and signed/unsigned integers (for now) as an artefact of
- // splitting the Standard dialect.
+ // Keep this broad so the custom verifier can produce arith.constant-specific
+ // diagnostics after checking that the value and result types match.
let results = (outs /*SignlessIntegerOrIndexOrFloatLike*/AnyType:$result);
let extraClassDeclaration = [{
diff --git a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
index ff524b5360670..9a6d330db72fe 100644
--- a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
+++ b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
@@ -53,24 +53,18 @@ static BoolAttr convertBoolAttr(Attribute srcAttr, Builder builder) {
/// Returns null attribute if conversion fails.
static IntegerAttr convertIntegerAttr(IntegerAttr srcAttr, IntegerType dstType,
Builder builder) {
- unsigned dstWidth = dstType.getWidth();
- APInt srcValue = srcAttr.getValue();
- bool isUnsigned = srcAttr.getType().isUnsignedInteger();
- APInt dstValue = isUnsigned ? srcValue.zextOrTrunc(dstWidth)
- : srcValue.sextOrTrunc(dstWidth);
-
// If the source number uses less active bits than the target bitwidth, then
// it should be safe to convert.
- if (srcValue.isIntN(dstWidth))
- return builder.getIntegerAttr(dstType, dstValue);
+ if (srcAttr.getValue().isIntN(dstType.getWidth()))
+ return builder.getIntegerAttr(dstType, srcAttr.getInt());
// XXX: Try again by interpreting the source number as a signed value.
// Although integers in the standard dialect are signless, they can represent
// a signed number. It's the operation decides how to interpret. This is
// dangerous, but it seems there is no good way of handling this if we still
// want to change the bitwidth. Emit a message at least.
- if (!isUnsigned && srcValue.isSignedIntN(dstWidth)) {
- auto dstAttr = builder.getIntegerAttr(dstType, dstValue);
+ if (srcAttr.getValue().isSignedIntN(dstType.getWidth())) {
+ auto dstAttr = builder.getIntegerAttr(dstType, srcAttr.getInt());
LLVM_DEBUG(llvm::dbgs() << "attribute '" << srcAttr << "' converted to '"
<< dstAttr << "' for type '" << dstType << "'\n");
return dstAttr;
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index dc3887c3e0b0e..397fa28227831 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -231,8 +231,8 @@ void arith::ConstantOp::getAsmResultNames(
LogicalResult arith::ConstantOp::verify() {
auto type = getType();
// Integer values must be signless.
- if (llvm::isa<IntegerType>(type) &&
- !llvm::cast<IntegerType>(type).isSignless())
+ if (auto intType = dyn_cast<IntegerType>(getElementTypeOrSelf(type));
+ intType && !intType.isSignless())
return emitOpError("integer return type must be signless");
// Any float or elements attribute are acceptable.
if (!llvm::isa<IntegerAttr, FloatAttr, ElementsAttr>(getValue())) {
diff --git a/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir b/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
index 53ee699024068..65c6e0587129e 100644
--- a/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
+++ b/mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir
@@ -72,13 +72,6 @@ func.func @tensor_2d() -> () {
return
}
-// CHECK-LABEL: func @tensor_unsigned_int
-// CHECK-NEXT: spirv.Constant dense<[10, 200]> : tensor<2xui32> : !spirv.array<2 x ui32>
-func.func @tensor_unsigned_int() -> () {
- %x = arith.constant dense<[10, 200]> : tensor<2xui8>
- return
-}
-
// We do not handle zero-element tensors yet. Just make we do not crash on them.
// CHECK-LABEL: func @tensor_2d_empty
// CHECK-NEXT: arith.constant dense<>
diff --git a/mlir/test/Dialect/Arith/invalid.mlir b/mlir/test/Dialect/Arith/invalid.mlir
index 421dac9cfee15..49f55e855663d 100644
--- a/mlir/test/Dialect/Arith/invalid.mlir
+++ b/mlir/test/Dialect/Arith/invalid.mlir
@@ -32,6 +32,14 @@ func.func @non_signless_constant() {
// -----
+func.func @non_signless_tensor_constant() {
+ // expected-error @+1 {{'arith.constant' op integer return type must be signless}}
+ %0 = arith.constant dense<[10, 20]> : tensor<2xui8>
+ return
+}
+
+// -----
+
func.func @complex_constant_wrong_attribute_type() {
// expected-error @+1 {{'arith.constant' op failed to verify that all of {value, result} have same type}}
%0 = "arith.constant" () {value = 1.0 : f32} : () -> complex<f32>
@@ -48,6 +56,14 @@ func.func @non_signless_constant() {
// -----
+func.func @non_signless_vector_constant() {
+ // expected-error @+1 {{'arith.constant' op integer return type must be signless}}
+ %0 = arith.constant dense<[0, 1]> : vector<2xsi32>
+ return
+}
+
+// -----
+
func.func @bitcast_different_bit_widths(%arg : f16) -> f32 {
// expected-error at +1 {{are cast incompatible}}
%res = arith.bitcast %arg : f16 to f32
More information about the Mlir-commits
mailing list