Index: mlir/lib/Parser/Parser.cpp =================================================================== --- mlir/lib/Parser/Parser.cpp +++ mlir/lib/Parser/Parser.cpp @@ -1809,19 +1809,31 @@ return nullptr; } - // Parse the integer literal. + // 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, *val, isNegative); if (apInt != *val) 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) + if (isNegative) + apInt.negate(); + + // If the value is negative, we have an overflow if the sign bit is not set in + // the negated apInt. + if (isNegative && !apInt.isSignBitSet()) + return emitError(loc, "integer constant out of range for attribute"), + nullptr; + + // If the value is positive and we have a signed integer or an index, we have + // an overflow if the sign bit is set. + if (!isNegative && (type.isSignedInteger() || type.isIndex()) && + apInt.isSignBitSet()) 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 Index: mlir/test/Dialect/SPIRV/canonicalize.mlir =================================================================== --- mlir/test/Dialect/SPIRV/canonicalize.mlir +++ mlir/test/Dialect/SPIRV/canonicalize.mlir @@ -273,7 +273,7 @@ %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 @@ %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 Index: mlir/test/IR/attribute.mlir =================================================================== --- mlir/test/IR/attribute.mlir +++ mlir/test/IR/attribute.mlir @@ -33,6 +33,151 @@ // ----- +//===----------------------------------------------------------------------===// +// Check that the maximum and minumum integer attribute values are +// representable and preserved during a round-trip. +//===----------------------------------------------------------------------===// + +func @int_attrs_pass() { + "test.i8_attr"() { + // CHECK: attr = -128 : i8 + attr = -128 : i8 + } : () -> () + + "test.i8_attr"() { + // CHECK: attr = 127 : i8 + attr = 127 : i8 + } : () -> () + + "test.si8_attr"() { + // CHECK: attr = -128 : si8 + attr = -128 : si8 + } : () -> () + + "test.si8_attr"() { + // CHECK: attr = 127 : si8 + attr = 127 : si8 + } : () -> () + + "test.ui8_attr"() { + // CHECK: attr = 255 : ui8 + attr = 255 : ui8 + } : () -> () + + "test.i16_attr"() { + // CHECK: attr = -32768 : i16 + attr = -32768 : i16 + } : () -> () + + "test.i16_attr"() { + // CHECK: attr = 32767 : i16 + attr = 32767 : i16 + } : () -> () + + "test.si16_attr"() { + // CHECK: attr = -32768 : si16 + attr = -32768 : si16 + } : () -> () + + "test.si16_attr"() { + // CHECK: attr = 32767 : si16 + attr = 32767 : si16 + } : () -> () + + "test.ui16_attr"() { + // CHECK: attr = 65535 : ui16 + attr = 65535 : ui16 + } : () -> () + + "test.i32_attr"() { + // CHECK: attr = -2147483647 : i32 + attr = -2147483647 : i32 + } : () -> () + + "test.i32_attr"() { + // CHECK: attr = 2147483646 : i32 + attr = 2147483646 : i32 + } : () -> () + + "test.si32_attr"() { + // CHECK: attr = -2147483647 : si32 + attr = -2147483647 : si32 + } : () -> () + + "test.si32_attr"() { + // CHECK: attr = 2147483646 : si32 + attr = 2147483646 : si32 + } : () -> () + + "test.ui32_attr"() { + // CHECK: attr = 4294967295 : ui32 + attr = 4294967295 : ui32 + } : () -> () + + "test.i64_attr"() { + // CHECK: attr = -9223372036854775808 : i64 + attr = -9223372036854775808 : i64 + } : () -> () + + "test.i64_attr"() { + // CHECK: attr = 9223372036854775807 : i64 + attr = 9223372036854775807 : i64 + } : () -> () + + "test.si64_attr"() { + // CHECK: attr = -9223372036854775808 : si64 + attr = -9223372036854775808 : si64 + } : () -> () + + "test.si64_attr"() { + // CHECK: attr = 9223372036854775807 : si64 + attr = 9223372036854775807 : si64 + } : () -> () + + "test.ui64_attr"() { + // CHECK: attr = 18446744073709551615 : ui64 + attr = 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 = -1 : i8 + attr = 255 : i8 + } : () -> () + + "test.i16_attr"() { + // CHECK: attr = -1 : i16 + attr = 65535 : i16 + } : () -> () + + "test.i32_attr"() { + // CHECK: attr = -1 : i32 + attr = 4294967295 : i32 + } : () -> () + + "test.i64_attr"() { + // CHECK: attr = -1 : i64 + attr = 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"() { Index: mlir/test/lib/TestDialect/TestOps.td =================================================================== --- mlir/test/lib/TestDialect/TestOps.td +++ mlir/test/lib/TestDialect/TestOps.td @@ -211,6 +211,54 @@ ); } +def I8AttrValueOp : TEST_Op<"i8_attr"> { + let arguments = (ins AnyI8Attr:$attr); +} + +def SI8AttrValueOp : TEST_Op<"si8_attr"> { + let arguments = (ins SI8Attr:$attr); +} + +def UI8AttrValueOp : TEST_Op<"ui8_attr"> { + let arguments = (ins UI8Attr:$attr); +} + +def I16AttrValueOp : TEST_Op<"i16_attr"> { + let arguments = (ins AnyI16Attr:$attr); +} + +def SI16AttrValueOp : TEST_Op<"si16_attr"> { + let arguments = (ins SI16Attr:$attr); +} + +def UI16AttrValueOp : TEST_Op<"ui16_attr"> { + let arguments = (ins UI16Attr:$attr); +} + +def I32AttrValueOp : TEST_Op<"i32_attr"> { + let arguments = (ins AnyI32Attr:$attr); +} + +def SI32AttrValueOp : TEST_Op<"si32_attr"> { + let arguments = (ins SI32Attr:$attr); +} + +def UI32AttrValueOp : TEST_Op<"ui32_attr"> { + let arguments = (ins UI32Attr:$attr); +} + +def I64AttrValueOp : TEST_Op<"i64_attr"> { + let arguments = (ins AnyI64Attr:$attr); +} + +def SI64AttrValueOp : TEST_Op<"si64_attr"> { + let arguments = (ins SI64Attr:$attr); +} + +def UI64AttrValueOp : TEST_Op<"ui64_attr"> { + let arguments = (ins UI64Attr:$attr); +} + def FloatElementsAttrOp : TEST_Op<"float_elements_attr"> { let arguments = (ins RankedF32ElementsAttr<[2]>:$scalar_f32_attr,