-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[IR] Add ImplicitTrunc argument to ConstantInt::get() #170865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add an ImplicitTrunc argument to ConstantInt::get(), which allows controlling whether implicit truncation of the value is permitted. This argument currently defaults to true, but will be switched to false in the future to guard against signed/unsigned confusion, similar to what has already happened for APInt. The argument gives an opt-out for cases where the truncation is intended. The patch contains one illustrative example where this happens.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) ChangesAdd an ImplicitTrunc argument to ConstantInt::get(), which allows controlling whether implicit truncation of the value is permitted. This argument currently defaults to true, but will be switched to false in the future to guard against signed/unsigned confusion, similar to what has already happened for APInt. The argument gives an opt-out for cases where the truncation is intended. The patch contains one illustrative example where this happens. Full diff: https://github.com/llvm/llvm-project/pull/170865.diff 3 Files Affected:
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index e3f2eb9fa44b8..d03c8851616df 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -112,16 +112,21 @@ class ConstantInt final : public ConstantData {
/// If Ty is a vector type, return a Constant with a splat of the given
/// value. Otherwise return a ConstantInt for the given value.
- LLVM_ABI static Constant *get(Type *Ty, uint64_t V, bool IsSigned = false);
+ /// \param ImplicitTrunc Whether to allow implicit truncation of the value.
+ // TODO: Make ImplicitTrunc default to false.
+ LLVM_ABI static Constant *get(Type *Ty, uint64_t V, bool IsSigned = false,
+ bool ImplicitTrunc = true);
/// Return a ConstantInt with the specified integer value for the specified
/// type. If the type is wider than 64 bits, the value will be zero-extended
/// to fit the type, unless IsSigned is true, in which case the value will
/// be interpreted as a 64-bit signed integer and sign-extended to fit
/// the type.
- /// Get a ConstantInt for a specific value.
+ /// \param ImplicitTrunc Whether to allow implicit truncation of the value.
+ // TODO: Make ImplicitTrunc default to false.
LLVM_ABI static ConstantInt *get(IntegerType *Ty, uint64_t V,
- bool IsSigned = false);
+ bool IsSigned = false,
+ bool ImplicitTrunc = true);
/// Return a ConstantInt with the specified value for the specified type. The
/// value V will be canonicalized to a an unsigned APInt. Accessing it with
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 6b82da140256f..2f020274c305a 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -960,8 +960,10 @@ ConstantInt *ConstantInt::get(LLVMContext &Context, ElementCount EC,
return Slot.get();
}
-Constant *ConstantInt::get(Type *Ty, uint64_t V, bool isSigned) {
- Constant *C = get(cast<IntegerType>(Ty->getScalarType()), V, isSigned);
+Constant *ConstantInt::get(Type *Ty, uint64_t V, bool IsSigned,
+ bool ImplicitTrunc) {
+ Constant *C =
+ get(cast<IntegerType>(Ty->getScalarType()), V, IsSigned, ImplicitTrunc);
// For vectors, broadcast the value.
if (VectorType *VTy = dyn_cast<VectorType>(Ty))
@@ -970,11 +972,10 @@ Constant *ConstantInt::get(Type *Ty, uint64_t V, bool isSigned) {
return C;
}
-ConstantInt *ConstantInt::get(IntegerType *Ty, uint64_t V, bool isSigned) {
- // TODO: Avoid implicit trunc?
- // See https://github.com/llvm/llvm-project/issues/112510.
+ConstantInt *ConstantInt::get(IntegerType *Ty, uint64_t V, bool IsSigned,
+ bool ImplicitTrunc) {
return get(Ty->getContext(),
- APInt(Ty->getBitWidth(), V, isSigned, /*implicitTrunc=*/true));
+ APInt(Ty->getBitWidth(), V, IsSigned, ImplicitTrunc));
}
Constant *ConstantInt::get(Type *Ty, const APInt& V) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 743c4f574e131..8e7282a4ffefe 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3811,7 +3811,9 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
if (VecToReduceCount.isFixed()) {
unsigned VectorSize = VecToReduceCount.getFixedValue();
return BinaryOperator::CreateMul(
- Splat, ConstantInt::get(Splat->getType(), VectorSize));
+ Splat,
+ ConstantInt::get(Splat->getType(), VectorSize, /*IsSigned=*/false,
+ /*ImplicitTrunc=*/true));
}
}
}
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
RKSimon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
topperc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Yingwei Zheng <dtcxzyw@qq.com>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/17790 Here is the relevant piece of the build log for the reference |
Add an ImplicitTrunc argument to ConstantInt::get(), which allows
controlling whether implicit truncation of the value is permitted.
This argument currently defaults to true, but will be switched to false
in the future to guard against signed/unsigned confusion, similar to
what has already happened for APInt.
The argument gives an opt-out for cases where the truncation is
intended. The patch contains one illustrative example where this
happens.
Add an ImplicitTrunc argument to ConstantInt::get(), which allows controlling whether implicit truncation of the value is permitted.
This argument currently defaults to true, but will be switched to false in the future to guard against signed/unsigned confusion, similar to what has already happened for APInt.
The argument gives an opt-out for cases where the truncation is intended. The patch contains one illustrative example where this happens.