From 92ba0be0e8a18cc24314669430ea546fd7960c51 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Mon, 23 Sep 2024 15:56:06 -0500 Subject: [PATCH 01/10] [IR][Attribute] Add support for intersecting AttributeLists; NFC Add support for taking the intersection of two AttributeLists s.t the result list contains attributes that are valid in the context of both inputs. i.e if we have `nonnull align(32) noundef` intersected with `nonnull align(16) dereferenceable(10)`, the result is `nonnull align(16)`. Further it handles attributes that are not-droppable. For example dropping `byval` can change the nature of a callsite/function so its impossible to correct a correct intersection if its dropped from the result. i.e `nonnull byval(i64)` intersected with `nonnull` is invalid. The motivation for the infrastructure is to enable sinking/hoisting callsites with differing attributes. --- llvm/include/llvm/IR/Attributes.h | 25 ++- llvm/include/llvm/IR/Attributes.td | 93 ++++++---- llvm/lib/IR/Attributes.cpp | 130 +++++++++++++ llvm/unittests/IR/AttributesTest.cpp | 268 +++++++++++++++++++++++++++ llvm/utils/TableGen/Attributes.cpp | 14 +- 5 files changed, 497 insertions(+), 33 deletions(-) diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h index 5a80a072dbbd2..ac0b00e2ccddb 100644 --- a/llvm/include/llvm/IR/Attributes.h +++ b/llvm/include/llvm/IR/Attributes.h @@ -117,6 +117,10 @@ class Attribute { static bool canUseAsParamAttr(AttrKind Kind); static bool canUseAsRetAttr(AttrKind Kind); + static bool intersectWithAnd(AttrKind Kind); + static bool intersectWithMin(AttrKind Kind); + static bool intersectWithCustom(AttrKind Kind); + private: AttributeImpl *pImpl = nullptr; @@ -208,8 +212,15 @@ class Attribute { /// Return true if the target-dependent attribute is present. bool hasAttribute(StringRef Val) const; + /// Returns true if the attribute's kind can be represented as an enum (Enum, + /// Integer, Type, ConstantRange, or ConstantRangeList attribute). + bool hasKindAsEnum() const { + return isEnumAttribute() || isIntAttribute() || isTypeAttribute() || + isConstantRangeAttribute() || isConstantRangeListAttribute(); + } + /// Return the attribute's kind as an enum (Attribute::AttrKind). This - /// requires the attribute to be an enum, integer, or type attribute. + /// requires the attribute be representable as an enum (see: `hasKindAsEnum`). Attribute::AttrKind getKindAsEnum() const; /// Return the attribute's value as an integer. This requires that the @@ -383,6 +394,12 @@ class AttributeSet { [[nodiscard]] AttributeSet removeAttributes(LLVMContext &C, const AttributeMask &AttrsToRemove) const; + /// Try to intersect this AttributeSet with Other. Returns std::nullopt if + /// the two lists are inherently incompatible (imply different behavior, not + /// just analysis). + [[nodiscard]] std::optional + intersectWith(LLVMContext &C, AttributeSet Other) const; + /// Return the number of attributes in this set. unsigned getNumAttributes() const; @@ -775,6 +792,12 @@ class AttributeList { addAllocSizeParamAttr(LLVMContext &C, unsigned ArgNo, unsigned ElemSizeArg, const std::optional &NumElemsArg); + /// Try to intersect this AttributeList with Other. Returns std::nullopt if + /// the two lists are inherently incompatible (imply different behavior, not + /// just analysis). + [[nodiscard]] std::optional + intersectAttributes(LLVMContext &C, AttributeList Other) const; + //===--------------------------------------------------------------------===// // AttributeList Accessors //===--------------------------------------------------------------------===// diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td index 9044d127b4794..ca24627912f27 100644 --- a/llvm/include/llvm/IR/Attributes.td +++ b/llvm/include/llvm/IR/Attributes.td @@ -22,6 +22,37 @@ def ParamAttr : AttrProperty; /// Can be used as return attribute. def RetAttr : AttrProperty; + + +/// Intersection rules. Used for example in sinking/hoisting two +/// callbases to find a set of attributes that apply to both. +/// If no rule is specified, the assumed rule is that the attrs +/// must be equal, otherwise the intersection will fail. Having +/// any rule implies that it is legal to drop the attribute. +/// Note, there are some attributes we can (probably) legally drop +/// but are intentionally excluded as of now. Those include: +/// - initializes +/// - allockind +/// - allocsize +/// - minsize +/// - optsize +/// - optnone +/// - optdebug +/// - optforfuzzing +/// +/// When intersecting take the AND of the two attrs. +/// Invalid for Int/ConstantRange/ConstantRangeList attrs. +def IntersectAnd : AttrProperty; + +/// When intersecting take the min value of the two attrs. +/// Only valid for Int attrs. +def IntersectMin : AttrProperty; + +/// When intersecting rely on some user defined code. +def IntersectCustom : AttrProperty; + + + /// Attribute base class. class Attr P> { // String representation of this attribute in the IR. @@ -54,17 +85,17 @@ class ConstantRangeListAttr P> : Attr; /// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias. /// 0 means unaligned (different from align(1)). -def Alignment : IntAttr<"align", [ParamAttr, RetAttr]>; +def Alignment : IntAttr<"align", [ParamAttr, RetAttr, IntersectCustom]>; /// Parameter of a function that tells us the alignment of an allocation, as in /// aligned_alloc and aligned ::operator::new. -def AllocAlign: EnumAttr<"allocalign", [ParamAttr]>; +def AllocAlign: EnumAttr<"allocalign", [ParamAttr, IntersectAnd]>; /// Describes behavior of an allocator function in terms of known properties. def AllocKind: IntAttr<"allockind", [FnAttr]>; /// Parameter is the pointer to be manipulated by the allocator function. -def AllocatedPointer : EnumAttr<"allocptr", [ParamAttr]>; +def AllocatedPointer : EnumAttr<"allocptr", [ParamAttr, IntersectAnd]>; /// The result of the function is guaranteed to point to a number of bytes that /// we can determine if we know the value of the function's arguments. @@ -84,23 +115,23 @@ def ByVal : TypeAttr<"byval", [ParamAttr]>; def ByRef : TypeAttr<"byref", [ParamAttr]>; /// Parameter or return value may not contain uninitialized or poison bits. -def NoUndef : EnumAttr<"noundef", [ParamAttr, RetAttr]>; +def NoUndef : EnumAttr<"noundef", [ParamAttr, RetAttr, IntersectAnd]>; /// Marks function as being in a cold path. -def Cold : EnumAttr<"cold", [FnAttr]>; +def Cold : EnumAttr<"cold", [FnAttr, IntersectAnd]>; /// Can only be moved to control-equivalent blocks. -def Convergent : EnumAttr<"convergent", [FnAttr]>; +def Convergent : EnumAttr<"convergent", [FnAttr, IntersectAnd]>; /// Marks function as being in a hot path and frequently called. -def Hot: EnumAttr<"hot", [FnAttr]>; +def Hot: EnumAttr<"hot", [FnAttr, IntersectAnd]>; /// Pointer is known to be dereferenceable. -def Dereferenceable : IntAttr<"dereferenceable", [ParamAttr, RetAttr]>; +def Dereferenceable : IntAttr<"dereferenceable", [ParamAttr, RetAttr, IntersectMin]>; /// Pointer is either null or dereferenceable. def DereferenceableOrNull : IntAttr<"dereferenceable_or_null", - [ParamAttr, RetAttr]>; + [ParamAttr, RetAttr, IntersectMin]>; /// Do not instrument function with sanitizers. def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>; @@ -122,7 +153,7 @@ def InAlloca : TypeAttr<"inalloca", [ParamAttr]>; def Initializes : ConstantRangeListAttr<"initializes", [ParamAttr]>; /// Source said inlining was desirable. -def InlineHint : EnumAttr<"inlinehint", [FnAttr]>; +def InlineHint : EnumAttr<"inlinehint", [FnAttr, IntersectAnd]>; /// Force argument to be passed in register. def InReg : EnumAttr<"inreg", [ParamAttr, RetAttr]>; @@ -131,10 +162,10 @@ def InReg : EnumAttr<"inreg", [ParamAttr, RetAttr]>; def JumpTable : EnumAttr<"jumptable", [FnAttr]>; /// Memory effects of the function. -def Memory : IntAttr<"memory", [FnAttr]>; +def Memory : IntAttr<"memory", [FnAttr, IntersectCustom]>; /// Forbidden floating-point classes. -def NoFPClass : IntAttr<"nofpclass", [ParamAttr, RetAttr]>; +def NoFPClass : IntAttr<"nofpclass", [ParamAttr, RetAttr, IntersectCustom]>; /// Function must be optimized for size first. def MinSize : EnumAttr<"minsize", [FnAttr]>; @@ -146,16 +177,16 @@ def Naked : EnumAttr<"naked", [FnAttr]>; def Nest : EnumAttr<"nest", [ParamAttr]>; /// Considered to not alias after call. -def NoAlias : EnumAttr<"noalias", [ParamAttr, RetAttr]>; +def NoAlias : EnumAttr<"noalias", [ParamAttr, RetAttr, IntersectAnd]>; /// Callee isn't recognized as a builtin. def NoBuiltin : EnumAttr<"nobuiltin", [FnAttr]>; /// Function cannot enter into caller's translation unit. -def NoCallback : EnumAttr<"nocallback", [FnAttr]>; +def NoCallback : EnumAttr<"nocallback", [FnAttr, IntersectAnd]>; /// Function creates no aliases of pointer. -def NoCapture : EnumAttr<"nocapture", [ParamAttr]>; +def NoCapture : EnumAttr<"nocapture", [ParamAttr, IntersectAnd]>; /// Call cannot be duplicated. def NoDuplicate : EnumAttr<"noduplicate", [FnAttr]>; @@ -164,10 +195,10 @@ def NoDuplicate : EnumAttr<"noduplicate", [FnAttr]>; def NoExt : EnumAttr<"noext", [ParamAttr, RetAttr]>; /// Function does not deallocate memory. -def NoFree : EnumAttr<"nofree", [FnAttr, ParamAttr]>; +def NoFree : EnumAttr<"nofree", [FnAttr, ParamAttr, IntersectAnd]>; /// Argument is dead if the call unwinds. -def DeadOnUnwind : EnumAttr<"dead_on_unwind", [ParamAttr]>; +def DeadOnUnwind : EnumAttr<"dead_on_unwind", [ParamAttr, IntersectAnd]>; /// Disable implicit floating point insts. def NoImplicitFloat : EnumAttr<"noimplicitfloat", [FnAttr]>; @@ -182,19 +213,19 @@ def NonLazyBind : EnumAttr<"nonlazybind", [FnAttr]>; def NoMerge : EnumAttr<"nomerge", [FnAttr]>; /// Pointer is known to be not null. -def NonNull : EnumAttr<"nonnull", [ParamAttr, RetAttr]>; +def NonNull : EnumAttr<"nonnull", [ParamAttr, RetAttr, IntersectAnd]>; /// The function does not recurse. -def NoRecurse : EnumAttr<"norecurse", [FnAttr]>; +def NoRecurse : EnumAttr<"norecurse", [FnAttr, IntersectAnd]>; /// Disable redzone. def NoRedZone : EnumAttr<"noredzone", [FnAttr]>; /// Mark the function as not returning. -def NoReturn : EnumAttr<"noreturn", [FnAttr]>; +def NoReturn : EnumAttr<"noreturn", [FnAttr, IntersectAnd]>; /// Function does not synchronize. -def NoSync : EnumAttr<"nosync", [FnAttr]>; +def NoSync : EnumAttr<"nosync", [FnAttr, IntersectAnd]>; /// Disable Indirect Branch Tracking. def NoCfCheck : EnumAttr<"nocf_check", [FnAttr]>; @@ -207,7 +238,7 @@ def NoProfile : EnumAttr<"noprofile", [FnAttr]>; def SkipProfile : EnumAttr<"skipprofile", [FnAttr]>; /// Function doesn't unwind stack. -def NoUnwind : EnumAttr<"nounwind", [FnAttr]>; +def NoUnwind : EnumAttr<"nounwind", [FnAttr, IntersectAnd]>; /// No SanitizeBounds instrumentation. def NoSanitizeBounds : EnumAttr<"nosanitize_bounds", [FnAttr]>; @@ -234,16 +265,16 @@ def OptimizeNone : EnumAttr<"optnone", [FnAttr]>; def Preallocated : TypeAttr<"preallocated", [FnAttr, ParamAttr]>; /// Parameter or return value is within the specified range. -def Range : ConstantRangeAttr<"range", [ParamAttr, RetAttr]>; +def Range : ConstantRangeAttr<"range", [ParamAttr, RetAttr, IntersectCustom]>; /// Function does not access memory. -def ReadNone : EnumAttr<"readnone", [ParamAttr]>; +def ReadNone : EnumAttr<"readnone", [ParamAttr, IntersectAnd]>; /// Function only reads from memory. -def ReadOnly : EnumAttr<"readonly", [ParamAttr]>; +def ReadOnly : EnumAttr<"readonly", [ParamAttr, IntersectAnd]>; /// Return value is always equal to this argument. -def Returned : EnumAttr<"returned", [ParamAttr]>; +def Returned : EnumAttr<"returned", [ParamAttr, IntersectAnd]>; /// Parameter is required to be a trivial constant. def ImmArg : EnumAttr<"immarg", [ParamAttr]>; @@ -265,7 +296,7 @@ def SExt : EnumAttr<"signext", [ParamAttr, RetAttr]>; def StackAlignment : IntAttr<"alignstack", [FnAttr, ParamAttr]>; /// Function can be speculated. -def Speculatable : EnumAttr<"speculatable", [FnAttr]>; +def Speculatable : EnumAttr<"speculatable", [FnAttr, IntersectAnd]>; /// Stack protection. def StackProtect : EnumAttr<"ssp", [FnAttr]>; @@ -332,19 +363,19 @@ def UWTable : IntAttr<"uwtable", [FnAttr]>; def VScaleRange : IntAttr<"vscale_range", [FnAttr]>; /// Function always comes back to callsite. -def WillReturn : EnumAttr<"willreturn", [FnAttr]>; +def WillReturn : EnumAttr<"willreturn", [FnAttr, IntersectAnd]>; /// Pointer argument is writable. -def Writable : EnumAttr<"writable", [ParamAttr]>; +def Writable : EnumAttr<"writable", [ParamAttr, IntersectAnd]>; /// Function only writes to memory. -def WriteOnly : EnumAttr<"writeonly", [ParamAttr]>; +def WriteOnly : EnumAttr<"writeonly", [ParamAttr, IntersectAnd]>; /// Zero extended before/after call. def ZExt : EnumAttr<"zeroext", [ParamAttr, RetAttr]>; /// Function is required to make Forward Progress. -def MustProgress : EnumAttr<"mustprogress", [FnAttr]>; +def MustProgress : EnumAttr<"mustprogress", [FnAttr, IntersectAnd]>; /// Function is a presplit coroutine. def PresplitCoroutine : EnumAttr<"presplitcoroutine", [FnAttr]>; diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index fa124e46483dc..2e54ef1aa3e00 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -727,6 +727,9 @@ enum AttributeProperty { FnAttr = (1 << 0), ParamAttr = (1 << 1), RetAttr = (1 << 2), + IntersectAnd = (1 << 3), + IntersectMin = (1 << 4), + IntersectCustom = (1 << 5), }; #define GET_ATTR_PROP_TABLE @@ -751,6 +754,16 @@ bool Attribute::canUseAsRetAttr(AttrKind Kind) { return hasAttributeProperty(Kind, AttributeProperty::RetAttr); } +bool Attribute::intersectWithAnd(AttrKind Kind) { + return hasAttributeProperty(Kind, AttributeProperty::IntersectAnd); +} +bool Attribute::intersectWithMin(AttrKind Kind) { + return hasAttributeProperty(Kind, AttributeProperty::IntersectMin); +} +bool Attribute::intersectWithCustom(AttrKind Kind) { + return hasAttributeProperty(Kind, AttributeProperty::IntersectCustom); +} + //===----------------------------------------------------------------------===// // AttributeImpl Definition //===----------------------------------------------------------------------===// @@ -903,6 +916,98 @@ AttributeSet AttributeSet::removeAttributes(LLVMContext &C, return get(C, B); } +std::optional +AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { + if (*this == Other) + return *this; + AttrBuilder Intersected(C); + // Get common set of all attributes then handle each attr according to its + // intersect rule. + AttributeSet Merged = addAttributes(C, Other); + for (Attribute Attr : Merged) { + // Only supporting enum attrs for now. + if (!Attr.hasKindAsEnum()) + return std::nullopt; + + Attribute::AttrKind Kind = Attr.getKindAsEnum(); + std::optional Attr0, Attr1; + if (hasAttribute(Kind)) + Attr0 = getAttribute(Kind); + if (Other.hasAttribute(Kind)) + Attr1 = Other.getAttribute(Kind); + + auto IntersectEq = [&]() { + if (!Attr0 || !Attr1) + return false; + if (*Attr0 != *Attr1) + return false; + Intersected.addAttribute(Attr); + return true; + }; + + // Attribute we can intersect with "and" + if (Attribute::intersectWithAnd(Kind)) { + assert(Attribute::isEnumAttrKind(Kind) && + "Invalid attr type of intersectAnd"); + if (Attr0 && Attr1) + Intersected.addAttribute(Kind); + continue; + } + bool BothValid = Attr0 && Attr1 && Attr0->isValid() && Attr1->isValid(); + + // Attribute we can intersect with "min" + if (Attribute::intersectWithMin(Kind)) { + assert(Attribute::isIntAttrKind(Kind) && + "Invalid attr type of intersectMin"); + if (!BothValid) + continue; + + uint64_t NewVal = std::min(getAttribute(Kind).getValueAsInt(), + Other.getAttribute(Kind).getValueAsInt()); + Intersected.addRawIntAttr(Kind, NewVal); + continue; + } + // Attribute we can intersect but need a custom rule for. + if (Attribute::intersectWithCustom(Kind)) { + switch (Kind) { + case Attribute::Alignment: + if (BothValid) + Intersected.addAlignmentAttr(std::min( + getAlignment().valueOrOne(), Other.getAlignment().valueOrOne())); + break; + case Attribute::Memory: + if (BothValid) + Intersected.addMemoryAttr(getMemoryEffects() | + Other.getMemoryEffects()); + break; + case Attribute::NoFPClass: + if (BothValid) + Intersected.addNoFPClassAttr(getNoFPClass() & Other.getNoFPClass()); + break; + case Attribute::Range: + if (BothValid) { + ConstantRange Range0 = getAttribute(Kind).getRange(); + ConstantRange Range1 = Other.getAttribute(Kind).getRange(); + ConstantRange NewRange = Range0.unionWith(Range1); + if (!NewRange.isFullSet()) + Intersected.addRangeAttr(NewRange); + } + break; + default: + llvm_unreachable("Unknown attribute with custom intersection rule"); + } + continue; + } + + // Attributes with no intersection rule. Only intersect if they are equal. + // Otherwise fail. + if (!IntersectEq()) + return std::nullopt; + } + + return get(C, Intersected); +} + unsigned AttributeSet::getNumAttributes() const { return SetNode ? SetNode->getNumAttributes() : 0; } @@ -1614,6 +1719,31 @@ AttributeList AttributeList::addAllocSizeParamAttr( return addParamAttributes(C, Index, B); } +std::optional +AttributeList::intersectAttributes(LLVMContext &C, AttributeList Other) const { + // Trivial case, the two lists are equal. + if (*this == Other) + return *this; + + // At least for now, only intersect lists with same number of params. + if (getNumAttrSets() != Other.getNumAttrSets()) + return std::nullopt; + + AttributeList IntersectedAttrs{}; + for (unsigned Idx : indexes()) { + auto IntersectedAB = + getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx)); + // If any index fails to intersect, fail. + if (!IntersectedAB) + return std::nullopt; + + IntersectedAttrs = + IntersectedAttrs.setAttributesAtIndex(C, Idx, *IntersectedAB); + } + + return IntersectedAttrs; +} + //===----------------------------------------------------------------------===// // AttributeList Accessor Methods //===----------------------------------------------------------------------===// diff --git a/llvm/unittests/IR/AttributesTest.cpp b/llvm/unittests/IR/AttributesTest.cpp index da72fa14510cb..2c22b960e5026 100644 --- a/llvm/unittests/IR/AttributesTest.cpp +++ b/llvm/unittests/IR/AttributesTest.cpp @@ -8,6 +8,7 @@ #include "llvm/IR/Attributes.h" #include "llvm-c/Core.h" +#include "llvm/ADT/FloatingPointMode.h" #include "llvm/AsmParser/Parser.h" #include "llvm/IR/AttributeMask.h" #include "llvm/IR/ConstantRange.h" @@ -386,4 +387,271 @@ TEST(Attributes, CalleeAttributes) { } } +TEST(Attributes, SetIntersect) { + LLVMContext C0, C1; + std::optional Res; + auto BuildAttr = [&](LLVMContext &C, Attribute::AttrKind Kind, uint64_t Int, + Type *Ty, ConstantRange &CR, + ArrayRef CRList) { + if (Attribute::isEnumAttrKind(Kind)) + return Attribute::get(C, Kind); + if (Attribute::isTypeAttrKind(Kind)) + return Attribute::get(C, Kind, Ty); + if (Attribute::isIntAttrKind(Kind)) + return Attribute::get(C, Kind, Int); + if (Attribute::isConstantRangeAttrKind(Kind)) + return Attribute::get(C, Kind, CR); + if (Attribute::isConstantRangeListAttrKind(Kind)) + return Attribute::get(C, Kind, CRList); + std::abort(); + }; + for (unsigned i = Attribute::AttrKind::None + 1, + e = Attribute::AttrKind::EndAttrKinds; + i < e; ++i) { + Attribute::AttrKind Kind = static_cast(i); + + Attribute::AttrKind Other = + Kind == Attribute::NoUndef ? Attribute::NonNull : Attribute::NoUndef; + AttributeSet AS0, AS1; + AttrBuilder AB0(C0); + AttrBuilder AB1(C1); + uint64_t V0, V1; + V0 = 0; + V1 = 0; + if (Attribute::intersectWithCustom(Kind)) { + switch (Kind) { + case Attribute::Alignment: + V0 = 2; + V1 = 4; + break; + case Attribute::Memory: + V0 = MemoryEffects::readOnly().toIntValue(); + V1 = MemoryEffects::none().toIntValue(); + break; + case Attribute::NoFPClass: + V0 = FPClassTest::fcNan | FPClassTest::fcInf; + V1 = FPClassTest::fcNan; + break; + case Attribute::Range: + break; + default: + ASSERT_FALSE(true); + } + } else { + V0 = (i & 2) + 1; + V1 = (2 - (i & 2)) + 1; + } + + ConstantRange CR0(APInt(32, 0), APInt(32, 10)); + ConstantRange CR1(APInt(32, 15), APInt(32, 20)); + ArrayRef CRL0 = {CR0}; + ArrayRef CRL1 = {CR0, CR1}; + Type *T0 = Type::getInt32Ty(C0); + Type *T1 = Type::getInt64Ty(C0); + Attribute Attr0 = BuildAttr(C0, Kind, V0, T0, CR0, CRL0); + Attribute Attr1 = BuildAttr( + C1, Attribute::isEnumAttrKind(Kind) ? Other : Kind, V1, T1, CR1, CRL1); + bool CanDrop = Attribute::intersectWithAnd(Kind) || + Attribute::intersectWithMin(Kind) || + Attribute::intersectWithCustom(Kind); + + AB0.addAttribute(Attr0); + AB1.addAttribute(Attr1); + + Res = AS0.intersectWith(C0, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_EQ(AS0, *Res); + + AS0 = AttributeSet::get(C0, AB0); + Res = AS0.intersectWith(C0, AS1); + ASSERT_EQ(Res.has_value(), CanDrop); + if (CanDrop) + ASSERT_FALSE(Res->hasAttributes()); + + AS1 = AttributeSet::get(C1, AB0); + Res = AS0.intersectWith(C0, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_EQ(AS0, *Res); + + AS1 = AttributeSet::get(C1, AB1); + Res = AS0.intersectWith(C0, AS1); + if (!CanDrop) { + ASSERT_FALSE(Res.has_value()); + continue; + } + if (Attribute::intersectWithAnd(Kind)) { + ASSERT_TRUE(Res.has_value()); + ASSERT_FALSE(Res->hasAttributes()); + + AS1 = AS1.addAttribute(C1, Kind); + Res = AS0.intersectWith(C0, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttributes()); + ASSERT_TRUE(Res->hasAttribute(Kind)); + ASSERT_FALSE(Res->hasAttribute(Other)); + } else if (Attribute::intersectWithMin(Kind)) { + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttributes()); + ASSERT_TRUE(Res->hasAttribute(Kind)); + ASSERT_EQ(Res->getAttribute(Kind).getValueAsInt(), std::min(V0, V1)); + } else if (Attribute::intersectWithCustom(Kind)) { + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttributes()); + ASSERT_TRUE(Res->hasAttribute(Kind)); + + switch (Kind) { + case Attribute::Alignment: + ASSERT_EQ(Res->getAlignment().valueOrOne(), MaybeAlign(2).valueOrOne()); + break; + case Attribute::Memory: + ASSERT_EQ(Res->getMemoryEffects(), MemoryEffects::readOnly()); + break; + case Attribute::NoFPClass: + ASSERT_EQ(Res->getNoFPClass(), FPClassTest::fcNan); + break; + case Attribute::Range: + ASSERT_EQ(Res->getAttribute(Kind).getRange(), + ConstantRange(APInt(32, 0), APInt(32, 20))); + break; + default: + ASSERT_FALSE(true); + } + } + AS0 = AS0.addAttribute(C0, Attribute::AlwaysInline); + ASSERT_FALSE(AS0.intersectWith(C0, AS1).has_value()); + } +} + +TEST(Attributes, ListIntersect) { + LLVMContext C; + AttributeList AL0; + AttributeList AL1; + std::optional Res; + AL0 = AL0.addRetAttribute(C, Attribute::NoUndef); + AL1 = AL1.addRetAttribute(C, Attribute::NoUndef); + + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_EQ(AL0, *Res); + + AL0 = AL0.addParamAttribute(C, 1, Attribute::NoUndef); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_FALSE(Res.has_value()); + + AL1 = AL1.addParamAttribute(C, 2, Attribute::NoUndef); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_FALSE(Res.has_value()); + + AL0 = AL0.addParamAttribute(C, 2, Attribute::NoUndef); + AL1 = AL1.addParamAttribute(C, 1, Attribute::NoUndef); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_EQ(AL0, *Res); + + AL0 = AL0.addParamAttribute(C, 2, Attribute::NonNull); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_NE(AL0, *Res); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + + AL0 = AL0.addRetAttribute(C, Attribute::NonNull); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_NE(AL0, *Res); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + + AL0 = AL0.addFnAttribute(C, Attribute::ReadOnly); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_NE(AL0, *Res); + ASSERT_FALSE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + + AL1 = AL1.addFnAttribute(C, Attribute::ReadOnly); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_NE(AL0, *Res); + ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + + AL1 = AL1.addFnAttribute(C, Attribute::AlwaysInline); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_FALSE(Res.has_value()); + + AL0 = AL0.addFnAttribute(C, Attribute::AlwaysInline); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); + ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + + AL1 = AL1.addParamAttribute(C, 2, Attribute::ReadNone); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); + ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone)); + + AL1 = AL1.addParamAttribute(C, 3, Attribute::ReadNone); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_FALSE(Res.has_value()); + + AL0 = AL0.addParamAttribute(C, 3, Attribute::ReadNone); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); + ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone)); + ASSERT_TRUE(Res->hasParamAttr(3, Attribute::ReadNone)); + + AL0 = AL0.addParamAttribute( + C, {3}, Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C))); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_FALSE(Res.has_value()); + + AL1 = AL1.addParamAttribute( + C, {3}, Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C))); + Res = AL0.intersectAttributes(C, AL1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); + ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); + ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); + ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull)); + ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef)); + ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); + ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone)); + ASSERT_TRUE(Res->hasParamAttr(3, Attribute::ReadNone)); + ASSERT_TRUE(Res->hasParamAttr(3, Attribute::ByVal)); +} + } // end anonymous namespace diff --git a/llvm/utils/TableGen/Attributes.cpp b/llvm/utils/TableGen/Attributes.cpp index 8d16ff89aae0f..2d2a9af186fd5 100644 --- a/llvm/utils/TableGen/Attributes.cpp +++ b/llvm/utils/TableGen/Attributes.cpp @@ -8,6 +8,7 @@ #include "llvm/TableGen/Record.h" #include "llvm/TableGen/TableGenBackend.h" +#include "llvm/TableGen/Error.h" #include using namespace llvm; @@ -117,10 +118,21 @@ void Attributes::emitAttributeProperties(raw_ostream &OS) { OS << "static const uint8_t AttrPropTable[] = {\n"; for (StringRef KindName : {"EnumAttr", "TypeAttr", "IntAttr", "ConstantRangeAttr", "ConstantRangeListAttr"}) { + bool AllowIntersectAnd = KindName == "EnumAttr"; + bool AllowIntersectMin = KindName == "IntAttr"; for (auto *A : Records.getAllDerivedDefinitions(KindName)) { OS << "0"; - for (Init *P : *A->getValueAsListInit("Properties")) + for (Init *P : *A->getValueAsListInit("Properties")) { + if (!AllowIntersectAnd && + cast(P)->getDef()->getName() == "IntersectAnd") + PrintFatalError( + "'IntersectAnd' only compatible with 'EnumAttr' or 'TypeAttr'"); + if (!AllowIntersectMin && + cast(P)->getDef()->getName() == "IntersectMin") + PrintFatalError("'IntersectMin' only compatible with 'IntAttr'"); + OS << " | AttributeProperty::" << cast(P)->getDef()->getName(); + } OS << ",\n"; } } From 16d2b022f3bd802a31326a45dd65bfe03a05845e Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Mon, 23 Sep 2024 16:41:32 -0500 Subject: [PATCH 02/10] Fix Fork --- llvm/utils/TableGen/Attributes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/utils/TableGen/Attributes.cpp b/llvm/utils/TableGen/Attributes.cpp index 2d2a9af186fd5..8b79462fc544f 100644 --- a/llvm/utils/TableGen/Attributes.cpp +++ b/llvm/utils/TableGen/Attributes.cpp @@ -6,9 +6,9 @@ // //===----------------------------------------------------------------------===// +#include "llvm/TableGen/Error.h" #include "llvm/TableGen/Record.h" #include "llvm/TableGen/TableGenBackend.h" -#include "llvm/TableGen/Error.h" #include using namespace llvm; From 2009dfe9a0d6068e76f26c899b93c986a2c72b78 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Tue, 24 Sep 2024 21:38:37 -0500 Subject: [PATCH 03/10] Improve intersect/incorperate suggestions --- llvm/include/llvm/IR/Attributes.h | 9 +- llvm/include/llvm/IR/Attributes.td | 227 ++++++++++++++------------- llvm/lib/IR/AttributeImpl.h | 3 + llvm/lib/IR/Attributes.cpp | 195 +++++++++++++++-------- llvm/unittests/IR/AttributesTest.cpp | 2 + llvm/utils/TableGen/Attributes.cpp | 2 +- 6 files changed, 252 insertions(+), 186 deletions(-) diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h index ac0b00e2ccddb..90981b7ce3fbb 100644 --- a/llvm/include/llvm/IR/Attributes.h +++ b/llvm/include/llvm/IR/Attributes.h @@ -117,6 +117,7 @@ class Attribute { static bool canUseAsParamAttr(AttrKind Kind); static bool canUseAsRetAttr(AttrKind Kind); + static bool intersectMustPreserve(AttrKind Kind); static bool intersectWithAnd(AttrKind Kind); static bool intersectWithMin(AttrKind Kind); static bool intersectWithCustom(AttrKind Kind); @@ -214,10 +215,7 @@ class Attribute { /// Returns true if the attribute's kind can be represented as an enum (Enum, /// Integer, Type, ConstantRange, or ConstantRangeList attribute). - bool hasKindAsEnum() const { - return isEnumAttribute() || isIntAttribute() || isTypeAttribute() || - isConstantRangeAttribute() || isConstantRangeListAttribute(); - } + bool hasKindAsEnum() const { return !isStringAttribute(); } /// Return the attribute's kind as an enum (Attribute::AttrKind). This /// requires the attribute be representable as an enum (see: `hasKindAsEnum`). @@ -306,6 +304,9 @@ class Attribute { bool operator==(Attribute A) const { return pImpl == A.pImpl; } bool operator!=(Attribute A) const { return pImpl != A.pImpl; } + /// Used to sort attribute by kind. + int cmpKind(Attribute A) const; + /// Less-than operator. Useful for sorting the attributes list. bool operator<(Attribute A) const; diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td index ca24627912f27..a0eb050876ec3 100644 --- a/llvm/include/llvm/IR/Attributes.td +++ b/llvm/include/llvm/IR/Attributes.td @@ -26,9 +26,6 @@ def RetAttr : AttrProperty; /// Intersection rules. Used for example in sinking/hoisting two /// callbases to find a set of attributes that apply to both. -/// If no rule is specified, the assumed rule is that the attrs -/// must be equal, otherwise the intersection will fail. Having -/// any rule implies that it is legal to drop the attribute. /// Note, there are some attributes we can (probably) legally drop /// but are intentionally excluded as of now. Those include: /// - initializes @@ -38,305 +35,310 @@ def RetAttr : AttrProperty; /// - optsize /// - optnone /// - optdebug -/// - optforfuzzing +/// - optforfuzzing /// +/// When intersecting the attributes must both present and equal. +/// Use this for attributes you are not certain it is safe to drop +/// at any time. I.e `byval(Ty)` on a parameter. +def IntersectPreserve : AttrProperty; + /// When intersecting take the AND of the two attrs. -/// Invalid for Int/ConstantRange/ConstantRangeList attrs. +/// Only valid for Enum attrs. def IntersectAnd : AttrProperty; /// When intersecting take the min value of the two attrs. /// Only valid for Int attrs. def IntersectMin : AttrProperty; -/// When intersecting rely on some user defined code. +/// When intersecting rely on some specially defined code. def IntersectCustom : AttrProperty; - + /// Attribute base class. -class Attr P> { +class Attr P> { // String representation of this attribute in the IR. string AttrString = S; - list Properties = P; + list Properties = P # [I]; } /// Enum attribute. -class EnumAttr P> : Attr; +class EnumAttr P> : Attr; /// Int attribute. -class IntAttr P> : Attr; +class IntAttr P> : Attr; /// Type attribute. -class TypeAttr P> : Attr; +class TypeAttr P> : Attr; /// StringBool attribute. -class StrBoolAttr : Attr; +class StrBoolAttr : Attr; /// Arbitrary string attribute. -class ComplexStrAttr P> : Attr; +class ComplexStrAttr P> : Attr; /// ConstantRange attribute. -class ConstantRangeAttr P> : Attr; +class ConstantRangeAttr P> : Attr; /// ConstantRangeList attribute. -class ConstantRangeListAttr P> : Attr; +class ConstantRangeListAttr P> : Attr; /// Target-independent enum attributes. /// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias. /// 0 means unaligned (different from align(1)). -def Alignment : IntAttr<"align", [ParamAttr, RetAttr, IntersectCustom]>; +def Alignment : IntAttr<"align", IntersectCustom, [ParamAttr, RetAttr]>; /// Parameter of a function that tells us the alignment of an allocation, as in /// aligned_alloc and aligned ::operator::new. -def AllocAlign: EnumAttr<"allocalign", [ParamAttr, IntersectAnd]>; +def AllocAlign: EnumAttr<"allocalign", IntersectAnd, [ParamAttr]>; /// Describes behavior of an allocator function in terms of known properties. -def AllocKind: IntAttr<"allockind", [FnAttr]>; +def AllocKind: IntAttr<"allockind", IntersectPreserve, [FnAttr]>; /// Parameter is the pointer to be manipulated by the allocator function. -def AllocatedPointer : EnumAttr<"allocptr", [ParamAttr, IntersectAnd]>; +def AllocatedPointer : EnumAttr<"allocptr", IntersectAnd, [ParamAttr]>; /// The result of the function is guaranteed to point to a number of bytes that /// we can determine if we know the value of the function's arguments. -def AllocSize : IntAttr<"allocsize", [FnAttr]>; +def AllocSize : IntAttr<"allocsize", IntersectPreserve, [FnAttr]>; /// inline=always. -def AlwaysInline : EnumAttr<"alwaysinline", [FnAttr]>; +def AlwaysInline : EnumAttr<"alwaysinline", IntersectPreserve, [FnAttr]>; /// Callee is recognized as a builtin, despite nobuiltin attribute on its /// declaration. -def Builtin : EnumAttr<"builtin", [FnAttr]>; +def Builtin : EnumAttr<"builtin", IntersectPreserve, [FnAttr]>; /// Pass structure by value. -def ByVal : TypeAttr<"byval", [ParamAttr]>; +def ByVal : TypeAttr<"byval", IntersectPreserve, [ParamAttr]>; /// Mark in-memory ABI type. -def ByRef : TypeAttr<"byref", [ParamAttr]>; +def ByRef : TypeAttr<"byref", IntersectPreserve, [ParamAttr]>; /// Parameter or return value may not contain uninitialized or poison bits. -def NoUndef : EnumAttr<"noundef", [ParamAttr, RetAttr, IntersectAnd]>; +def NoUndef : EnumAttr<"noundef", IntersectAnd, [ParamAttr, RetAttr]>; /// Marks function as being in a cold path. -def Cold : EnumAttr<"cold", [FnAttr, IntersectAnd]>; +def Cold : EnumAttr<"cold", IntersectAnd, [FnAttr]>; /// Can only be moved to control-equivalent blocks. -def Convergent : EnumAttr<"convergent", [FnAttr, IntersectAnd]>; +def Convergent : EnumAttr<"convergent", IntersectAnd, [FnAttr]>; /// Marks function as being in a hot path and frequently called. -def Hot: EnumAttr<"hot", [FnAttr, IntersectAnd]>; +def Hot: EnumAttr<"hot", IntersectAnd, [FnAttr]>; /// Pointer is known to be dereferenceable. -def Dereferenceable : IntAttr<"dereferenceable", [ParamAttr, RetAttr, IntersectMin]>; +def Dereferenceable : IntAttr<"dereferenceable", IntersectMin, [ParamAttr, RetAttr]>; /// Pointer is either null or dereferenceable. -def DereferenceableOrNull : IntAttr<"dereferenceable_or_null", - [ParamAttr, RetAttr, IntersectMin]>; +def DereferenceableOrNull : IntAttr<"dereferenceable_or_null", IntersectMin, + [ParamAttr, RetAttr]>; /// Do not instrument function with sanitizers. -def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>; +def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", IntersectPreserve, [FnAttr]>; /// Provide pointer element type to intrinsic. -def ElementType : TypeAttr<"elementtype", [ParamAttr]>; +def ElementType : TypeAttr<"elementtype", IntersectPreserve, [ParamAttr]>; /// Whether to keep return instructions, or replace with a jump to an external /// symbol. -def FnRetThunkExtern : EnumAttr<"fn_ret_thunk_extern", [FnAttr]>; +def FnRetThunkExtern : EnumAttr<"fn_ret_thunk_extern", IntersectPreserve, [FnAttr]>; /// Function has a hybrid patchable thunk. -def HybridPatchable : EnumAttr<"hybrid_patchable", [FnAttr]>; +def HybridPatchable : EnumAttr<"hybrid_patchable", IntersectPreserve, [FnAttr]>; /// Pass structure in an alloca. -def InAlloca : TypeAttr<"inalloca", [ParamAttr]>; +def InAlloca : TypeAttr<"inalloca", IntersectPreserve, [ParamAttr]>; /// Pointer argument memory is initialized. -def Initializes : ConstantRangeListAttr<"initializes", [ParamAttr]>; +def Initializes : ConstantRangeListAttr<"initializes", IntersectPreserve, [ParamAttr]>; /// Source said inlining was desirable. -def InlineHint : EnumAttr<"inlinehint", [FnAttr, IntersectAnd]>; +def InlineHint : EnumAttr<"inlinehint", IntersectAnd, [FnAttr]>; /// Force argument to be passed in register. -def InReg : EnumAttr<"inreg", [ParamAttr, RetAttr]>; +def InReg : EnumAttr<"inreg", IntersectPreserve, [ParamAttr, RetAttr]>; /// Build jump-instruction tables and replace refs. -def JumpTable : EnumAttr<"jumptable", [FnAttr]>; +def JumpTable : EnumAttr<"jumptable", IntersectPreserve, [FnAttr]>; /// Memory effects of the function. -def Memory : IntAttr<"memory", [FnAttr, IntersectCustom]>; +def Memory : IntAttr<"memory", IntersectCustom, [FnAttr]>; /// Forbidden floating-point classes. -def NoFPClass : IntAttr<"nofpclass", [ParamAttr, RetAttr, IntersectCustom]>; +def NoFPClass : IntAttr<"nofpclass", IntersectCustom, [ParamAttr, RetAttr]>; /// Function must be optimized for size first. -def MinSize : EnumAttr<"minsize", [FnAttr]>; +def MinSize : EnumAttr<"minsize", IntersectPreserve, [FnAttr]>; /// Naked function. -def Naked : EnumAttr<"naked", [FnAttr]>; +def Naked : EnumAttr<"naked", IntersectPreserve, [FnAttr]>; /// Nested function static chain. -def Nest : EnumAttr<"nest", [ParamAttr]>; +def Nest : EnumAttr<"nest", IntersectPreserve, [ParamAttr]>; /// Considered to not alias after call. -def NoAlias : EnumAttr<"noalias", [ParamAttr, RetAttr, IntersectAnd]>; +def NoAlias : EnumAttr<"noalias", IntersectAnd, [ParamAttr, RetAttr]>; /// Callee isn't recognized as a builtin. -def NoBuiltin : EnumAttr<"nobuiltin", [FnAttr]>; +def NoBuiltin : EnumAttr<"nobuiltin", IntersectPreserve, [FnAttr]>; /// Function cannot enter into caller's translation unit. -def NoCallback : EnumAttr<"nocallback", [FnAttr, IntersectAnd]>; +def NoCallback : EnumAttr<"nocallback", IntersectAnd, [FnAttr]>; /// Function creates no aliases of pointer. -def NoCapture : EnumAttr<"nocapture", [ParamAttr, IntersectAnd]>; +def NoCapture : EnumAttr<"nocapture", IntersectAnd, [ParamAttr]>; /// Call cannot be duplicated. -def NoDuplicate : EnumAttr<"noduplicate", [FnAttr]>; +def NoDuplicate : EnumAttr<"noduplicate", IntersectPreserve, [FnAttr]>; /// No extension needed before/after call (high bits are undefined). -def NoExt : EnumAttr<"noext", [ParamAttr, RetAttr]>; +def NoExt : EnumAttr<"noext", IntersectPreserve, [ParamAttr, RetAttr]>; /// Function does not deallocate memory. -def NoFree : EnumAttr<"nofree", [FnAttr, ParamAttr, IntersectAnd]>; +def NoFree : EnumAttr<"nofree", IntersectAnd, [FnAttr, ParamAttr]>; /// Argument is dead if the call unwinds. -def DeadOnUnwind : EnumAttr<"dead_on_unwind", [ParamAttr, IntersectAnd]>; +def DeadOnUnwind : EnumAttr<"dead_on_unwind", IntersectAnd, [ParamAttr]>; /// Disable implicit floating point insts. -def NoImplicitFloat : EnumAttr<"noimplicitfloat", [FnAttr]>; +def NoImplicitFloat : EnumAttr<"noimplicitfloat", IntersectPreserve, [FnAttr]>; /// inline=never. -def NoInline : EnumAttr<"noinline", [FnAttr]>; +def NoInline : EnumAttr<"noinline", IntersectPreserve, [FnAttr]>; /// Function is called early and/or often, so lazy binding isn't worthwhile. -def NonLazyBind : EnumAttr<"nonlazybind", [FnAttr]>; +def NonLazyBind : EnumAttr<"nonlazybind", IntersectPreserve, [FnAttr]>; /// Disable merging for specified functions or call sites. -def NoMerge : EnumAttr<"nomerge", [FnAttr]>; +def NoMerge : EnumAttr<"nomerge", IntersectPreserve, [FnAttr]>; /// Pointer is known to be not null. -def NonNull : EnumAttr<"nonnull", [ParamAttr, RetAttr, IntersectAnd]>; +def NonNull : EnumAttr<"nonnull", IntersectAnd, [ParamAttr, RetAttr]>; /// The function does not recurse. -def NoRecurse : EnumAttr<"norecurse", [FnAttr, IntersectAnd]>; +def NoRecurse : EnumAttr<"norecurse", IntersectAnd, [FnAttr]>; /// Disable redzone. -def NoRedZone : EnumAttr<"noredzone", [FnAttr]>; +def NoRedZone : EnumAttr<"noredzone", IntersectPreserve, [FnAttr]>; /// Mark the function as not returning. -def NoReturn : EnumAttr<"noreturn", [FnAttr, IntersectAnd]>; +def NoReturn : EnumAttr<"noreturn", IntersectAnd, [FnAttr]>; /// Function does not synchronize. -def NoSync : EnumAttr<"nosync", [FnAttr, IntersectAnd]>; +def NoSync : EnumAttr<"nosync", IntersectAnd, [FnAttr]>; /// Disable Indirect Branch Tracking. -def NoCfCheck : EnumAttr<"nocf_check", [FnAttr]>; +def NoCfCheck : EnumAttr<"nocf_check", IntersectPreserve, [FnAttr]>; /// Function should not be instrumented. -def NoProfile : EnumAttr<"noprofile", [FnAttr]>; +def NoProfile : EnumAttr<"noprofile", IntersectPreserve, [FnAttr]>; /// This function should not be instrumented but it is ok to inline profiled // functions into it. -def SkipProfile : EnumAttr<"skipprofile", [FnAttr]>; +def SkipProfile : EnumAttr<"skipprofile", IntersectPreserve, [FnAttr]>; /// Function doesn't unwind stack. -def NoUnwind : EnumAttr<"nounwind", [FnAttr, IntersectAnd]>; +def NoUnwind : EnumAttr<"nounwind", IntersectAnd, [FnAttr]>; /// No SanitizeBounds instrumentation. -def NoSanitizeBounds : EnumAttr<"nosanitize_bounds", [FnAttr]>; +def NoSanitizeBounds : EnumAttr<"nosanitize_bounds", IntersectPreserve, [FnAttr]>; /// No SanitizeCoverage instrumentation. -def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>; +def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", IntersectPreserve, [FnAttr]>; /// Null pointer in address space zero is valid. -def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>; +def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", IntersectPreserve, [FnAttr]>; /// Select optimizations that give decent debug info. -def OptimizeForDebugging : EnumAttr<"optdebug", [FnAttr]>; +def OptimizeForDebugging : EnumAttr<"optdebug", IntersectPreserve, [FnAttr]>; /// Select optimizations for best fuzzing signal. -def OptForFuzzing : EnumAttr<"optforfuzzing", [FnAttr]>; +def OptForFuzzing : EnumAttr<"optforfuzzing", IntersectPreserve, [FnAttr]>; /// opt_size. -def OptimizeForSize : EnumAttr<"optsize", [FnAttr]>; +def OptimizeForSize : EnumAttr<"optsize", IntersectPreserve, [FnAttr]>; /// Function must not be optimized. -def OptimizeNone : EnumAttr<"optnone", [FnAttr]>; +def OptimizeNone : EnumAttr<"optnone", IntersectPreserve, [FnAttr]>; /// Similar to byval but without a copy. -def Preallocated : TypeAttr<"preallocated", [FnAttr, ParamAttr]>; +def Preallocated : TypeAttr<"preallocated", IntersectPreserve, [FnAttr, ParamAttr]>; /// Parameter or return value is within the specified range. -def Range : ConstantRangeAttr<"range", [ParamAttr, RetAttr, IntersectCustom]>; +def Range : ConstantRangeAttr<"range", IntersectCustom, [ParamAttr, RetAttr]>; /// Function does not access memory. -def ReadNone : EnumAttr<"readnone", [ParamAttr, IntersectAnd]>; +def ReadNone : EnumAttr<"readnone", IntersectAnd, [ParamAttr]>; /// Function only reads from memory. -def ReadOnly : EnumAttr<"readonly", [ParamAttr, IntersectAnd]>; +def ReadOnly : EnumAttr<"readonly", IntersectAnd, [ParamAttr]>; /// Return value is always equal to this argument. -def Returned : EnumAttr<"returned", [ParamAttr, IntersectAnd]>; +def Returned : EnumAttr<"returned", IntersectAnd, [ParamAttr]>; /// Parameter is required to be a trivial constant. -def ImmArg : EnumAttr<"immarg", [ParamAttr]>; +def ImmArg : EnumAttr<"immarg", IntersectPreserve, [ParamAttr]>; /// Function can return twice. -def ReturnsTwice : EnumAttr<"returns_twice", [FnAttr]>; +def ReturnsTwice : EnumAttr<"returns_twice", IntersectPreserve, [FnAttr]>; /// Safe Stack protection. -def SafeStack : EnumAttr<"safestack", [FnAttr]>; +def SafeStack : EnumAttr<"safestack", IntersectPreserve, [FnAttr]>; /// Shadow Call Stack protection. -def ShadowCallStack : EnumAttr<"shadowcallstack", [FnAttr]>; +def ShadowCallStack : EnumAttr<"shadowcallstack", IntersectPreserve, [FnAttr]>; /// Sign extended before/after call. -def SExt : EnumAttr<"signext", [ParamAttr, RetAttr]>; +def SExt : EnumAttr<"signext", IntersectPreserve, [ParamAttr, RetAttr]>; /// Alignment of stack for function (3 bits) stored as log2 of alignment with /// +1 bias 0 means unaligned (different from alignstack=(1)). -def StackAlignment : IntAttr<"alignstack", [FnAttr, ParamAttr]>; +def StackAlignment : IntAttr<"alignstack", IntersectPreserve, [FnAttr, ParamAttr]>; /// Function can be speculated. -def Speculatable : EnumAttr<"speculatable", [FnAttr, IntersectAnd]>; +def Speculatable : EnumAttr<"speculatable", IntersectAnd, [FnAttr]>; /// Stack protection. -def StackProtect : EnumAttr<"ssp", [FnAttr]>; +def StackProtect : EnumAttr<"ssp", IntersectPreserve, [FnAttr]>; /// Stack protection required. -def StackProtectReq : EnumAttr<"sspreq", [FnAttr]>; +def StackProtectReq : EnumAttr<"sspreq", IntersectPreserve, [FnAttr]>; /// Strong Stack protection. -def StackProtectStrong : EnumAttr<"sspstrong", [FnAttr]>; +def StackProtectStrong : EnumAttr<"sspstrong", IntersectPreserve, [FnAttr]>; /// Function was called in a scope requiring strict floating point semantics. -def StrictFP : EnumAttr<"strictfp", [FnAttr]>; +def StrictFP : EnumAttr<"strictfp", IntersectPreserve, [FnAttr]>; /// Hidden pointer to structure to return. -def StructRet : TypeAttr<"sret", [ParamAttr]>; +def StructRet : TypeAttr<"sret", IntersectPreserve, [ParamAttr]>; /// AddressSanitizer is on. -def SanitizeAddress : EnumAttr<"sanitize_address", [FnAttr]>; +def SanitizeAddress : EnumAttr<"sanitize_address", IntersectPreserve, [FnAttr]>; /// ThreadSanitizer is on. -def SanitizeThread : EnumAttr<"sanitize_thread", [FnAttr]>; +def SanitizeThread : EnumAttr<"sanitize_thread", IntersectPreserve, [FnAttr]>; /// MemorySanitizer is on. -def SanitizeMemory : EnumAttr<"sanitize_memory", [FnAttr]>; +def SanitizeMemory : EnumAttr<"sanitize_memory", IntersectPreserve, [FnAttr]>; /// HWAddressSanitizer is on. -def SanitizeHWAddress : EnumAttr<"sanitize_hwaddress", [FnAttr]>; +def SanitizeHWAddress : EnumAttr<"sanitize_hwaddress", IntersectPreserve, [FnAttr]>; /// MemTagSanitizer is on. -def SanitizeMemTag : EnumAttr<"sanitize_memtag", [FnAttr]>; +def SanitizeMemTag : EnumAttr<"sanitize_memtag", IntersectPreserve, [FnAttr]>; /// NumericalStabilitySanitizer is on. -def SanitizeNumericalStability : EnumAttr<"sanitize_numerical_stability", [FnAttr]>; +def SanitizeNumericalStability : EnumAttr<"sanitize_numerical_stability", IntersectPreserve, [FnAttr]>; /// RealtimeSanitizer is on. -def SanitizeRealtime : EnumAttr<"sanitize_realtime", [FnAttr]>; +def SanitizeRealtime : EnumAttr<"sanitize_realtime", IntersectPreserve, [FnAttr]>; /// RealtimeSanitizer should error if a real-time unsafe function is invoked /// during a real-time sanitized function (see `sanitize_realtime`). -def SanitizeRealtimeUnsafe : EnumAttr<"sanitize_realtime_unsafe", [FnAttr]>; +def SanitizeRealtimeUnsafe : EnumAttr<"sanitize_realtime_unsafe", IntersectPreserve, [FnAttr]>; /// Speculative Load Hardening is enabled. /// @@ -345,47 +347,48 @@ def SanitizeRealtimeUnsafe : EnumAttr<"sanitize_realtime_unsafe", [FnAttr]>; /// body will add the attribute to the caller. This ensures that code carrying /// this attribute will always be lowered with hardening enabled. def SpeculativeLoadHardening : EnumAttr<"speculative_load_hardening", + IntersectPreserve, [FnAttr]>; /// Argument is swift error. -def SwiftError : EnumAttr<"swifterror", [ParamAttr]>; +def SwiftError : EnumAttr<"swifterror", IntersectPreserve, [ParamAttr]>; /// Argument is swift self/context. -def SwiftSelf : EnumAttr<"swiftself", [ParamAttr]>; +def SwiftSelf : EnumAttr<"swiftself", IntersectPreserve, [ParamAttr]>; /// Argument is swift async context. -def SwiftAsync : EnumAttr<"swiftasync", [ParamAttr]>; +def SwiftAsync : EnumAttr<"swiftasync", IntersectPreserve, [ParamAttr]>; /// Function must be in a unwind table. -def UWTable : IntAttr<"uwtable", [FnAttr]>; +def UWTable : IntAttr<"uwtable", IntersectPreserve, [FnAttr]>; /// Minimum/Maximum vscale value for function. -def VScaleRange : IntAttr<"vscale_range", [FnAttr]>; +def VScaleRange : IntAttr<"vscale_range", IntersectPreserve, [FnAttr]>; /// Function always comes back to callsite. -def WillReturn : EnumAttr<"willreturn", [FnAttr, IntersectAnd]>; +def WillReturn : EnumAttr<"willreturn", IntersectAnd, [FnAttr]>; /// Pointer argument is writable. -def Writable : EnumAttr<"writable", [ParamAttr, IntersectAnd]>; +def Writable : EnumAttr<"writable", IntersectAnd, [ParamAttr]>; /// Function only writes to memory. -def WriteOnly : EnumAttr<"writeonly", [ParamAttr, IntersectAnd]>; +def WriteOnly : EnumAttr<"writeonly", IntersectAnd, [ParamAttr]>; /// Zero extended before/after call. -def ZExt : EnumAttr<"zeroext", [ParamAttr, RetAttr]>; +def ZExt : EnumAttr<"zeroext", IntersectPreserve, [ParamAttr, RetAttr]>; /// Function is required to make Forward Progress. -def MustProgress : EnumAttr<"mustprogress", [FnAttr, IntersectAnd]>; +def MustProgress : EnumAttr<"mustprogress", IntersectAnd, [FnAttr]>; /// Function is a presplit coroutine. -def PresplitCoroutine : EnumAttr<"presplitcoroutine", [FnAttr]>; +def PresplitCoroutine : EnumAttr<"presplitcoroutine", IntersectPreserve, [FnAttr]>; /// The coroutine would only be destroyed when it is complete. -def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", [FnAttr]>; +def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", IntersectPreserve, [FnAttr]>; /// The coroutine call meets the elide requirement. Hint the optimization /// pipeline to perform elide on the call or invoke instruction. -def CoroElideSafe : EnumAttr<"coro_elide_safe", [FnAttr]>; +def CoroElideSafe : EnumAttr<"coro_elide_safe", IntersectPreserve, [FnAttr]>; /// Target-independent string attributes. def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">; diff --git a/llvm/lib/IR/AttributeImpl.h b/llvm/lib/IR/AttributeImpl.h index 2f1c7b85e6650..82c501dcafcb7 100644 --- a/llvm/lib/IR/AttributeImpl.h +++ b/llvm/lib/IR/AttributeImpl.h @@ -86,6 +86,9 @@ class AttributeImpl : public FoldingSetNode { ArrayRef getValueAsConstantRangeList() const; + /// Used to sort attributes. KindOnly controls if the sort includes the + /// attributes' values or just the kind. + int cmp(const AttributeImpl &AI, bool KindOnly) const; /// Used when sorting the attributes. bool operator<(const AttributeImpl &AI) const; diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 2e54ef1aa3e00..89b94199236bc 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -362,8 +362,7 @@ bool Attribute::isConstantRangeListAttribute() const { Attribute::AttrKind Attribute::getKindAsEnum() const { if (!pImpl) return None; - assert((isEnumAttribute() || isIntAttribute() || isTypeAttribute() || - isConstantRangeAttribute() || isConstantRangeListAttribute()) && + assert(hasKindAsEnum() && "Invalid attribute type to get the kind as an enum!"); return pImpl->getKindAsEnum(); } @@ -712,6 +711,16 @@ bool Attribute::hasParentContext(LLVMContext &C) const { return C.pImpl->AttrsSet.FindNodeOrInsertPos(ID, Unused) == pImpl; } +int Attribute::cmpKind(Attribute A) const { + if (!pImpl && !A.pImpl) + return 0; + if (!pImpl) + return 1; + if (!A.pImpl) + return -1; + return pImpl->cmp(*A.pImpl, /*KindOnly=*/true); +} + bool Attribute::operator<(Attribute A) const { if (!pImpl && !A.pImpl) return false; if (!pImpl) return true; @@ -727,19 +736,25 @@ enum AttributeProperty { FnAttr = (1 << 0), ParamAttr = (1 << 1), RetAttr = (1 << 2), + IntersectPreserve = (0 << 3), IntersectAnd = (1 << 3), - IntersectMin = (1 << 4), - IntersectCustom = (1 << 5), + IntersectMin = (2 << 3), + IntersectCustom = (3 << 3), + IntersectPropertyMask = (3 << 3), }; #define GET_ATTR_PROP_TABLE #include "llvm/IR/Attributes.inc" -static bool hasAttributeProperty(Attribute::AttrKind Kind, - AttributeProperty Prop) { +static unsigned getAttributeProperties(Attribute::AttrKind Kind) { unsigned Index = Kind - 1; assert(Index < std::size(AttrPropTable) && "Invalid attribute kind"); - return AttrPropTable[Index] & Prop; + return AttrPropTable[Index]; +} + +static bool hasAttributeProperty(Attribute::AttrKind Kind, + AttributeProperty Prop) { + return getAttributeProperties(Kind) & Prop; } bool Attribute::canUseAsFnAttr(AttrKind Kind) { @@ -754,14 +769,28 @@ bool Attribute::canUseAsRetAttr(AttrKind Kind) { return hasAttributeProperty(Kind, AttributeProperty::RetAttr); } +static bool hasIntersectProperty(Attribute::AttrKind Kind, + AttributeProperty Prop) { + assert(Prop == AttributeProperty::IntersectPreserve || + Prop == AttributeProperty::IntersectAnd || + Prop == AttributeProperty::IntersectMin || + Prop == AttributeProperty::IntersectCustom && + "Unknown intersect property"); + return (getAttributeProperties(Kind) & + AttributeProperty::IntersectPropertyMask) == Prop; +} + +bool Attribute::intersectMustPreserve(AttrKind Kind) { + return hasIntersectProperty(Kind, AttributeProperty::IntersectPreserve); +} bool Attribute::intersectWithAnd(AttrKind Kind) { - return hasAttributeProperty(Kind, AttributeProperty::IntersectAnd); + return hasIntersectProperty(Kind, AttributeProperty::IntersectAnd); } bool Attribute::intersectWithMin(AttrKind Kind) { - return hasAttributeProperty(Kind, AttributeProperty::IntersectMin); + return hasIntersectProperty(Kind, AttributeProperty::IntersectMin); } bool Attribute::intersectWithCustom(AttrKind Kind) { - return hasAttributeProperty(Kind, AttributeProperty::IntersectCustom); + return hasIntersectProperty(Kind, AttributeProperty::IntersectCustom); } //===----------------------------------------------------------------------===// @@ -821,17 +850,21 @@ ArrayRef AttributeImpl::getValueAsConstantRangeList() const { ->getConstantRangeListValue(); } -bool AttributeImpl::operator<(const AttributeImpl &AI) const { +int AttributeImpl::cmp(const AttributeImpl &AI, bool KindOnly) const { if (this == &AI) - return false; + return 0; // This sorts the attributes with Attribute::AttrKinds coming first (sorted // relative to their enum value) and then strings. if (!isStringAttribute()) { if (AI.isStringAttribute()) - return true; + return -1; + if (getKindAsEnum() != AI.getKindAsEnum()) - return getKindAsEnum() < AI.getKindAsEnum(); + return getKindAsEnum() < AI.getKindAsEnum() ? -1 : 1; + else if (KindOnly) + return 0; + assert(!AI.isEnumAttribute() && "Non-unique attribute"); assert(!AI.isTypeAttribute() && "Comparison of types would be unstable"); assert(!AI.isConstantRangeAttribute() && "Unclear how to compare ranges"); @@ -839,14 +872,21 @@ bool AttributeImpl::operator<(const AttributeImpl &AI) const { "Unclear how to compare range list"); // TODO: Is this actually needed? assert(AI.isIntAttribute() && "Only possibility left"); - return getValueAsInt() < AI.getValueAsInt(); + if (getValueAsInt() < AI.getValueAsInt()) + return -1; + return getValueAsInt() == AI.getValueAsInt() ? 0 : 1; } - if (!AI.isStringAttribute()) - return false; + return 1; + if (KindOnly) + return getKindAsString().compare(AI.getKindAsString()); if (getKindAsString() == AI.getKindAsString()) - return getValueAsString() < AI.getValueAsString(); - return getKindAsString() < AI.getKindAsString(); + return getValueAsString().compare(AI.getValueAsString()); + return getKindAsString().compare(AI.getKindAsString()); +} + +bool AttributeImpl::operator<(const AttributeImpl &AI) const { + return cmp(AI, /*KindOnly=*/false) < 0; } //===----------------------------------------------------------------------===// @@ -918,24 +958,34 @@ AttributeSet AttributeSet::removeAttributes(LLVMContext &C, std::optional AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { + // return std::nullopt; if (*this == Other) return *this; AttrBuilder Intersected(C); - // Get common set of all attributes then handle each attr according to its - // intersect rule. - AttributeSet Merged = addAttributes(C, Other); - for (Attribute Attr : Merged) { - // Only supporting enum attrs for now. - if (!Attr.hasKindAsEnum()) - return std::nullopt; - Attribute::AttrKind Kind = Attr.getKindAsEnum(); - std::optional Attr0, Attr1; - if (hasAttribute(Kind)) - Attr0 = getAttribute(Kind); - if (Other.hasAttribute(Kind)) - Attr1 = Other.getAttribute(Kind); + // Iterate over both attr sets at once. + auto ItBegin0 = begin(); + auto ItEnd0 = end(); + auto ItBegin1 = Other.begin(); + auto ItEnd1 = Other.end(); + while (ItBegin0 != ItEnd0 || ItBegin1 != ItEnd1) { + std::optional Attr0, Attr1; + if (ItBegin1 == ItEnd1) + Attr0 = *ItBegin0++; + else if (ItBegin0 == ItEnd0) + Attr1 = *ItBegin1++; + else { + int Cmp = ItBegin0->cmpKind(*ItBegin1); + if (Cmp == 0) { + Attr0 = *ItBegin0++; + Attr1 = *ItBegin1++; + } else if (Cmp < 0) + Attr0 = *ItBegin0++; + else + Attr1 = *ItBegin1++; + } + Attribute Attr = Attr0 ? *Attr0 : *Attr1; auto IntersectEq = [&]() { if (!Attr0 || !Attr1) return false; @@ -945,56 +995,61 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { return true; }; + if (!Attr.hasKindAsEnum()) { + if (!IntersectEq()) + return std::nullopt; + continue; + } + + Attribute::AttrKind Kind = Attr.getKindAsEnum(); + bool BothValid = Attr0 && Attr1 && Attr0->isValid() && Attr1->isValid(); // Attribute we can intersect with "and" if (Attribute::intersectWithAnd(Kind)) { assert(Attribute::isEnumAttrKind(Kind) && "Invalid attr type of intersectAnd"); - if (Attr0 && Attr1) + if (BothValid) Intersected.addAttribute(Kind); continue; } - bool BothValid = Attr0 && Attr1 && Attr0->isValid() && Attr1->isValid(); // Attribute we can intersect with "min" if (Attribute::intersectWithMin(Kind)) { assert(Attribute::isIntAttrKind(Kind) && "Invalid attr type of intersectMin"); - if (!BothValid) - continue; - - uint64_t NewVal = std::min(getAttribute(Kind).getValueAsInt(), - Other.getAttribute(Kind).getValueAsInt()); - Intersected.addRawIntAttr(Kind, NewVal); + if (BothValid) { + uint64_t NewVal = + std::min(Attr0->getValueAsInt(), Attr1->getValueAsInt()); + Intersected.addRawIntAttr(Kind, NewVal); + } continue; } // Attribute we can intersect but need a custom rule for. if (Attribute::intersectWithCustom(Kind)) { - switch (Kind) { - case Attribute::Alignment: - if (BothValid) - Intersected.addAlignmentAttr(std::min( - getAlignment().valueOrOne(), Other.getAlignment().valueOrOne())); - break; - case Attribute::Memory: - if (BothValid) - Intersected.addMemoryAttr(getMemoryEffects() | - Other.getMemoryEffects()); - break; - case Attribute::NoFPClass: - if (BothValid) - Intersected.addNoFPClassAttr(getNoFPClass() & Other.getNoFPClass()); - break; - case Attribute::Range: - if (BothValid) { - ConstantRange Range0 = getAttribute(Kind).getRange(); - ConstantRange Range1 = Other.getAttribute(Kind).getRange(); + if (BothValid) { + switch (Kind) { + case Attribute::Alignment: + Intersected.addAlignmentAttr( + std::min(Attr0->getAlignment().valueOrOne(), + Attr1->getAlignment().valueOrOne())); + break; + case Attribute::Memory: + Intersected.addMemoryAttr(Attr0->getMemoryEffects() | + Attr1->getMemoryEffects()); + break; + case Attribute::NoFPClass: + Intersected.addNoFPClassAttr(Attr0->getNoFPClass() & + Attr1->getNoFPClass()); + break; + case Attribute::Range: { + ConstantRange Range0 = Attr0->getRange(); + ConstantRange Range1 = Attr1->getRange(); ConstantRange NewRange = Range0.unionWith(Range1); if (!NewRange.isFullSet()) Intersected.addRangeAttr(NewRange); + } break; + default: + llvm_unreachable("Unknown attribute with custom intersection rule"); } - break; - default: - llvm_unreachable("Unknown attribute with custom intersection rule"); } continue; } @@ -1729,19 +1784,21 @@ AttributeList::intersectAttributes(LLVMContext &C, AttributeList Other) const { if (getNumAttrSets() != Other.getNumAttrSets()) return std::nullopt; - AttributeList IntersectedAttrs{}; + SmallVector> IntersectedAttrs; + // AttributeList IntersectedAttrs{}; for (unsigned Idx : indexes()) { - auto IntersectedAB = + auto IntersectedAS = getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx)); // If any index fails to intersect, fail. - if (!IntersectedAB) + if (!IntersectedAS) return std::nullopt; - - IntersectedAttrs = - IntersectedAttrs.setAttributesAtIndex(C, Idx, *IntersectedAB); + if (!IntersectedAS->hasAttributes()) + continue; + IntersectedAttrs.push_back(std::make_pair(Idx, *IntersectedAS)); } - return IntersectedAttrs; + llvm::sort(IntersectedAttrs, llvm::less_first()); + return AttributeList::get(C, IntersectedAttrs); } //===----------------------------------------------------------------------===// diff --git a/llvm/unittests/IR/AttributesTest.cpp b/llvm/unittests/IR/AttributesTest.cpp index 2c22b960e5026..e2f5b29a1ef49 100644 --- a/llvm/unittests/IR/AttributesTest.cpp +++ b/llvm/unittests/IR/AttributesTest.cpp @@ -48,6 +48,8 @@ TEST(Attributes, Ordering) { EXPECT_TRUE(Align4 < Deref4); EXPECT_TRUE(Align4 < Deref5); EXPECT_TRUE(Align5 < Deref4); + EXPECT_EQ(Deref5.cmpKind(Deref4), 0); + EXPECT_EQ(Align4.cmpKind(Align5), 0); Attribute ByVal = Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C)); EXPECT_FALSE(ByVal < Attribute::get(C, Attribute::ZExt)); diff --git a/llvm/utils/TableGen/Attributes.cpp b/llvm/utils/TableGen/Attributes.cpp index 8b79462fc544f..ff5ef94832ee8 100644 --- a/llvm/utils/TableGen/Attributes.cpp +++ b/llvm/utils/TableGen/Attributes.cpp @@ -126,7 +126,7 @@ void Attributes::emitAttributeProperties(raw_ostream &OS) { if (!AllowIntersectAnd && cast(P)->getDef()->getName() == "IntersectAnd") PrintFatalError( - "'IntersectAnd' only compatible with 'EnumAttr' or 'TypeAttr'"); + "'IntersectAnd' only compatible with 'EnumAttr'"); if (!AllowIntersectMin && cast(P)->getDef()->getName() == "IntersectMin") PrintFatalError("'IntersectMin' only compatible with 'IntAttr'"); From 794c30e99602a5d0b8bb15eb5a8bfbffba5c5939 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Wed, 25 Sep 2024 16:10:59 -0500 Subject: [PATCH 04/10] Fixup fmt --- llvm/utils/TableGen/Attributes.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/utils/TableGen/Attributes.cpp b/llvm/utils/TableGen/Attributes.cpp index ff5ef94832ee8..ed00debc398cb 100644 --- a/llvm/utils/TableGen/Attributes.cpp +++ b/llvm/utils/TableGen/Attributes.cpp @@ -125,8 +125,7 @@ void Attributes::emitAttributeProperties(raw_ostream &OS) { for (Init *P : *A->getValueAsListInit("Properties")) { if (!AllowIntersectAnd && cast(P)->getDef()->getName() == "IntersectAnd") - PrintFatalError( - "'IntersectAnd' only compatible with 'EnumAttr'"); + PrintFatalError("'IntersectAnd' only compatible with 'EnumAttr'"); if (!AllowIntersectMin && cast(P)->getDef()->getName() == "IntersectMin") PrintFatalError("'IntersectMin' only compatible with 'IntAttr'"); From 9ac21328bbb25e1aa034e1932afa3b34936ccecf Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Thu, 26 Sep 2024 01:06:16 -0500 Subject: [PATCH 05/10] Fix fmt again --- llvm/lib/IR/Attributes.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 89b94199236bc..9453219e5423b 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -958,11 +958,10 @@ AttributeSet AttributeSet::removeAttributes(LLVMContext &C, std::optional AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { - // return std::nullopt; if (*this == Other) return *this; - AttrBuilder Intersected(C); + AttrBuilder Intersected(C); // Iterate over both attr sets at once. auto ItBegin0 = begin(); auto ItEnd0 = end(); From 381af53990f141d6a0bd7147526774df367ecb6d Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Thu, 26 Sep 2024 17:18:39 -0500 Subject: [PATCH 06/10] Address coments --- llvm/include/llvm/IR/Attributes.h | 2 +- llvm/include/llvm/IR/Attributes.td | 18 ++--- llvm/lib/IR/Attributes.cpp | 102 ++++++++++++++++----------- llvm/unittests/IR/AttributesTest.cpp | 101 ++++++++++++++++++++++---- 4 files changed, 152 insertions(+), 71 deletions(-) diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h index 90981b7ce3fbb..52690594ecc25 100644 --- a/llvm/include/llvm/IR/Attributes.h +++ b/llvm/include/llvm/IR/Attributes.h @@ -797,7 +797,7 @@ class AttributeList { /// the two lists are inherently incompatible (imply different behavior, not /// just analysis). [[nodiscard]] std::optional - intersectAttributes(LLVMContext &C, AttributeList Other) const; + intersectWith(LLVMContext &C, AttributeList Other) const; //===--------------------------------------------------------------------===// // AttributeList Accessors diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td index a0eb050876ec3..6798f8dffc5b1 100644 --- a/llvm/include/llvm/IR/Attributes.td +++ b/llvm/include/llvm/IR/Attributes.td @@ -27,19 +27,11 @@ def RetAttr : AttrProperty; /// Intersection rules. Used for example in sinking/hoisting two /// callbases to find a set of attributes that apply to both. /// Note, there are some attributes we can (probably) legally drop -/// but are intentionally excluded as of now. Those include: -/// - initializes -/// - allockind -/// - allocsize -/// - minsize -/// - optsize -/// - optnone -/// - optdebug -/// - optforfuzzing +/// but are intentionally excluded as of now. /// -/// When intersecting the attributes must both present and equal. -/// Use this for attributes you are not certain it is safe to drop -/// at any time. I.e `byval(Ty)` on a parameter. +/// When intersecting the attributes both must be present and equal. +/// Use this for attributes it is not safe to drop at any time. E.g. +/// `byval(Ty)` on a parameter. def IntersectPreserve : AttrProperty; /// When intersecting take the AND of the two attrs. @@ -123,7 +115,7 @@ def NoUndef : EnumAttr<"noundef", IntersectAnd, [ParamAttr, RetAttr]>; def Cold : EnumAttr<"cold", IntersectAnd, [FnAttr]>; /// Can only be moved to control-equivalent blocks. -def Convergent : EnumAttr<"convergent", IntersectAnd, [FnAttr]>; +def Convergent : EnumAttr<"convergent", IntersectPreserve, [FnAttr]>; /// Marks function as being in a hot path and frequently called. def Hot: EnumAttr<"hot", IntersectAnd, [FnAttr]>; diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 9453219e5423b..99e4c32be6f35 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -969,7 +969,7 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { auto ItEnd1 = Other.end(); while (ItBegin0 != ItEnd0 || ItBegin1 != ItEnd1) { - std::optional Attr0, Attr1; + Attribute Attr0, Attr1; if (ItBegin1 == ItEnd1) Attr0 = *ItBegin0++; else if (ItBegin0 == ItEnd0) @@ -984,30 +984,46 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { else Attr1 = *ItBegin1++; } - Attribute Attr = Attr0 ? *Attr0 : *Attr1; + assert(Attr0.isValid() || + Attr1.isValid() && "Iteration should never yield no valid attrs"); + + // If we don't have both attributes, then fail if the attribute is + // must-preserve or drop it otherwise. + if (!Attr0.isValid() || !Attr1.isValid()) { + Attribute Attr = Attr0.isValid() ? Attr0 : Attr1; + // Non-enum assume we must preserve. + if (!Attr.hasKindAsEnum()) + return std::nullopt; + Attribute::AttrKind Kind = Attr.getKindAsEnum(); + if (Attribute::intersectMustPreserve(Kind)) + return std::nullopt; + continue; + } + // We have both attributes so apply the intersection rule. + auto IntersectEq = [&]() { - if (!Attr0 || !Attr1) - return false; - if (*Attr0 != *Attr1) + if (Attr0 != Attr1) return false; - Intersected.addAttribute(Attr); + Intersected.addAttribute(Attr0); return true; }; - if (!Attr.hasKindAsEnum()) { + // Non-enum assume we must preserve + if (!Attr0.hasKindAsEnum()) { if (!IntersectEq()) return std::nullopt; continue; } - Attribute::AttrKind Kind = Attr.getKindAsEnum(); - bool BothValid = Attr0 && Attr1 && Attr0->isValid() && Attr1->isValid(); + Attribute::AttrKind Kind = Attr0.getKindAsEnum(); + assert(Attr1.hasKindAsEnum() && Kind == Attr1.getKindAsEnum() && + "Iterator picked up two different attributes in the same iteration"); + // Attribute we can intersect with "and" if (Attribute::intersectWithAnd(Kind)) { assert(Attribute::isEnumAttrKind(Kind) && "Invalid attr type of intersectAnd"); - if (BothValid) - Intersected.addAttribute(Kind); + Intersected.addAttribute(Kind); continue; } @@ -1015,40 +1031,35 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { if (Attribute::intersectWithMin(Kind)) { assert(Attribute::isIntAttrKind(Kind) && "Invalid attr type of intersectMin"); - if (BothValid) { - uint64_t NewVal = - std::min(Attr0->getValueAsInt(), Attr1->getValueAsInt()); - Intersected.addRawIntAttr(Kind, NewVal); - } + uint64_t NewVal = std::min(Attr0.getValueAsInt(), Attr1.getValueAsInt()); + Intersected.addRawIntAttr(Kind, NewVal); continue; } // Attribute we can intersect but need a custom rule for. if (Attribute::intersectWithCustom(Kind)) { - if (BothValid) { - switch (Kind) { - case Attribute::Alignment: - Intersected.addAlignmentAttr( - std::min(Attr0->getAlignment().valueOrOne(), - Attr1->getAlignment().valueOrOne())); - break; - case Attribute::Memory: - Intersected.addMemoryAttr(Attr0->getMemoryEffects() | - Attr1->getMemoryEffects()); - break; - case Attribute::NoFPClass: - Intersected.addNoFPClassAttr(Attr0->getNoFPClass() & - Attr1->getNoFPClass()); - break; - case Attribute::Range: { - ConstantRange Range0 = Attr0->getRange(); - ConstantRange Range1 = Attr1->getRange(); - ConstantRange NewRange = Range0.unionWith(Range1); - if (!NewRange.isFullSet()) - Intersected.addRangeAttr(NewRange); - } break; - default: - llvm_unreachable("Unknown attribute with custom intersection rule"); - } + switch (Kind) { + case Attribute::Alignment: + Intersected.addAlignmentAttr( + std::min(Attr0.getAlignment().valueOrOne(), + Attr1.getAlignment().valueOrOne())); + break; + case Attribute::Memory: + Intersected.addMemoryAttr(Attr0.getMemoryEffects() | + Attr1.getMemoryEffects()); + break; + case Attribute::NoFPClass: + Intersected.addNoFPClassAttr(Attr0.getNoFPClass() & + Attr1.getNoFPClass()); + break; + case Attribute::Range: { + ConstantRange Range0 = Attr0.getRange(); + ConstantRange Range1 = Attr1.getRange(); + ConstantRange NewRange = Range0.unionWith(Range1); + if (!NewRange.isFullSet()) + Intersected.addRangeAttr(NewRange); + } break; + default: + llvm_unreachable("Unknown attribute with custom intersection rule"); } continue; } @@ -1057,6 +1068,13 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { // Otherwise fail. if (!IntersectEq()) return std::nullopt; + + // Special handling of `byval`. `byval` essentially turns align attr into + // must-preserve + if (Kind == Attribute::ByVal && + getAttribute(Attribute::Alignment) != + Other.getAttribute(Attribute::Alignment)) + return std::nullopt; } return get(C, Intersected); @@ -1774,7 +1792,7 @@ AttributeList AttributeList::addAllocSizeParamAttr( } std::optional -AttributeList::intersectAttributes(LLVMContext &C, AttributeList Other) const { +AttributeList::intersectWith(LLVMContext &C, AttributeList Other) const { // Trivial case, the two lists are equal. if (*this == Other) return *this; diff --git a/llvm/unittests/IR/AttributesTest.cpp b/llvm/unittests/IR/AttributesTest.cpp index e2f5b29a1ef49..88946a6f19dba 100644 --- a/llvm/unittests/IR/AttributesTest.cpp +++ b/llvm/unittests/IR/AttributesTest.cpp @@ -524,6 +524,77 @@ TEST(Attributes, SetIntersect) { } } +TEST(Attributes, SetIntersectByValAlign) { + LLVMContext C; + AttributeSet AS0, AS1; + + Attribute ByVal = Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C)); + Attribute Align0 = Attribute::get(C, Attribute::Alignment, 4); + Attribute Align1 = Attribute::get(C, Attribute::Alignment, 8); + + { + AttrBuilder AB0(C), AB1(C); + AB0.addAttribute(Align0); + AB1.addAttribute(Align1); + AB0.addAttribute(Attribute::NoUndef); + AS0 = AttributeSet::get(C, AB0); + AS1 = AttributeSet::get(C, AB1); + auto Res = AS0.intersectWith(C, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttribute(Attribute::Alignment)); + } + { + AttrBuilder AB0(C), AB1(C); + AB0.addAttribute(Align0); + AB0.addAttribute(ByVal); + AB1.addAttribute(Align1); + AB1.addAttribute(ByVal); + AB0.addAttribute(Attribute::NoUndef); + AS0 = AttributeSet::get(C, AB0); + AS1 = AttributeSet::get(C, AB1); + auto Res = AS0.intersectWith(C, AS1); + ASSERT_FALSE(Res.has_value()); + } + { + AttrBuilder AB0(C), AB1(C); + AB0.addAttribute(Align0); + AB0.addAttribute(ByVal); + AB1.addAttribute(ByVal); + AB0.addAttribute(Attribute::NoUndef); + AS0 = AttributeSet::get(C, AB0); + AS1 = AttributeSet::get(C, AB1); + ASSERT_FALSE(AS0.intersectWith(C, AS1).has_value()); + ASSERT_FALSE(AS1.intersectWith(C, AS0).has_value()); + } + { + AttrBuilder AB0(C), AB1(C); + AB0.addAttribute(ByVal); + AB1.addAttribute(ByVal); + AB0.addAttribute(Attribute::NoUndef); + AS0 = AttributeSet::get(C, AB0); + AS1 = AttributeSet::get(C, AB1); + + auto Res = AS0.intersectWith(C, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttribute(Attribute::ByVal)); + } + { + AttrBuilder AB0(C), AB1(C); + AB0.addAttribute(ByVal); + AB0.addAttribute(Align0); + AB1.addAttribute(ByVal); + AB1.addAttribute(Align0); + AB0.addAttribute(Attribute::NoUndef); + AS0 = AttributeSet::get(C, AB0); + AS1 = AttributeSet::get(C, AB1); + + auto Res = AS0.intersectWith(C, AS1); + ASSERT_TRUE(Res.has_value()); + ASSERT_TRUE(Res->hasAttribute(Attribute::ByVal)); + ASSERT_TRUE(Res->hasAttribute(Attribute::Alignment)); + } +} + TEST(Attributes, ListIntersect) { LLVMContext C; AttributeList AL0; @@ -532,26 +603,26 @@ TEST(Attributes, ListIntersect) { AL0 = AL0.addRetAttribute(C, Attribute::NoUndef); AL1 = AL1.addRetAttribute(C, Attribute::NoUndef); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_EQ(AL0, *Res); AL0 = AL0.addParamAttribute(C, 1, Attribute::NoUndef); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_FALSE(Res.has_value()); AL1 = AL1.addParamAttribute(C, 2, Attribute::NoUndef); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_FALSE(Res.has_value()); AL0 = AL0.addParamAttribute(C, 2, Attribute::NoUndef); AL1 = AL1.addParamAttribute(C, 1, Attribute::NoUndef); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_EQ(AL0, *Res); AL0 = AL0.addParamAttribute(C, 2, Attribute::NonNull); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_NE(AL0, *Res); ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); @@ -560,7 +631,7 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); AL0 = AL0.addRetAttribute(C, Attribute::NonNull); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_NE(AL0, *Res); ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef)); @@ -570,7 +641,7 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); AL0 = AL0.addFnAttribute(C, Attribute::ReadOnly); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_NE(AL0, *Res); ASSERT_FALSE(Res->hasFnAttr(Attribute::ReadOnly)); @@ -581,7 +652,7 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); AL1 = AL1.addFnAttribute(C, Attribute::ReadOnly); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_NE(AL0, *Res); ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); @@ -592,11 +663,11 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); AL1 = AL1.addFnAttribute(C, Attribute::AlwaysInline); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_FALSE(Res.has_value()); AL0 = AL0.addFnAttribute(C, Attribute::AlwaysInline); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); @@ -607,7 +678,7 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull)); AL1 = AL1.addParamAttribute(C, 2, Attribute::ReadNone); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); @@ -619,11 +690,11 @@ TEST(Attributes, ListIntersect) { ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone)); AL1 = AL1.addParamAttribute(C, 3, Attribute::ReadNone); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_FALSE(Res.has_value()); AL0 = AL0.addParamAttribute(C, 3, Attribute::ReadNone); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); @@ -637,12 +708,12 @@ TEST(Attributes, ListIntersect) { AL0 = AL0.addParamAttribute( C, {3}, Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C))); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_FALSE(Res.has_value()); AL1 = AL1.addParamAttribute( C, {3}, Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C))); - Res = AL0.intersectAttributes(C, AL1); + Res = AL0.intersectWith(C, AL1); ASSERT_TRUE(Res.has_value()); ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline)); ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly)); From 484a4bd608281132531d8371bb747af44c818d5e Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Thu, 26 Sep 2024 17:20:44 -0500 Subject: [PATCH 07/10] Missing comments --- llvm/include/llvm/IR/Attributes.td | 1 + llvm/lib/IR/Attributes.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td index 6798f8dffc5b1..d05a6ca92aaba 100644 --- a/llvm/include/llvm/IR/Attributes.td +++ b/llvm/include/llvm/IR/Attributes.td @@ -115,6 +115,7 @@ def NoUndef : EnumAttr<"noundef", IntersectAnd, [ParamAttr, RetAttr]>; def Cold : EnumAttr<"cold", IntersectAnd, [FnAttr]>; /// Can only be moved to control-equivalent blocks. +/// NB: Could be IntersectCustom with "or" handling. def Convergent : EnumAttr<"convergent", IntersectPreserve, [FnAttr]>; /// Marks function as being in a hot path and frequently called. diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 99e4c32be6f35..3f3e313ac0305 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -1039,6 +1039,8 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { if (Attribute::intersectWithCustom(Kind)) { switch (Kind) { case Attribute::Alignment: + // If `byval` is present, alignment become must-preserve. This is + // handled below if we have `byval`. Intersected.addAlignmentAttr( std::min(Attr0.getAlignment().valueOrOne(), Attr1.getAlignment().valueOrOne())); From a68e01522710370155ad7998e66499408f3b0ea8 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Tue, 1 Oct 2024 08:58:16 -0500 Subject: [PATCH 08/10] Make iteration clearer --- llvm/lib/IR/Attributes.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 3f3e313ac0305..681e06b49b30f 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -969,6 +969,9 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { auto ItEnd1 = Other.end(); while (ItBegin0 != ItEnd0 || ItBegin1 != ItEnd1) { + // Loop through all attributes in both this and Other in sorted order. If + // the attribute is only present in one of the sets, it will be set in + // Attr0. If it is present in both sets both Attr0 and Attr1 will be set. Attribute Attr0, Attr1; if (ItBegin1 == ItEnd1) Attr0 = *ItBegin0++; @@ -982,7 +985,7 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { } else if (Cmp < 0) Attr0 = *ItBegin0++; else - Attr1 = *ItBegin1++; + Attr0 = *ItBegin1++; } assert(Attr0.isValid() || Attr1.isValid() && "Iteration should never yield no valid attrs"); @@ -990,11 +993,10 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { // If we don't have both attributes, then fail if the attribute is // must-preserve or drop it otherwise. if (!Attr0.isValid() || !Attr1.isValid()) { - Attribute Attr = Attr0.isValid() ? Attr0 : Attr1; // Non-enum assume we must preserve. - if (!Attr.hasKindAsEnum()) + if (!Attr0.hasKindAsEnum()) return std::nullopt; - Attribute::AttrKind Kind = Attr.getKindAsEnum(); + Attribute::AttrKind Kind = Attr0.getKindAsEnum(); if (Attribute::intersectMustPreserve(Kind)) return std::nullopt; continue; From 200ddaade3d2383aa0e984365c3b7fc914a225aa Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Tue, 1 Oct 2024 09:14:37 -0500 Subject: [PATCH 09/10] Cleanup further --- llvm/lib/IR/Attributes.cpp | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 681e06b49b30f..84dd1ef1362ea 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -976,7 +976,7 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { if (ItBegin1 == ItEnd1) Attr0 = *ItBegin0++; else if (ItBegin0 == ItEnd0) - Attr1 = *ItBegin1++; + Attr0 = *ItBegin1++; else { int Cmp = ItBegin0->cmpKind(*ItBegin1); if (Cmp == 0) { @@ -987,30 +987,19 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { else Attr0 = *ItBegin1++; } - assert(Attr0.isValid() || - Attr1.isValid() && "Iteration should never yield no valid attrs"); - - // If we don't have both attributes, then fail if the attribute is - // must-preserve or drop it otherwise. - if (!Attr0.isValid() || !Attr1.isValid()) { - // Non-enum assume we must preserve. - if (!Attr0.hasKindAsEnum()) - return std::nullopt; - Attribute::AttrKind Kind = Attr0.getKindAsEnum(); - if (Attribute::intersectMustPreserve(Kind)) - return std::nullopt; - continue; - } - // We have both attributes so apply the intersection rule. + assert(Attr0.isValid() && "Iteration should always yield a valid attr"); auto IntersectEq = [&]() { + if (!Attr1.isValid()) + return false; if (Attr0 != Attr1) return false; Intersected.addAttribute(Attr0); return true; }; - // Non-enum assume we must preserve + // Non-enum assume we must preserve. Handle early so we can unconditionally + // use Kind below. if (!Attr0.hasKindAsEnum()) { if (!IntersectEq()) return std::nullopt; @@ -1018,6 +1007,15 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const { } Attribute::AttrKind Kind = Attr0.getKindAsEnum(); + // If we don't have both attributes, then fail if the attribute is + // must-preserve or drop it otherwise. + if (!Attr1.isValid()) { + if (Attribute::intersectMustPreserve(Kind)) + return std::nullopt; + continue; + } + + // We have both attributes so apply the intersection rule. assert(Attr1.hasKindAsEnum() && Kind == Attr1.getKindAsEnum() && "Iterator picked up two different attributes in the same iteration"); From 6652140ee71c71a24f34a11c467f258651aeab14 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Tue, 1 Oct 2024 10:14:11 -0500 Subject: [PATCH 10/10] Remove stray comment --- llvm/lib/IR/Attributes.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 84dd1ef1362ea..9fb7b4a8f7be1 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -1804,7 +1804,6 @@ AttributeList::intersectWith(LLVMContext &C, AttributeList Other) const { return std::nullopt; SmallVector> IntersectedAttrs; - // AttributeList IntersectedAttrs{}; for (unsigned Idx : indexes()) { auto IntersectedAS = getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx));