[IR][Attribute] Add support for intersecting AttributeLists; NFC #109719
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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/pr-subscribers-tablegen @llvm/pr-subscribers-llvm-ir Author: None (goldsteinn) ChangesAdd support for taking the intersection of two AttributeLists s.t the i.e if we have Further it handles attributes that are not-droppable. For example The motivation for the infrastructure is to enable sinking/hoisting Patch is 29.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109719.diff 5 Files Affected:
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index 5a80a072dbbd27..ac0b00e2ccddbe 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<AttributeSet>
+ 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<unsigned> &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<AttributeList>
+ 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 9044d127b47946..ca24627912f27f 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<string S, list<AttrProperty> P> {
// String representation of this attribute in the IR.
@@ -54,17 +85,17 @@ class ConstantRangeListAttr<string S, list<AttrProperty> P> : Attr<S, P>;
/// 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 fa124e46483dce..2e54ef1aa3e00c 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>
+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<Attribute> 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>
+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 da72fa14510cbe..2c22b960e50265 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<AttributeSet> Res;
+ auto BuildAttr = [&](LLVMContext &C, Attribute::AttrKind Kind, uint64_t Int,
+ Type *Ty, ConstantRange &CR,
+ ArrayRef<ConstantRange> 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<Attribute::AttrKind>(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<ConstantRange> CRL0...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
andjo403
reviewed
Sep 24, 2024
andjo403
reviewed
Sep 24, 2024
nikic
reviewed
Sep 24, 2024
jvoung
reviewed
Sep 24, 2024
llvm/lib/IR/Attributes.cpp
Outdated
| Intersected.addAttribute(Kind); | ||
| continue; | ||
| } | ||
| bool BothValid = Attr0 && Attr1 && Attr0->isValid() && Attr1->isValid(); |
There was a problem hiding this comment.
Is there a reason BothValid is only computed after handling intersectWithAnd?
Separately, it looks like getAttribute(Kind) will return a not isValid() Attribute if not hasAttribute(Kind), so could the layer of std::optional<> be avoided and just check isValid() (and just call getAttribute without checking hasAttribute first)?
There was a problem hiding this comment.
Is there a reason BothValid is only computed after handling intersectWithAnd?
intersectWithAnd doesn't explicitly need to deref pImpl so it wasn't strictly necessary. I've moved it before.
Separately, it looks like getAttribute(Kind) will return a not isValid() Attribute if not hasAttribute(Kind), so could the layer of std::optional<> be avoided and just check isValid() (and just call getAttribute without checking hasAttribute first)?
After dropping the merged list iteration, the getAttribute calls have been dropped so this is a non-issue now.
nikic
reviewed
Sep 26, 2024
|
ping |
nikic
reviewed
Oct 1, 2024
llvm/lib/IR/Attributes.cpp
Outdated
| return std::nullopt; | ||
| continue; | ||
| } | ||
| // We have both attributes so apply the intersection rule. |
There was a problem hiding this comment.
Instead of calling the IntersectEq helper, can we handle the Attr0 == Attr1 case upfront here, and then know that the attributes differ in all following code?
There was a problem hiding this comment.
We could do that although I think it complicates the code. We still need all the unequal handling in intersectMin/interesectCustom so it doesn't really save any codes and the ByVal special case becomes more complicated.
| assert(Attr1.hasKindAsEnum() && Kind == Attr1.getKindAsEnum() && | ||
| "Iterator picked up two different attributes in the same iteration"); | ||
|
|
||
| // Attribute we can intersect with "and" |
There was a problem hiding this comment.
We could also extract Attribute::intersectWith for intersection of same enum attributes to break this down further, but not sure it's really worthwhile.
There was a problem hiding this comment.
Don't think its worthwhile. I am going to move up the string attr handling so we only need to handle that in one place though.
nikic
reviewed
Oct 1, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/6755 Here is the relevant piece of the build log for the reference |
vitalybuka
reviewed
Oct 2, 2024
| V1 = (2 - (i & 2)) + 1; | ||
| } | ||
|
|
||
| ConstantRange CR0(APInt(32, 0), APInt(32, 10)); |
There was a problem hiding this comment.
This creates array ref to dead temps
There was a problem hiding this comment.
Sorry about that and thank you for the prompt fix.
vitalybuka
added a commit
that referenced
this pull request
Oct 2, 2024
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
…m#109719) 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.
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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) noundefintersected withnonnull align(16) dereferenceable(10), the result isnonnull align(16).Further it handles attributes that are not-droppable. For example
dropping
byvalcan change the nature of a callsite/function so itsimpossible to correct a correct intersection if its dropped from the
result. i.e
nonnull byval(i64)intersected withnonnullisinvalid.
The motivation for the infrastructure is to enable sinking/hoisting
callsites with differing attributes.