[MLIR] Add support for explicitly signed / unsigned integer constants

[andidr]

Summary

Currently, only signless integers are allowed as constants. This patch additionally allows for explicitly signed and explicitly unsigned integer constants.

[stephenneuendorffer]

mlir/test/IR/parser.mlir

501

My understanding of the design is that i32 is signless, ui32 is unsigned, and si32 is signed... It seems like you're not testing the explicitly signed case?

[rriddle]

Thanks for the patch! StandardOps is designed to use only signless integers, any deviation will require an RFC and justification as to why the change is desired. See https://mlir.llvm.org/docs/Rationale/#integer-signedness-semantics
Thanks again.

[andidr]

It seems that I have misinterpreted https://reviews.llvm.org/D72533 as the introduction of explicitly signed / unsigned types to MLIR in general, including std. Thanks for your clarification!

The patch originates from work on a frontend for Tensor Expressions that includes unsigned types in the source language and that generates operations from std and linalg. The translation is implemenyed recursively with functions simply returning mlir::Values. D72533 seemed like a comfortable way to keep this scheme and to avoid modeling the sign externally. However, without support for explicitly signed data types for the operations from std, this means that either the operands need to be converted to signless integers or the operations need to be re-implemented outside of std with support for explicitly signed / unsigned operands. I am not aware of any way to convert the operands and re-implementation of the operations seems quite redundant.

I understand your motivations to keep signless integers in std wherever possible. Any hint on how to work around the above-mentioned issues is highly appreciated. Thanks!

[andidr]

mlir/test/IR/parser.mlir

501

Thanks for pointing this out. This is coherent with my understanding. I'll add test cases for signed integers should the decision to support only signless integers in std be revised.

[rriddle]

In D76136#1922172, @andidr wrote:

It seems that I have misinterpreted https://reviews.llvm.org/D72533 as the introduction of explicitly signed / unsigned types to MLIR in general, including std. Thanks for your clarification!

The patch originates from work on a frontend for Tensor Expressions that includes unsigned types in the source language and that generates operations from std and linalg. The translation is implemenyed recursively with functions simply returning mlir::Values. D72533 seemed like a comfortable way to keep this scheme and to avoid modeling the sign externally. However, without support for explicitly signed data types for the operations from std, this means that either the operands need to be converted to signless integers or the operations need to be re-implemented outside of std with support for explicitly signed / unsigned operands. I am not aware of any way to convert the operands and re-implementation of the operations seems quite redundant.

I understand your motivations to keep signless integers in std wherever possible. Any hint on how to work around the above-mentioned issues is highly appreciated. Thanks!

I'd love to discuss more. To make it easier, could you post about what you are doing on https://llvm.discourse.group/c/llvm-project/mlir/?

[andidr]

I'd love to discuss more. To make it easier, could you post about what you are doing on https://llvm.discourse.group/c/llvm-project/mlir/?

Done: https://llvm.discourse.group/t/explicitly-signed-unsigned-integers-in-std/731. Thanks for the suggestion!

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

if (auto intCst = getValue().dyn_cast<IntegerAttr>()) {
 
     // Sugar i1 constants with 'true' and 'false'.
     if (intTy && intTy.getWidth() == 1)
       return setNameFn(getResult(), (intCst.getInt() ? "true" : "false"));
 
     // Otherwise, build a complex name with the value and type.
     SmallString<32> specialNameBuffer;
     llvm::raw_svector_ostream specialName(specialNameBuffer);
-    specialName << 'c' << intCst.getInt();
+    specialName << 'c' << intCst.getValue();
     if (intTy)
       specialName << '_' << type;
     setNameFn(getResult(), specialName.str());
 
   } else if (type.isa<FunctionType>()) {
     setNameFn(getResult(), "f");
   } else {
     setNameFn(getResult(), "cst");

 
 bool ConstantFloatOp::classof(Operation *op) {
   return ConstantOp::classof(op) && op->getResult(0).getType().isa<FloatType>();
 }
 
 /// ConstantIntOp only matches values whose result type is an IntegerType.
 bool ConstantIntOp::classof(Operation *op) {
   return ConstantOp::classof(op) &&
-         op->getResult(0).getType().isSignlessInteger();
+    (op->getResult(0).getType().isa<IntegerType>());
 }
 
 void ConstantIntOp::build(Builder *builder, OperationState &result,
                           int64_t value, unsigned width) {
   Type type = builder->getIntegerType(width);
   ConstantOp::build(builder, result, type,
                     builder->getIntegerAttr(type, value));
 }

mlir/test/IR/parser.mlir

 ^bb2(%x2 : i64, %y2 : i32, %z2 : i32):
   %z = "foo"() : () -> i32
   return %z : i32
 }
 
 
 // Test pretty printing of constant names.
 // CHECK-LABEL: func @constants
-func @constants() -> (i32, i23, i23, i1, i1) {
+func @constants() -> (i32, i23, ui32, ui23, i23, i1, i1) {
   // CHECK: %{{.*}} = constant 42 : i32
   %x = constant 42 : i32
   // CHECK: %{{.*}} = constant 17 : i23
   %y = constant 17 : i23
 
+  // CHECK: %{{.*}} = constant 42 : ui32
+  %ux = constant 42 : ui32
+  // CHECK: %{{.*}} = constant 17 : ui23
+  %uy = constant 17 : ui23
+
   // This is a redundant definition of 17, the asmprinter gives it a unique name
   // CHECK: %{{.*}} = constant 17 : i23
   %z = constant 17 : i23
 
   // CHECK: %{{.*}} = constant 1 : i1
   %t = constant 1 : i1
   // CHECK: %{{.*}} = constant 0 : i1
   %f = constant 0 : i1
 
   // The trick to parse type declarations should not interfere with hex
   // literals.
   // CHECK: %{{.*}} = constant 3890 : i32
   %h = constant 0xf32 : i32
 
   // CHECK: return %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}
-  return %x, %y, %z, %t, %f : i32, i23, i23, i1, i1
+  return %x, %y, %ux, %uy, %z, %t, %f : i32, i23, ui32, ui23, i23, i1, i1
 }
 
 // CHECK-LABEL: func @typeattr
 func @typeattr() -> () {
 ^bb0:
 // CHECK: "foo"() {bar = tensor<*xf32>} : () -> ()
   "foo"(){bar = tensor<*xf32>} : () -> ()
   return