[Mlir-commits] [mlir] e692af8 - [MLIR] Update APInt construction to correctly set isSigned/implicitTrunc (#110466)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Oct 14 06:01:08 PDT 2024
Author: Nikita Popov
Date: 2024-10-14T15:01:05+02:00
New Revision: e692af85966903614d470a7742ed89d124baf1a6
URL: https://github.com/llvm/llvm-project/commit/e692af85966903614d470a7742ed89d124baf1a6
DIFF: https://github.com/llvm/llvm-project/commit/e692af85966903614d470a7742ed89d124baf1a6.diff
LOG: [MLIR] Update APInt construction to correctly set isSigned/implicitTrunc (#110466)
This fixes all the places in MLIR that hit the new assertion added in
#106524, in preparation for enabling it by default. That is, cases where
the value passed to the APInt constructor is not an N-bit
signed/unsigned integer, where N is the bit width and signedness is
determined by the isSigned flag.
The fixes either set the correct value for isSigned, or set the
implicitTrunc flag to retain the old behavior. I've left TODOs for the
latter case in some places, where I think that it may be worthwhile to
stop doing implicit truncation in the future.
Note that the assertion is currently still disabled by default, so this
patch is mostly NFC.
This is just the MLIR changes split off from
https://github.com/llvm/llvm-project/pull/80309.
Added:
Modified:
mlir/include/mlir/IR/BuiltinAttributes.td
mlir/include/mlir/IR/OpImplementation.h
mlir/lib/Conversion/TosaToArith/TosaToArith.cpp
mlir/lib/Dialect/ControlFlow/IR/ControlFlowOps.cpp
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
mlir/lib/IR/Builders.cpp
mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
mlir/unittests/Dialect/SPIRV/SerializationTest.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/BuiltinAttributes.td b/mlir/include/mlir/IR/BuiltinAttributes.td
index f0d41754001400..530ba7d2f11e5c 100644
--- a/mlir/include/mlir/IR/BuiltinAttributes.td
+++ b/mlir/include/mlir/IR/BuiltinAttributes.td
@@ -701,8 +701,10 @@ def Builtin_IntegerAttr : Builtin_Attr<"Integer", "integer",
return $_get(type.getContext(), type, apValue);
}
+ // TODO: Avoid implicit trunc?
IntegerType intTy = ::llvm::cast<IntegerType>(type);
- APInt apValue(intTy.getWidth(), value, intTy.isSignedInteger());
+ APInt apValue(intTy.getWidth(), value, intTy.isSignedInteger(),
+ /*implicitTrunc=*/true);
return $_get(type.getContext(), type, apValue);
}]>
];
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index e2472eea8a3714..606a56c7fd55b5 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -749,7 +749,8 @@ class AsmParser {
// zero for non-negated integers.
result =
(IntT)uintResult.sextOrTrunc(sizeof(IntT) * CHAR_BIT).getLimitedValue();
- if (APInt(uintResult.getBitWidth(), result) != uintResult)
+ if (APInt(uintResult.getBitWidth(), result, /*isSigned=*/true,
+ /*implicitTrunc=*/true) != uintResult)
return emitError(loc, "integer value too large");
return success();
}
diff --git a/mlir/lib/Conversion/TosaToArith/TosaToArith.cpp b/mlir/lib/Conversion/TosaToArith/TosaToArith.cpp
index 50e57682a2dc8d..593dbaa6c6545a 100644
--- a/mlir/lib/Conversion/TosaToArith/TosaToArith.cpp
+++ b/mlir/lib/Conversion/TosaToArith/TosaToArith.cpp
@@ -43,7 +43,7 @@ Type matchContainerType(Type element, Type container) {
TypedAttr getConstantAttr(Type type, int64_t value, PatternRewriter &rewriter) {
if (auto shapedTy = dyn_cast<ShapedType>(type)) {
Type eTy = shapedTy.getElementType();
- APInt valueInt(eTy.getIntOrFloatBitWidth(), value);
+ APInt valueInt(eTy.getIntOrFloatBitWidth(), value, /*isSigned=*/true);
return DenseIntElementsAttr::get(shapedTy, valueInt);
}
diff --git a/mlir/lib/Dialect/ControlFlow/IR/ControlFlowOps.cpp b/mlir/lib/Dialect/ControlFlow/IR/ControlFlowOps.cpp
index 98b429de1fd85c..edd7f607f24f4d 100644
--- a/mlir/lib/Dialect/ControlFlow/IR/ControlFlowOps.cpp
+++ b/mlir/lib/Dialect/ControlFlow/IR/ControlFlowOps.cpp
@@ -528,7 +528,7 @@ static ParseResult parseSwitchOpCases(
int64_t value = 0;
if (failed(parser.parseInteger(value)))
return failure();
- values.push_back(APInt(bitWidth, value));
+ values.push_back(APInt(bitWidth, value, /*isSigned=*/true));
Block *destination;
SmallVector<OpAsmParser::UnresolvedOperand> operands;
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 55c4e03aaefe7b..12ed8cc88ae7b7 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -598,7 +598,7 @@ static ParseResult parseSwitchOpCases(
int64_t value = 0;
if (failed(parser.parseInteger(value)))
return failure();
- values.push_back(APInt(bitWidth, value));
+ values.push_back(APInt(bitWidth, value, /*isSigned=*/true));
Block *destination;
SmallVector<OpAsmParser::UnresolvedOperand> operands;
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index f552221bbdcaf9..f87a46a7e5995a 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1404,7 +1404,7 @@ static ParseResult parseMembersIndex(OpAsmParser &parser,
if (parser.parseInteger(value))
return failure();
shapeTmp++;
- values.push_back(APInt(32, value));
+ values.push_back(APInt(32, value, /*isSigned=*/true));
return success();
};
diff --git a/mlir/lib/IR/Builders.cpp b/mlir/lib/IR/Builders.cpp
index a9bc3c0ef65a23..5359432a04f3e5 100644
--- a/mlir/lib/IR/Builders.cpp
+++ b/mlir/lib/IR/Builders.cpp
@@ -238,7 +238,10 @@ DenseIntElementsAttr Builder::getIndexTensorAttr(ArrayRef<int64_t> values) {
}
IntegerAttr Builder::getI32IntegerAttr(int32_t value) {
- return IntegerAttr::get(getIntegerType(32), APInt(32, value));
+ // The APInt always uses isSigned=true here because we accept the value
+ // as int32_t.
+ return IntegerAttr::get(getIntegerType(32),
+ APInt(32, value, /*isSigned=*/true));
}
IntegerAttr Builder::getSI32IntegerAttr(int32_t value) {
@@ -256,14 +259,19 @@ IntegerAttr Builder::getI16IntegerAttr(int16_t value) {
}
IntegerAttr Builder::getI8IntegerAttr(int8_t value) {
- return IntegerAttr::get(getIntegerType(8), APInt(8, value));
+ // The APInt always uses isSigned=true here because we accept the value
+ // as int8_t.
+ return IntegerAttr::get(getIntegerType(8),
+ APInt(8, value, /*isSigned=*/true));
}
IntegerAttr Builder::getIntegerAttr(Type type, int64_t value) {
if (type.isIndex())
return IntegerAttr::get(type, APInt(64, value));
- return IntegerAttr::get(
- type, APInt(type.getIntOrFloatBitWidth(), value, type.isSignedInteger()));
+ // TODO: Avoid implicit trunc?
+ return IntegerAttr::get(type, APInt(type.getIntOrFloatBitWidth(), value,
+ type.isSignedInteger(),
+ /*implicitTrunc=*/true));
}
IntegerAttr Builder::getIntegerAttr(Type type, const APInt &value) {
diff --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 6c7fe41069824f..462d3e326b6c27 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -1287,7 +1287,8 @@ LogicalResult spirv::Deserializer::processConstant(ArrayRef<uint32_t> operands,
} words = {operands[2], operands[3]};
value = APInt(64, llvm::bit_cast<uint64_t>(words), /*isSigned=*/true);
} else if (bitwidth <= 32) {
- value = APInt(bitwidth, operands[2], /*isSigned=*/true);
+ value = APInt(bitwidth, operands[2], /*isSigned=*/true,
+ /*implicitTrunc=*/true);
}
auto attr = opBuilder.getIntegerAttr(intType, value);
diff --git a/mlir/unittests/Dialect/SPIRV/SerializationTest.cpp b/mlir/unittests/Dialect/SPIRV/SerializationTest.cpp
index 9d2f690ed898af..ef89c1645d373f 100644
--- a/mlir/unittests/Dialect/SPIRV/SerializationTest.cpp
+++ b/mlir/unittests/Dialect/SPIRV/SerializationTest.cpp
@@ -176,7 +176,7 @@ TEST_F(SerializationTest, SignlessVsSignedIntegerConstantBitExtension) {
IntegerType::get(&context, 16, IntegerType::Signless);
auto signedInt16Type = IntegerType::get(&context, 16, IntegerType::Signed);
// Check the bit extension of same value under
diff erent signedness semantics.
- APInt signlessIntConstVal(signlessInt16Type.getWidth(), -1,
+ APInt signlessIntConstVal(signlessInt16Type.getWidth(), 0xffff,
signlessInt16Type.getSignedness());
APInt signedIntConstVal(signedInt16Type.getWidth(), -1,
signedInt16Type.getSignedness());
More information about the Mlir-commits
mailing list