[mlir][Parser] Fix attribute parser errors for ui64

Summary:
The attribute parser fails to correctly parse unsigned 64 bit
attributes as the check `isNegative ? (int64_t)-val.getValue() >= 0
: (int64_t)val.getValue() < 0` will falsely detect an overflow for
unsigned values larger than 2^63-1.

This patch reworks the overflow logic to instead of doing arithmetic
on int64_t use APInt::isSignBitSet() and knowledge of the attribute
type.

Test-cases which verify the de-facto behavior of the parser and
triggered the previous faulty handing of unsigned 64 bit attrbutes are
also added.

Differential Revision: https://reviews.llvm.org/D76493

mlir/lib/Parser/Parser.cpp

@@ -1759,6 +1759,33 @@ static Optional<APFloat> buildHexadecimalFloatLiteral(Parser *p, FloatType type,
   return APFloat(type.getFloatSemantics(), apInt);
 }
 
+/// Construct an APint from a parsed value, a known attribute type and
+/// sign.
+static Optional<APInt> buildAttributeAPInt(Type type, bool isNegative,
+                                           uint64_t value) {
+  // We have the integer literal as an uint64_t in val, now convert it into an
+  // APInt and check that we don't overflow.
+  int width = type.isIndex() ? 64 : type.getIntOrFloatBitWidth();
+  APInt apInt(width, value, isNegative);
+  if (apInt != value)
+    return llvm::None;
+
+  if (isNegative) {
+    // The value is negative, we have an overflow if the sign bit is not set
+    // in the negated apInt.
+    apInt.negate();
+    if (!apInt.isSignBitSet())
+      return llvm::None;
+  } else if ((type.isSignedInteger() || type.isIndex()) &&
+             apInt.isSignBitSet()) {
+    // The value is a positive signed integer or index,
+    // we have an overflow if the sign bit is set.
+    return llvm::None;
+  }
+
+  return apInt;
+}
+
 /// Parse a decimal or a hexadecimal literal, which can be either an integer
 /// or a float attribute.
 Attribute Parser::parseDecOrHexAttr(Type type, bool isNegative) {
@@ -1809,19 +1836,12 @@ Attribute Parser::parseDecOrHexAttr(Type type, bool isNegative) {
     return nullptr;
   }
 
-  // Parse the integer literal.
-  int width = type.isIndex() ? 64 : type.getIntOrFloatBitWidth();
-  APInt apInt(width, *val, isNegative);
-  if (apInt != *val)
+  Optional<APInt> apInt = buildAttributeAPInt(type, isNegative, *val);
+
+  if (!apInt)
     return emitError(loc, "integer constant out of range for attribute"),
            nullptr;
-
-  // Otherwise construct an integer attribute.
-  if (isNegative ? (int64_t)-val.getValue() >= 0 : (int64_t)val.getValue() < 0)
-    return emitError(loc, "integer constant out of range for attribute"),
-           nullptr;
-
-  return builder.getIntegerAttr(type, isNegative ? -apInt : apInt);
+  return builder.getIntegerAttr(type, *apInt);
 }
 
 /// Parse elements values stored within a hex etring. On success, the values are
