[llvm-branch-commits] [llvm] Attributor: Add only new attributes when manifesting (PR #234241)
Matt Arsenault via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Oct 21 15:05:48 PDT 2026
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/234241
>From e86e0daae243b2c9a6411ec94573c2dac28f79d4 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Tue, 20 Oct 2026 12:19:45 -0700
Subject: [PATCH] Attributor: Add only new attributes when manifesting
manifestAttrs collected the attributes to add in an AttrBuilder, then
copied the position's attribute set into another AttrBuilder to merge
them in. Collect them in an array and merge them into the set in one
pass. This requires the attributes passed to manifestAttrs to be sorted
like an attribute set, which all callers already do. The only one
passing more than one attribute is AMDGPUAttributor, whose implicit
argument attributes are sorted.
Instructions spent in amdgpu-attributor:
rocPRIM test_device_scan, gfx942: 770.2M -> 710.8M (-7.7%)
Blender Cycles HIP kernel, gfx1100: 380.0M -> 379.5M (-0.1%)
rocPRIM 16 tests, full LTO, closed world: 3019.8M -> 2855.9M (-5.4%)
Co-authored-by: Claude Opus 5.5 <noreply at anthropic.com>
---
llvm/include/llvm/Transforms/IPO/Attributor.h | 4 +-
llvm/lib/Transforms/IPO/Attributor.cpp | 101 ++++++++++--------
2 files changed, 61 insertions(+), 44 deletions(-)
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 67bc27f6a2d1f7..fe94fb8aeea2b2 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -1910,7 +1910,9 @@ struct Attributor {
ArrayRef<StringRef> Attrs);
/// Attach \p DeducedAttrs to \p IRP, if \p ForceReplace is set we do this
- /// even if the same attribute kind was already present.
+ /// even if the same attribute kind was already present. \p DeducedAttrs
+ /// must be sorted like an attribute set, with at most one attribute of each
+ /// kind.
LLVM_ABI ChangeStatus manifestAttrs(const IRPosition &IRP,
ArrayRef<Attribute> DeducedAttrs,
bool ForceReplace = false);
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 7f7b1de467f170..df132d1342f26c 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1002,51 +1002,36 @@ static bool isEqualOrWorse(const Attribute &New, const Attribute &Old) {
return Old.getValueAsInt() >= New.getValueAsInt();
}
-/// Return true if the information provided by \p Attr was added to the
-/// attribute set \p AttrSet. This is only the case if it was not already
-/// present in \p AttrSet.
-static bool addIfNotExistent(LLVMContext &Ctx, const Attribute &Attr,
- AttributeSet AttrSet, bool ForceReplace,
- AttrBuilder &AB) {
-
- if (Attr.isEnumAttribute()) {
- Attribute::AttrKind Kind = Attr.getKindAsEnum();
- if (AttrSet.hasAttribute(Kind))
- return false;
- AB.addAttribute(Kind);
- return true;
- }
+/// Return the attribute to add to a position to provide the information of
+/// \p Attr, given the attribute \p Existing of the same kind at the position
+/// (or none), or an invalid attribute if \p Existing already provides it.
+static Attribute getAttrToAdd(LLVMContext &Ctx, Attribute Attr,
+ Attribute Existing, bool ForceReplace) {
+ if (Attr.isEnumAttribute())
+ return Existing.isValid() ? Attribute() : Attr;
if (Attr.isStringAttribute()) {
- Attribute Existing = AttrSet.getAttribute(Attr.getKindAsString());
if (Existing.isValid() && (!ForceReplace || Existing == Attr))
- return false;
- AB.addAttribute(Attr);
- return true;
+ return Attribute();
+ return Attr;
}
+
if (Attr.isIntAttribute()) {
- Attribute::AttrKind Kind = Attr.getKindAsEnum();
- if (!ForceReplace && Kind == Attribute::Memory) {
- MemoryEffects ME = Attr.getMemoryEffects() & AttrSet.getMemoryEffects();
- if (ME == AttrSet.getMemoryEffects())
- return false;
- AB.addMemoryAttr(ME);
- return true;
- }
- if (AttrSet.hasAttribute(Kind)) {
- if (!ForceReplace && isEqualOrWorse(Attr, AttrSet.getAttribute(Kind)))
- return false;
+ if (!ForceReplace && Attr.getKindAsEnum() == Attribute::Memory) {
+ MemoryEffects OldME = Existing.isValid() ? Existing.getMemoryEffects()
+ : MemoryEffects::unknown();
+ MemoryEffects ME = Attr.getMemoryEffects() & OldME;
+ if (ME == OldME)
+ return Attribute();
+ return Attribute::getWithMemoryEffects(Ctx, ME);
}
- AB.addAttribute(Attr);
- return true;
- }
- if (Attr.isConstantRangeAttribute()) {
- Attribute::AttrKind Kind = Attr.getKindAsEnum();
- if (!ForceReplace && AttrSet.hasAttribute(Kind))
- return false;
- AB.addAttribute(Attr);
- return true;
+ if (Existing.isValid() && !ForceReplace && isEqualOrWorse(Attr, Existing))
+ return Attribute();
+ return Attr;
}
+ if (Attr.isConstantRangeAttribute())
+ return !ForceReplace && Existing.isValid() ? Attribute() : Attr;
+
llvm_unreachable("Expected enum or string attribute!");
}
@@ -1303,12 +1288,42 @@ ChangeStatus Attributor::removeAttrs(const IRPosition &IRP,
ChangeStatus Attributor::manifestAttrs(const IRPosition &IRP,
ArrayRef<Attribute> Attrs,
bool ForceReplace) {
+ if (Attrs.empty())
+ return ChangeStatus::UNCHANGED;
+
+ switch (IRP.getPositionKind()) {
+ case IRPosition::IRP_FLOAT:
+ case IRPosition::IRP_INVALID:
+ return ChangeStatus::UNCHANGED;
+ default:
+ break;
+ }
+
+ AttributeList AL = IRP.getAttrList();
+ auto [Iter, Inserted] = AttrsMap.insert({IRP.getAttrListAnchor(), AL});
+ if (!Inserted)
+ AL = Iter->second;
+
LLVMContext &Ctx = IRP.getAnchorValue().getContext();
- auto AddAttrCB = [&](const Attribute &Attr, AttributeSet AttrSet,
- AttributeMask &, AttrBuilder &AB) {
- return addIfNotExistent(Ctx, Attr, AttrSet, ForceReplace, AB);
- };
- return updateAttrMap<Attribute>(IRP, Attrs, AddAttrCB);
+ auto AttrIdx = IRP.getAttrIdx();
+ AttributeSet AS = AL.getAttributes(AttrIdx);
+ SmallVector<Attribute, 8> Added;
+ for (Attribute Attr : Attrs) {
+ Attribute Existing = Attr.isStringAttribute()
+ ? AS.getAttribute(Attr.getKindAsString())
+ : AS.getAttribute(Attr.getKindAsEnum());
+ if (Attribute New = getAttrToAdd(Ctx, Attr, Existing, ForceReplace);
+ New.isValid())
+ Added.push_back(New);
+ }
+
+ if (Added.empty())
+ return ChangeStatus::UNCHANGED;
+
+ // Added keeps the order of Attrs, so it is sorted with distinct kinds too.
+ AL = AL.addAttributesAtIndex(Ctx, AttrIdx, Added);
+ Iter->second = AL;
+ return ChangeStatus::CHANGED;
}
SubsumingPositionIterator::SubsumingPositionIterator(const IRPosition &IRP) {
More information about the llvm-branch-commits
mailing list