[Mlir-commits] [mlir] [mlir][ArithToSPIRV] Fix handling of unsigned integers in `convertIntegerAttr` (PR #204937)
Chirag Wattamwar
llvmlistbot at llvm.org
Sun Jun 21 00:28:16 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/3] [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/3] 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
>From 80cee6606edab653c6faab925f7a5a461f067e27 Mon Sep 17 00:00:00 2001
From: Chirag <wattamwarchiraggg at gmail.com>
Date: Sun, 21 Jun 2026 12:57:43 +0530
Subject: [PATCH 3/3] removed conversion cases from tests as tests were using
arith.constant s
---
.../Dialect/Tosa/tosa-arith-const-to-tosa-const.mlir | 8 --------
mlir/test/Dialect/common_folders.mlir | 12 ++++++------
mlir/test/lib/Dialect/Test/TestOps.td | 4 ++--
3 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/mlir/test/Dialect/Tosa/tosa-arith-const-to-tosa-const.mlir b/mlir/test/Dialect/Tosa/tosa-arith-const-to-tosa-const.mlir
index fc2d77ef375ec..79219510a239f 100644
--- a/mlir/test/Dialect/Tosa/tosa-arith-const-to-tosa-const.mlir
+++ b/mlir/test/Dialect/Tosa/tosa-arith-const-to-tosa-const.mlir
@@ -65,14 +65,6 @@ func.func @rewrite_resource_tensor() -> tensor<4xf32> {
// -----
-// CHECK-LABEL: func.func @rewrite_quant_tensor
-// CHECK: %[[CST:.*]] = "tosa.const"() <{values = dense<[10, 20]> : tensor<2xui8>}> : () -> tensor<2xui8>
-func.func @rewrite_quant_tensor() -> tensor<2xui8> {
- %c = arith.constant dense<[10, 20]> : tensor<2xui8>
- return %c : tensor<2xui8>
-}
-
-// -----
// CHECK-LABEL: func.func @rewrite_quant_uniform_tensor
// CHECK: %[[CST:.*]] = "tosa.const"() <{values = dense<["10", "20"]> : tensor<2x!quant.uniform<i8:f32, 5.000000e-01>>}> : () -> tensor<2x!quant.uniform<i8:f32, 5.000000e-01>>
diff --git a/mlir/test/Dialect/common_folders.mlir b/mlir/test/Dialect/common_folders.mlir
index 92598b4937552..9e38c1479e2e7 100644
--- a/mlir/test/Dialect/common_folders.mlir
+++ b/mlir/test/Dialect/common_folders.mlir
@@ -1,12 +1,12 @@
// RUN: mlir-opt %s --test-fold-type-converting-op --split-input-file | FileCheck %s
-// CHECK-LABEL: @test_fold_unary_op_f32_to_si32(
-func.func @test_fold_unary_op_f32_to_si32() -> tensor<4x2xsi32> {
- // CHECK-NEXT: %[[POSITIVE_ONE:.*]] = arith.constant dense<1> : tensor<4x2xsi32>
- // CHECK-NEXT: return %[[POSITIVE_ONE]] : tensor<4x2xsi32>
+// CHECK-LABEL: @test_fold_unary_op_f32_to_i32(
+func.func @test_fold_unary_op_f32_to_i32() -> tensor<4x2xi32> {
+ // CHECK-NEXT: %[[POSITIVE_ONE:.*]] = arith.constant dense<1> : tensor<4x2xi32>
+ // CHECK-NEXT: return %[[POSITIVE_ONE]] : tensor<4x2xi32>
%operand = arith.constant dense<5.1> : tensor<4x2xf32>
- %sign = test.sign %operand : (tensor<4x2xf32>) -> tensor<4x2xsi32>
- return %sign : tensor<4x2xsi32>
+ %sign = test.sign %operand : (tensor<4x2xf32>) -> tensor<4x2xi32>
+ return %sign : tensor<4x2xi32>
}
// -----
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 31002d1f17a75..fed7da09467a0 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -1358,10 +1358,10 @@ def OpQ : TEST_Op<"op_q"> {
let results = (outs AnyType);
}
-// Test constant-folding a pattern that maps `(F32) -> SI32`.
+// Test constant-folding a pattern that maps `(F32) -> I32`.
def SignOp : TEST_Op<"sign", [SameOperandsAndResultShape]> {
let arguments = (ins RankedTensorOf<[F32]>:$operand);
- let results = (outs RankedTensorOf<[SI32]>:$result);
+ let results = (outs RankedTensorOf<[I32]>:$result);
let assemblyFormat = [{
$operand attr-dict `:` functional-type(operands, results)
More information about the Mlir-commits
mailing list