@@ -2038,16 +2058,15 @@ DenseElementsAttr TensorLiteralParser::getIntAttr(llvm::SMLoc loc,
 
     // Create APInt values for each element with the correct bitwidth.
     auto val = token.getUInt64IntegerValue();
-    if (!val.hasValue() || (isNegative ? (int64_t)-val.getValue() >= 0
-                                       : (int64_t)val.getValue() < 0)) {
+    if (!val.hasValue()) {
       p.emitError(tokenLoc, "integer constant out of range for attribute");
       return nullptr;
     }
-    APInt apInt(eltTy.getWidth(), val.getValue(), isNegative);
-    if (apInt != val.getValue())
+    Optional<APInt> apInt = buildAttributeAPInt(eltTy, isNegative, *val);
+    if (!apInt)
       return (p.emitError(tokenLoc, "integer constant out of range for type"),
               nullptr);
-    intElements.push_back(isNegative ? -apInt : apInt);
+    intElements.push_back(*apInt);
   }
 
   return DenseElementsAttr::get(type, intElements);

mlir/test/Dialect/SPIRV/canonicalize.mlir

@@ -273,7 +273,7 @@ func @const_fold_scalar_imul_flow() -> (i32, i32, i32) {
   %c1 = spv.constant 2 : i32
   %c2 = spv.constant 4 : i32
   %c3 = spv.constant 4294967295 : i32  // 2^32 - 1 : 0xffff ffff
-  %c4 = spv.constant -2147483649 : i32 // -2^31 - 1: 0x7fff ffff
+  %c4 = spv.constant 2147483647 : i32  // 2^31 - 1 : 0x7fff ffff
 
   // (0xffff ffff << 1) = 0x1 ffff fffe -> 0xffff fffe
   // CHECK: %[[CST2:.*]] = spv.constant -2
@@ -331,7 +331,7 @@ func @const_fold_scalar_isub_flow() -> (i32, i32, i32, i32) {
   %c1 = spv.constant 0 : i32
   %c2 = spv.constant 1 : i32
   %c3 = spv.constant 4294967295 : i32  // 2^32 - 1 : 0xffff ffff
-  %c4 = spv.constant -2147483649 : i32 // -2^31 - 1: 0x7fff ffff
+  %c4 = spv.constant 2147483647 : i32  // 2^31     : 0x7fff ffff
   %c5 = spv.constant -1 : i32          //          : 0xffff ffff
   %c6 = spv.constant -2 : i32          //          : 0xffff fffe
 

mlir/test/IR/attribute.mlir

@@ -33,6 +33,83 @@ func @int_attrs_pass() {
 
 // -----
 
+//===----------------------------------------------------------------------===//
+// Check that the maximum and minumum integer attribute values are
+// representable and preserved during a round-trip.
+//===----------------------------------------------------------------------===//
+
+func @int_attrs_pass() {
+  "test.in_range_attrs"() {
+    // CHECK: attr_00 = -128 : i8
+    attr_00 = -128 : i8,
+    // CHECK-SAME: attr_01 = 127 : i8
+    attr_01 = 127 : i8,
+    // CHECK-SAME: attr_02 = -128 : si8
+    attr_02 = -128 : si8,
+    // CHECK-SAME: attr_03 = 127 : si8
+    attr_03 = 127 : si8,
+    // CHECK-SAME: attr_04 = 255 : ui8
+    attr_04 = 255 : ui8,
+    // CHECK-SAME: attr_05 = -32768 : i16
+    attr_05 = -32768 : i16,
+    // CHECK-SAME: attr_06 = 32767 : i16
+    attr_06 = 32767 : i16,
+    // CHECK-SAME: attr_07 = -32768 : si16
+    attr_07 = -32768 : si16,
+    // CHECK-SAME: attr_08 = 32767 : si16
+    attr_08 = 32767 : si16,
+    // CHECK-SAME: attr_09 = 65535 : ui16
+    attr_09 = 65535 : ui16,
+    // CHECK-SAME: attr_10 = -2147483647 : i32
+    attr_10 = -2147483647 : i32,
+    // CHECK-SAME: attr_11 = 2147483646 : i32
+    attr_11 = 2147483646 : i32,
+    // CHECK-SAME: attr_12 = -2147483647 : si32
+    attr_12 = -2147483647 : si32,
+    // CHECK-SAME: attr_13 = 2147483646 : si32
+    attr_13 = 2147483646 : si32,
+    // CHECK-SAME: attr_14 = 4294967295 : ui32
+    attr_14 = 4294967295 : ui32,
+    // CHECK-SAME: attr_15 = -9223372036854775808 : i64
+    attr_15 = -9223372036854775808 : i64,
+    // CHECK-SAME: attr_16 = 9223372036854775807 : i64
+    attr_16 = 9223372036854775807 : i64,
+    // CHECK-SAME: attr_17 = -9223372036854775808 : si64
+    attr_17 = -9223372036854775808 : si64,
+    // CHECK-SAME: attr_18 = 9223372036854775807 : si64
+    attr_18 = 9223372036854775807 : si64,
+    // CHECK-SAME: attr_19 = 18446744073709551615 : ui64
+    attr_19 = 18446744073709551615 : ui64
+  } : () -> ()
+
+  return
+}
+
+// -----
+
+//===----------------------------------------------------------------------===//
+// Check that positive values larger than 2^n-1 for signless integers
+// are mapped to their negative signed counterpart. This behaviour is
+// undocumented in the language specification, but it is what the
+// parser currently does.
+//===----------------------------------------------------------------------===//
+
+func @int_attrs_pass() {
+  "test.i8_attr"() {
+    // CHECK: attr_00 = -1 : i8
+    attr_00 = 255 : i8,
+    // CHECK-SAME: attr_01 = -1 : i16
+    attr_01 = 65535 : i16,
+    // CHECK-SAME: attr_02 = -1 : i32
+    attr_02 = 4294967295 : i32,
+    // CHECK-SAME: attr_03 = -1 : i64
+    attr_03 = 18446744073709551615 : i64
+  } : () -> ()
+  return
+}
+// -----
+
+
 func @wrong_int_attrs_signedness_fail() {
   // expected-error @+1 {{'si32_attr' failed to satisfy constraint: 32-bit signed integer attribute}}
   "test.int_attrs"() {

mlir/test/IR/invalid.mlir

@@ -1231,3 +1231,243 @@ func @negative_value_in_unsigned_vector_attr() {
   // expected-error @+1 {{expected unsigned integer elements, but parsed negative value}}
   "foo"() {bar = dense<[5, -5]> : vector<2xui32>} : () -> ()
 }
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -129 : i8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 256 : i8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -129 : si8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 129 : si8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{negative integer literal not valid for unsigned integer type}}
+    attr = -1 : ui8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 256 : ui8
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -32769 : i16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 65536 : i16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -32769 : si16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 32768 : si16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{negative integer literal not valid for unsigned integer type}}
+    attr = -1 : ui16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 65536: ui16
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -2147483649 : i32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 4294967296 : i32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -2147483649 : si32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 2147483648 : si32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{negative integer literal not valid for unsigned integer type}}
+    attr = -1 : ui32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 4294967296 : ui32
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -9223372036854775809 : i64
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 18446744073709551616 : i64
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = -9223372036854775809 : si64
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 9223372036854775808 : si64
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{negative integer literal not valid for unsigned integer type}}
+    attr = -1 : ui64
+  } : () -> ()
+  return
+}
+
+// -----
+
+func @large_bound() {
+  "test.out_of_range_attribute"() {
+    // expected-error @+1 {{integer constant out of range for attribute}}
+    attr = 18446744073709551616 : ui64
+  } : () -> ()
+  return
+}