[Mlir-commits] [mlir] [Fix][mlir](convert-tensor-to-spirv) mlir-opt crashes at BuiltinAttributes.cpp:366 with assertion (getType().isIndex() || getType().isSignlessInteger()) && 'must be signless integer' failed. (PR #204937)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-spirv

Author: Chirag Wattamwar (ChiragSW)

<details>
<summary>Changes</summary>

Fixes: #<!-- -->204911 

### Root cause
The crash happened because SPIR-V constant lowering used IntegerAttr::getInt() on an unsigned integer attribute from tensor<2xui8>. getInt() only supports signless/index integer attrs, so it worked that way.

### Fix
Fix was to avoid getInt() and convert through APInt directly by using
zero-extension: for unsigned attrs 
sign-extension: for signed/signless attr
Also added a regression test for the same in tensor-ops-to-spirv.mlir

---
Full diff: https://github.com/llvm/llvm-project/pull/204937.diff


2 Files Affected:

- (modified) mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp (+10-4) 
- (modified) mlir/test/Conversion/TensorToSPIRV/tensor-ops-to-spirv.mlir (+7) 


``````````diff
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<>

``````````

</details>


https://github.com/llvm/llvm-project/pull/204937