Skip to content
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

[SimplifyLibCall][Attribute] Fix bug where we may keep range attr with incompatible type #112649

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/include/llvm/IR/Argument.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update doc comment for AS parameter. In particular, it should be clear that the returned mask is not required to be a subset of AS. AS is effectively just a type hint.

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
Expand Down
16 changes: 14 additions & 2 deletions llvm/include/llvm/IR/InstrTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1453,14 +1453,26 @@ 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.
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're already here, please also drop these empty ///.

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
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
}

Expand Down
7 changes: 6 additions & 1 deletion llvm/lib/IR/Attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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()) {
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/IR/AutoUpgrade.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/IR/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -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();
Expand All @@ -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));
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Utils/CloneFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Utils/InlineFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
12 changes: 12 additions & 0 deletions llvm/test/Transforms/InstCombine/simplify-libcalls.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
4 changes: 2 additions & 2 deletions llvm/test/Verifier/range-attr.ll
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading