[llvm] [SimplifyLibCall][Attribute] Fix bug where we may keep `range` attr with incompatible type (PR #112649)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 16 19:36:49 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: None (goldsteinn)
<details>
<summary>Changes</summary>
In a variety of places we change the bitwidth of a parameter but don't
update the attributes.
The issue in this case is from the `range` attribute when inlining
`__memset_chk`. `optimizeMemSetChk` will replace an `i32` with an
`i8`, and if the `i32` had a `range` attr assosiated it will cause an
error.
Fixes #<!-- -->112633
---
Full diff: https://github.com/llvm/llvm-project/pull/112649.diff
18 Files Affected:
- (modified) llvm/include/llvm/IR/Argument.h (+2)
- (modified) llvm/include/llvm/IR/Attributes.h (+2-1)
- (modified) llvm/include/llvm/IR/InstrTypes.h (+12-2)
- (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+3-2)
- (modified) llvm/lib/IR/Attributes.cpp (+6-1)
- (modified) llvm/lib/IR/AutoUpgrade.cpp (+4-2)
- (modified) llvm/lib/IR/Function.cpp (+4)
- (modified) llvm/lib/IR/Verifier.cpp (+1-1)
- (modified) llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp (+2-1)
- (modified) llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp (+5-3)
- (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+7-4)
- (modified) llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp (+2-2)
- (modified) llvm/lib/Transforms/Utils/CallPromotionUtils.cpp (+4-2)
- (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+2-2)
- (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+2-2)
- (modified) llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp (+7-1)
- (modified) llvm/test/Transforms/InstCombine/simplify-libcalls.ll (+12)
- (modified) llvm/test/Verifier/range-attr.ll (+2-2)
``````````diff
diff --git a/llvm/include/llvm/IR/Argument.h b/llvm/include/llvm/IR/Argument.h
index 0ffcb05519d44f..5be58d7eca0606 100644
--- a/llvm/include/llvm/IR/Argument.h
+++ b/llvm/include/llvm/IR/Argument.h
@@ -182,6 +182,8 @@ class Argument final : public Value {
Attribute getAttribute(Attribute::AttrKind Kind) const;
+ AttributeSet getAttributes() const;
+
/// Method for support type inquiry through isa, cast, and dyn_cast.
static bool classof(const Value *V) {
return V->getValueID() == ArgumentVal;
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index 57db52e4879b5b..63534b5ec59cf1 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -1292,7 +1292,8 @@ bool isNoFPClassCompatibleType(Type *Ty);
/// if only attributes that are known to be safely droppable are contained in
/// the mask; only attributes that might be unsafe to drop (e.g., ABI-related
/// attributes) are in the mask; or both.
-AttributeMask typeIncompatible(Type *Ty, AttributeSafetyKind ASK = ASK_ALL);
+AttributeMask typeIncompatible(Type *Ty, AttributeSet AS,
+ AttributeSafetyKind ASK = ASK_ALL);
/// Get param/return attributes which imply immediate undefined behavior if an
/// invalid value is passed. For example, this includes noundef (where undef
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index 86d88da3d9460e..54b33ec03d512d 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -1453,14 +1453,24 @@ class CallBase : public Instruction {
/// looking through to the attributes on the called function when necessary).
///@{
- /// Return the parameter attributes for this call.
+ /// Return the attributes for this call.
///
AttributeList getAttributes() const { return Attrs; }
- /// Set the parameter attributes for this call.
+ /// Set the attributes for this call.
///
void setAttributes(AttributeList A) { Attrs = A; }
+ /// Return the return attributes for this call.
+ ///
+ AttributeSet getRetAttributes() const { return getAttributes().getRetAttrs(); }
+
+ /// Return the param attributes for this call.
+ ///
+ AttributeSet getParamAttributes(unsigned ArgNo) const {
+ return getAttributes().getParamAttrs(ArgNo);
+ }
+
/// Try to intersect the attributes from 'this' CallBase and the
/// 'Other' CallBase. Sets the intersected attributes to 'this' and
/// return true if successful. Doesn't modify 'this' and returns
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index e09532dc4735fb..6d615c04c04b5f 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7039,11 +7039,12 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
// Remove incompatible attributes on function calls.
if (auto *CI = dyn_cast<CallBase>(&I)) {
CI->removeRetAttrs(AttributeFuncs::typeIncompatible(
- CI->getFunctionType()->getReturnType()));
+ CI->getFunctionType()->getReturnType(), CI->getRetAttributes()));
for (unsigned ArgNo = 0; ArgNo < CI->arg_size(); ++ArgNo)
CI->removeParamAttrs(ArgNo, AttributeFuncs::typeIncompatible(
- CI->getArgOperand(ArgNo)->getType()));
+ CI->getArgOperand(ArgNo)->getType(),
+ CI->getParamAttributes(ArgNo)));
}
}
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index c2fba49692c774..223c917766a457 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -2300,7 +2300,7 @@ bool AttributeFuncs::isNoFPClassCompatibleType(Type *Ty) {
}
/// Which attributes cannot be applied to a type.
-AttributeMask AttributeFuncs::typeIncompatible(Type *Ty,
+AttributeMask AttributeFuncs::typeIncompatible(Type *Ty, AttributeSet AS,
AttributeSafetyKind ASK) {
AttributeMask Incompatible;
@@ -2316,6 +2316,11 @@ AttributeMask AttributeFuncs::typeIncompatible(Type *Ty,
// Attributes that only apply to integers or vector of integers.
if (ASK & ASK_SAFE_TO_DROP)
Incompatible.addAttribute(Attribute::Range);
+ } else {
+ Attribute RangeAttr = AS.getAttribute(Attribute::Range);
+ if (RangeAttr.isValid() &&
+ RangeAttr.getRange().getBitWidth() != Ty->getScalarSizeInBits())
+ Incompatible.addAttribute(Attribute::Range);
}
if (!Ty->isPointerTy()) {
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 519ff8d74c5af4..fbbc6c05ad3352 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5396,9 +5396,11 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
}
// Remove all incompatibile attributes from function.
- F.removeRetAttrs(AttributeFuncs::typeIncompatible(F.getReturnType()));
+ F.removeRetAttrs(AttributeFuncs::typeIncompatible(
+ F.getReturnType(), F.getAttributes().getRetAttrs()));
for (auto &Arg : F.args())
- Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
+ Arg.removeAttrs(
+ AttributeFuncs::typeIncompatible(Arg.getType(), Arg.getAttributes()));
// Older versions of LLVM treated an "implicit-section-name" attribute
// similarly to directly setting the section on a Function.
diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp
index 09b90713b9c793..889295956dbf7c 100644
--- a/llvm/lib/IR/Function.cpp
+++ b/llvm/lib/IR/Function.cpp
@@ -359,6 +359,10 @@ Attribute Argument::getAttribute(Attribute::AttrKind Kind) const {
return getParent()->getParamAttribute(getArgNo(), Kind);
}
+AttributeSet Argument::getAttributes() const {
+ return getParent()->getAttributes().getParamAttrs(getArgNo());
+}
+
//===----------------------------------------------------------------------===//
// Helper Methods in Function
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 0412b93798b94d..f34fe7594c8602 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -2012,7 +2012,7 @@ void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
Attrs.hasAttribute(Attribute::ReadOnly)),
"Attributes writable and readonly are incompatible!", V);
- AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(Ty);
+ AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(Ty, Attrs);
for (Attribute Attr : Attrs) {
if (!Attr.isStringAttribute() &&
IncompatibleAttrs.contains(Attr.getKindAsEnum())) {
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp b/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
index eb553ae4eb80ff..26d8ce77d9a9b2 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
@@ -788,7 +788,8 @@ bool AMDGPULibCalls::fold(CallInst *CI) {
B.CreateFPToSI(FPOp->getOperand(1), PownType->getParamType(1));
// Have to drop any nofpclass attributes on the original call site.
Call->removeParamAttrs(
- 1, AttributeFuncs::typeIncompatible(CastedArg->getType()));
+ 1, AttributeFuncs::typeIncompatible(CastedArg->getType(),
+ Call->getParamAttributes(1)));
Call->setCalledFunction(PownFunc);
Call->setArgOperand(1, CastedArg);
return fold_pow(FPOp, B, PownInfo) || true;
diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index d1548592b1ce26..ed93b4491c50e4 100644
--- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -857,9 +857,10 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) {
// here. Currently, this should not be possible, but special handling might be
// required when new return value attributes are added.
if (NRetTy->isVoidTy())
- RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
+ RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy, PAL.getRetAttrs()));
else
- assert(!RAttrs.overlaps(AttributeFuncs::typeIncompatible(NRetTy)) &&
+ assert(!RAttrs.overlaps(
+ AttributeFuncs::typeIncompatible(NRetTy, PAL.getRetAttrs())) &&
"Return attributes no longer compatible?");
AttributeSet RetAttrs = AttributeSet::get(F->getContext(), RAttrs);
@@ -903,7 +904,8 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) {
// Adjust the call return attributes in case the function was changed to
// return void.
AttrBuilder RAttrs(F->getContext(), CallPAL.getRetAttrs());
- RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy));
+ RAttrs.remove(
+ AttributeFuncs::typeIncompatible(NRetTy, CallPAL.getRetAttrs()));
AttributeSet RetAttrs = AttributeSet::get(F->getContext(), RAttrs);
// Declare these outside of the loops, so we can reuse them for the second
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 3cc50ee6e233ae..e6fd222ca47609 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -4154,7 +4154,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
if (!CallerPAL.isEmpty() && !Caller->use_empty()) {
AttrBuilder RAttrs(FT->getContext(), CallerPAL.getRetAttrs());
- if (RAttrs.overlaps(AttributeFuncs::typeIncompatible(NewRetTy)))
+ if (RAttrs.overlaps(AttributeFuncs::typeIncompatible(
+ NewRetTy, CallerPAL.getRetAttrs())))
return false; // Attribute not compatible with transformed value.
}
@@ -4200,7 +4201,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
// Check if there are any incompatible attributes we cannot drop safely.
if (AttrBuilder(FT->getContext(), CallerPAL.getParamAttrs(i))
.overlaps(AttributeFuncs::typeIncompatible(
- ParamTy, AttributeFuncs::ASK_UNSAFE_TO_DROP)))
+ ParamTy, CallerPAL.getParamAttrs(i),
+ AttributeFuncs::ASK_UNSAFE_TO_DROP)))
return false; // Attribute not compatible with transformed value.
if (Call.isInAllocaArgument(i) ||
@@ -4238,7 +4240,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
// If the return value is not being used, the type may not be compatible
// with the existing attributes. Wipe out any problematic attributes.
- RAttrs.remove(AttributeFuncs::typeIncompatible(NewRetTy));
+ RAttrs.remove(
+ AttributeFuncs::typeIncompatible(NewRetTy, CallerPAL.getRetAttrs()));
LLVMContext &Ctx = Call.getContext();
AI = Call.arg_begin();
@@ -4253,7 +4256,7 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
// Add any parameter attributes except the ones incompatible with the new
// type. Note that we made sure all incompatible ones are safe to drop.
AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(
- ParamTy, AttributeFuncs::ASK_SAFE_TO_DROP);
+ ParamTy, CallerPAL.getParamAttrs(i), AttributeFuncs::ASK_SAFE_TO_DROP);
ArgAttrs.push_back(
CallerPAL.getParamAttrs(i).removeAttributes(Ctx, IncompatibleAttrs));
}
diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
index 577647cac3f58c..e226727e64d350 100644
--- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -1305,8 +1305,8 @@ DataFlowSanitizer::buildWrapperFunction(Function *F, StringRef NewFName,
Function *NewF = Function::Create(NewFT, NewFLink, F->getAddressSpace(),
NewFName, F->getParent());
NewF->copyAttributesFrom(F);
- NewF->removeRetAttrs(
- AttributeFuncs::typeIncompatible(NewFT->getReturnType()));
+ NewF->removeRetAttrs(AttributeFuncs::typeIncompatible(
+ NewFT->getReturnType(), NewF->getAttributes().getRetAttrs()));
BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF);
if (F->isVarArg()) {
diff --git a/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp b/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
index 4ead5cdcf29c01..17cba2e642a19a 100644
--- a/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
@@ -529,7 +529,8 @@ CallBase &llvm::promoteCall(CallBase &CB, Function *Callee,
// Remove any incompatible attributes for the argument.
AttrBuilder ArgAttrs(Ctx, CallerPAL.getParamAttrs(ArgNo));
- ArgAttrs.remove(AttributeFuncs::typeIncompatible(FormalTy));
+ ArgAttrs.remove(AttributeFuncs::typeIncompatible(
+ FormalTy, CallerPAL.getParamAttrs(ArgNo)));
// We may have a different byval/inalloca type.
if (ArgAttrs.getByValType())
@@ -549,7 +550,8 @@ CallBase &llvm::promoteCall(CallBase &CB, Function *Callee,
AttrBuilder RAttrs(Ctx, CallerPAL.getRetAttrs());
if (!CallSiteRetTy->isVoidTy() && CallSiteRetTy != CalleeRetTy) {
createRetBitCast(CB, CallSiteRetTy, RetBitCast);
- RAttrs.remove(AttributeFuncs::typeIncompatible(CalleeRetTy));
+ RAttrs.remove(
+ AttributeFuncs::typeIncompatible(CalleeRetTy, CallerPAL.getRetAttrs()));
AttributeChanged = true;
}
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index c6ba85bd9e57d4..5dc82a8dfb2dbe 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -819,9 +819,9 @@ void llvm::CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
// Drop all incompatible return attributes that cannot be applied to NewFunc
// during cloning, so as to allow instruction simplification to reason on the
// old state of the function. The original attributes are restored later.
- AttributeMask IncompatibleAttrs =
- AttributeFuncs::typeIncompatible(OldFunc->getReturnType());
AttributeList Attrs = NewFunc->getAttributes();
+ AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(
+ OldFunc->getReturnType(), Attrs.getRetAttrs());
NewFunc->removeRetAttrs(IncompatibleAttrs);
// As phi-nodes have been now remapped, allow incremental simplification of
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 55ad2b6d620003..7b353a415f49f5 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -3062,8 +3062,8 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
else
Builder.CreateRet(NewDeoptCall);
// Since the ret type is changed, remove the incompatible attributes.
- NewDeoptCall->removeRetAttrs(
- AttributeFuncs::typeIncompatible(NewDeoptCall->getType()));
+ NewDeoptCall->removeRetAttrs(AttributeFuncs::typeIncompatible(
+ NewDeoptCall->getType(), NewDeoptCall->getRetAttributes()));
}
// Leave behind the normal returns so we can merge control flow.
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index db2acb9eed0938..465bd72681232f 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -342,7 +342,13 @@ static Value *copyFlags(const CallInst &Old, Value *New) {
static Value *mergeAttributesAndFlags(CallInst *NewCI, const CallInst &Old) {
NewCI->setAttributes(AttributeList::get(
NewCI->getContext(), {NewCI->getAttributes(), Old.getAttributes()}));
- NewCI->removeRetAttrs(AttributeFuncs::typeIncompatible(NewCI->getType()));
+ NewCI->removeRetAttrs(AttributeFuncs::typeIncompatible(
+ NewCI->getType(), NewCI->getRetAttributes()));
+ for (unsigned I = 0; I < NewCI->arg_size(); ++I)
+ NewCI->removeParamAttrs(
+ I, AttributeFuncs::typeIncompatible(NewCI->getArgOperand(I)->getType(),
+ NewCI->getParamAttributes(I)));
+
return copyFlags(Old, NewCI);
}
diff --git a/llvm/test/Transforms/InstCombine/simplify-libcalls.ll b/llvm/test/Transforms/InstCombine/simplify-libcalls.ll
index bb2728a103ec6c..c4ae7e7e16bcf4 100644
--- a/llvm/test/Transforms/InstCombine/simplify-libcalls.ll
+++ b/llvm/test/Transforms/InstCombine/simplify-libcalls.ll
@@ -342,5 +342,17 @@ define signext i32 @emit_stpncpy() {
ret i32 0
}
+define void @simplify_memset_chk_pr112633(ptr %p, i32 %conv) {
+; CHECK-LABEL: @simplify_memset_chk_pr112633(
+; CHECK-NEXT: [[CALL_I:%.*]] = tail call ptr @__memset_chk(ptr [[P:%.*]], i32 range(i32 0, 123) [[CONV:%.*]], i64 1, i64 1)
+; CHECK-NEXT: ret void
+;
+ %call.i = tail call ptr @__memset_chk(ptr %p, i32 range(i32 0, 123) %conv, i64 1, i64 1)
+ ret void
+}
+
+declare ptr @__memset_chk(ptr, i32, i64, i64)
+
+
attributes #0 = { nobuiltin }
attributes #1 = { builtin }
diff --git a/llvm/test/Verifier/range-attr.ll b/llvm/test/Verifier/range-attr.ll
index f985ab696eacba..91412369b0b20d 100644
--- a/llvm/test/Verifier/range-attr.ll
+++ b/llvm/test/Verifier/range-attr.ll
@@ -1,12 +1,12 @@
; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
-; CHECK: Range bit width must match type bit width!
+; CHECK: Attribute 'range(i8 1, 0)' applied to incompatible type!
; CHECK-NEXT: ptr @bit_widths_do_not_match
define void @bit_widths_do_not_match(i32 range(i8 1, 0) %a) {
ret void
}
-; CHECK: Range bit width must match type bit width!
+; CHECK: Attribute 'range(i8 1, 0)' applied to incompatible type!
; CHECK-NEXT: ptr @bit_widths_do_not_match_vector
define void @bit_widths_do_not_match_vector(<4 x i32> range(i8 1, 0) %a) {
ret void
``````````
</details>
https://github.com/llvm/llvm-project/pull/112649
More information about the llvm-commits
mailing list