From ed5a269ff10ee288c1047495d8b2eecd15b9109f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 25 Jul 2021 18:21:13 +0200 Subject: [PATCH] [Attributes] Clean up handling of UB implying attributes (NFC) Rather than adding methods for dropping these attributes in various places, add a function that returns an AttrBuilder with these attributes, which can then be used with existing methods for dropping attributes. This is with an eye on D104641, which also needs to drop them from returns, not just parameters. Also be more explicit about the semantics of the method in the documentation. Refer to UB rather than Undef, which is what this is actually about. --- include/llvm/IR/Attributes.h | 13 +++++++------ include/llvm/IR/InstrTypes.h | 9 --------- lib/IR/Attributes.cpp | 18 ++++++++---------- lib/IR/Function.cpp | 6 ------ lib/Transforms/IPO/DeadArgumentElimination.cpp | 5 +++-- 5 files changed, 18 insertions(+), 33 deletions(-) diff --git a/include/llvm/IR/Attributes.h b/include/llvm/IR/Attributes.h index 6033efd978f..d7bd3edb3d4 100644 --- a/include/llvm/IR/Attributes.h +++ b/include/llvm/IR/Attributes.h @@ -546,12 +546,6 @@ public: return removeAttributes(C, ArgNo + FirstArgIndex, AttrsToRemove); } - /// Remove noundef attribute and other attributes that imply undefined - /// behavior if a `undef` or `poison` value is passed from this attribute - /// list. Returns a new list because attribute lists are immutable. - LLVM_NODISCARD AttributeList - removeParamUndefImplyingAttributes(LLVMContext &C, unsigned ArgNo) const; - /// Remove all attributes at the specified arg index from this /// attribute list. Returns a new list because attribute lists are immutable. LLVM_NODISCARD AttributeList removeParamAttributes(LLVMContext &C, @@ -1038,6 +1032,13 @@ namespace AttributeFuncs { /// Which attributes cannot be applied to a type. AttrBuilder typeIncompatible(Type *Ty); +/// Get param/return attributes which imply immediate undefined behavior if an +/// invalid value is passed. For example, this includes noundef (where undef +/// implies UB), but not nonnull (where null implies poison). It also does not +/// include attributes like nocapture, which constrain the function +/// implementation rather than the passed value. +AttrBuilder getUBImplyingAttributes(); + /// \returns Return true if the two functions have compatible target-independent /// attributes for inlining purposes. bool areInlineCompatible(const Function &Caller, const Function &Callee); diff --git a/include/llvm/IR/InstrTypes.h b/include/llvm/IR/InstrTypes.h index 2f31db0fa4d..ef2c279ed45 100644 --- a/include/llvm/IR/InstrTypes.h +++ b/include/llvm/IR/InstrTypes.h @@ -1558,15 +1558,6 @@ public: setAttributes(PAL); } - /// Removes noundef and other attributes that imply undefined behavior if a - /// `undef` or `poison` value is passed from the given argument. - void removeParamUndefImplyingAttrs(unsigned ArgNo) { - assert(ArgNo < getNumArgOperands() && "Out of bounds"); - AttributeList PAL = getAttributes(); - PAL = PAL.removeParamUndefImplyingAttributes(getContext(), ArgNo); - setAttributes(PAL); - } - /// adds the dereferenceable attribute to the list of attributes. void addDereferenceableAttr(unsigned i, uint64_t Bytes) { AttributeList PAL = getAttributes(); diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp index 92721c74317..5cd1bafccc4 100644 --- a/lib/IR/Attributes.cpp +++ b/lib/IR/Attributes.cpp @@ -1335,16 +1335,6 @@ AttributeList AttributeList::removeAttributes(LLVMContext &C, return getImpl(C, AttrSets); } -AttributeList -AttributeList::removeParamUndefImplyingAttributes(LLVMContext &C, - unsigned ArgNo) const { - AttrBuilder B; - B.addAttribute(Attribute::NoUndef); - B.addDereferenceableAttr(1); - B.addDereferenceableOrNullAttr(1); - return removeParamAttributes(C, ArgNo, B); -} - AttributeList AttributeList::addDereferenceableAttr(LLVMContext &C, unsigned Index, uint64_t Bytes) const { @@ -1926,6 +1916,14 @@ AttrBuilder AttributeFuncs::typeIncompatible(Type *Ty) { return Incompatible; } +AttrBuilder AttributeFuncs::getUBImplyingAttributes() { + AttrBuilder B; + B.addAttribute(Attribute::NoUndef); + B.addDereferenceableAttr(1); + B.addDereferenceableOrNullAttr(1); + return B; +} + template static bool isEqual(const Function &Caller, const Function &Callee) { return Caller.getFnAttribute(AttrClass::getKind()) == diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index 4f4a8dbbd98..4034b1505bd 100644 --- a/lib/IR/Function.cpp +++ b/lib/IR/Function.cpp @@ -601,12 +601,6 @@ void Function::removeParamAttrs(unsigned ArgNo, const AttrBuilder &Attrs) { setAttributes(PAL); } -void Function::removeParamUndefImplyingAttrs(unsigned ArgNo) { - AttributeList PAL = getAttributes(); - PAL = PAL.removeParamUndefImplyingAttributes(getContext(), ArgNo); - setAttributes(PAL); -} - void Function::addDereferenceableAttr(unsigned i, uint64_t Bytes) { AttributeList PAL = getAttributes(); PAL = PAL.addDereferenceableAttr(getContext(), i, Bytes); diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp index 3154d2468c6..d95fd55870f 100644 --- a/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -287,6 +287,7 @@ bool DeadArgumentEliminationPass::RemoveDeadArgumentsFromCallers(Function &Fn) { SmallVector UnusedArgs; bool Changed = false; + AttrBuilder UBImplyingAttributes = AttributeFuncs::getUBImplyingAttributes(); for (Argument &Arg : Fn.args()) { if (!Arg.hasSwiftErrorAttr() && Arg.use_empty() && !Arg.hasPassPointeeByValueCopyAttr()) { @@ -295,7 +296,7 @@ bool DeadArgumentEliminationPass::RemoveDeadArgumentsFromCallers(Function &Fn) { Changed = true; } UnusedArgs.push_back(Arg.getArgNo()); - Fn.removeParamUndefImplyingAttrs(Arg.getArgNo()); + Fn.removeParamAttrs(Arg.getArgNo(), UBImplyingAttributes); } } @@ -313,7 +314,7 @@ bool DeadArgumentEliminationPass::RemoveDeadArgumentsFromCallers(Function &Fn) { Value *Arg = CB->getArgOperand(ArgNo); CB->setArgOperand(ArgNo, UndefValue::get(Arg->getType())); - CB->removeParamUndefImplyingAttrs(ArgNo); + CB->removeParamAttrs(ArgNo, UBImplyingAttributes); ++NumArgumentsReplacedWithUndef; Changed = true;