IR: Remove uselist for constantdata #134692
Open
Conversation
This file contains 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
This was referenced Apr 7, 2025
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
arsenm
changed the title
arsenm
force-pushed
the
users/arsenm/scev-expander/do-not-look-at-uses-of-constants
branch
from
f543f05 to
aedd9ec
Compare
arsenm
force-pushed
the
users/arsenm/ir/remove-uselist-for-constantdata
branch
from
f79a8b0 to
01b6438
Compare
|
IROutliner patch: #134850 |
arsenm
force-pushed
the
users/arsenm/scev-expander/do-not-look-at-uses-of-constants
branch
from
aedd9ec to
30c317d
Compare
arsenm
force-pushed
the
users/arsenm/ir/remove-uselist-for-constantdata
branch
from
01b6438 to
9313a3e
Compare
This is a resurrected version of the patch attached to this RFC: https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606 In this adaptation, there are a few differences. In the original patch, the Use's use list was replaced with an unsigned* to the reference count in the value. This version leaves them as null and leaves the ref counting only in Value. Remove use-lists from instances of ConstantData (which are shared across modules and have no operands). To continue supporting most of the use-list API, store a ref-count in place of the use-list; this is for API like Value::use_empty and Value::hasNUses. Operations that actually need the use-list -- like Value::use_begin -- will assert. This change has three benefits: 1. The compiler output cannot in any way depend on the use-list order of instances of ConstantData. 2. There's no use-list traffic when adding and removing simple constants from operand lists (although there is ref-count traffic; YMMV). 3. It's cheaper to serialize use-lists (since we're no longer serializing the use-list order of things like i32 0). The downside is that you can't look at all the users of ConstantData, but traversals of users of i32 0 are already ill-advised. Possible follow-ups: - Track if an instance of a ConstantVector/ConstantArray/etc. is known to have all ConstantData arguments, and drop the use-lists to ref-counts in those cases. Callers need to check Value::hasUseList before iterating through the use-list. - Remove even the ref-counts. I'm not sure they have any benefit besides minimizing the scope of this commit, and maintaining the counts is not free. Fixes #58629
arsenm
force-pushed
the
users/arsenm/scev-expander/do-not-look-at-uses-of-constants
branch
from
30c317d to
36377c3
Compare
arsenm
force-pushed
the
users/arsenm/ir/remove-uselist-for-constantdata
branch
from
9313a3e to
21970c5
Compare
|
@llvm/pr-subscribers-backend-spir-v @llvm/pr-subscribers-llvm-ir Author: Matt Arsenault (arsenm) ChangesThis is a resurrected version of the patch attached to this RFC: https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606 In this adaptation, there are a few differences. In the original patch, the Use's Remove use-lists from instances of ConstantData (which are shared To continue supporting most of the use-list API, store a ref-count in This change has three benefits:
The downside is that you can't look at all the users of ConstantData, Possible follow-ups:
Fixes #58629 Patch is 29.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134692.diff 24 Files Affected:
diff --git a/llvm/include/llvm/IR/Use.h b/llvm/include/llvm/IR/Use.h
index a86b9c46c1f69..bcd1fd6677497 100644
--- a/llvm/include/llvm/IR/Use.h
+++ b/llvm/include/llvm/IR/Use.h
@@ -23,6 +23,7 @@
namespace llvm {
template <typename> struct simplify_type;
+class ConstantData;
class User;
class Value;
@@ -42,10 +43,7 @@ class Use {
private:
/// Destructor - Only for zap()
- ~Use() {
- if (Val)
- removeFromList();
- }
+ ~Use();
/// Constructor
Use(User *Parent) : Parent(Parent) {}
@@ -87,19 +85,10 @@ class Use {
Use **Prev = nullptr;
User *Parent = nullptr;
- void addToList(Use **List) {
- Next = *List;
- if (Next)
- Next->Prev = &Next;
- Prev = List;
- *Prev = this;
- }
-
- void removeFromList() {
- *Prev = Next;
- if (Next)
- Next->Prev = Prev;
- }
+ inline void addToList(unsigned &Count);
+ inline void addToList(Use *&List);
+ inline void removeFromList(unsigned &Count);
+ inline void removeFromList(Use *&List);
};
/// Allow clients to treat uses just like values when using
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index cfed12e2f5f8d..96e4cf446cb93 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -116,7 +116,10 @@ class Value {
private:
Type *VTy;
- Use *UseList;
+ union {
+ Use *List = nullptr;
+ unsigned Count;
+ } Uses;
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -341,21 +344,28 @@ class Value {
#endif
}
+ /// Check if this Value has a use-list.
+ bool hasUseList() const { return !isa<ConstantData>(this); }
+
bool use_empty() const {
assertModuleIsMaterialized();
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
}
bool materialized_use_empty() const {
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : !Uses.Count;
}
using use_iterator = use_iterator_impl<Use>;
using const_use_iterator = use_iterator_impl<const Use>;
- use_iterator materialized_use_begin() { return use_iterator(UseList); }
+ use_iterator materialized_use_begin() {
+ assert(hasUseList());
+ return use_iterator(Uses.List);
+ }
const_use_iterator materialized_use_begin() const {
- return const_use_iterator(UseList);
+ assert(hasUseList());
+ return const_use_iterator(Uses.List);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
@@ -382,17 +392,18 @@ class Value {
return materialized_uses();
}
- bool user_empty() const {
- assertModuleIsMaterialized();
- return UseList == nullptr;
- }
+ bool user_empty() const { return use_empty(); }
using user_iterator = user_iterator_impl<User>;
using const_user_iterator = user_iterator_impl<const User>;
- user_iterator materialized_user_begin() { return user_iterator(UseList); }
+ user_iterator materialized_user_begin() {
+ assert(hasUseList());
+ return user_iterator(Uses.List);
+ }
const_user_iterator materialized_user_begin() const {
- return const_user_iterator(UseList);
+ assert(hasUseList());
+ return const_user_iterator(Uses.List);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
@@ -431,7 +442,11 @@ class Value {
///
/// This is specialized because it is a common request and does not require
/// traversing the whole use list.
- bool hasOneUse() const { return hasSingleElement(uses()); }
+ bool hasOneUse() const {
+ if (!hasUseList())
+ return Uses.Count == 1;
+ return hasSingleElement(uses());
+ }
/// Return true if this Value has exactly N uses.
bool hasNUses(unsigned N) const;
@@ -493,6 +508,8 @@ class Value {
static void dropDroppableUse(Use &U);
/// Check if this value is used in the specified basic block.
+ ///
+ /// Not supported for ConstantData.
bool isUsedInBasicBlock(const BasicBlock *BB) const;
/// This method computes the number of uses of this Value.
@@ -502,7 +519,19 @@ class Value {
unsigned getNumUses() const;
/// This method should only be used by the Use class.
- void addUse(Use &U) { U.addToList(&UseList); }
+ void addUse(Use &U) {
+ if (hasUseList())
+ U.addToList(Uses.List);
+ else
+ U.addToList(Uses.Count);
+ }
+
+ void removeUse(Use &U) {
+ if (hasUseList())
+ U.removeFromList(Uses.List);
+ else
+ U.removeFromList(Uses.Count);
+ }
/// Concrete subclass of this.
///
@@ -843,7 +872,8 @@ class Value {
///
/// \return the first element in the list.
///
- /// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
+ /// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
+ /// update).
template <class Compare>
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
Use *Merged;
@@ -889,10 +919,50 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
return OS;
}
+inline Use::~Use() {
+ if (Val)
+ Val->removeUse(*this);
+}
+
+void Use::addToList(unsigned &Count) {
+ assert(isa<ConstantData>(Val) && "Only ConstantData is ref-counted");
+ ++Count;
+
+ // We don't have a uselist - clear the remnant if we are replacing a
+ // non-constant value.
+ Prev = nullptr;
+ Next = nullptr;
+}
+
+void Use::addToList(Use *&List) {
+ assert(!isa<ConstantData>(Val) && "ConstantData has no use-list");
+
+ Next = List;
+ if (Next)
+ Next->Prev = &Next;
+ Prev = &List;
+ List = this;
+}
+
+void Use::removeFromList(unsigned &Count) {
+ assert(isa<ConstantData>(Val));
+ assert(Count > 0 && "reference count underflow");
+ assert(!Prev && !Next && "should not have uselist remnant");
+ --Count;
+}
+
+void Use::removeFromList(Use *&List) {
+ *Prev = Next;
+ if (Next)
+ Next->Prev = Prev;
+}
+
void Use::set(Value *V) {
- if (Val) removeFromList();
+ if (Val)
+ Val->removeUse(*this);
Val = V;
- if (V) V->addUse(*this);
+ if (V)
+ V->addUse(*this);
}
Value *Use::operator=(Value *RHS) {
@@ -906,7 +976,7 @@ const Use &Use::operator=(const Use &RHS) {
}
template <class Compare> void Value::sortUseList(Compare Cmp) {
- if (!UseList || !UseList->Next)
+ if (!hasUseList() || !Uses.List || !Uses.List->Next)
// No need to sort 0 or 1 uses.
return;
@@ -919,10 +989,10 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
Use *Slots[MaxSlots];
// Collect the first use, turning it into a single-item list.
- Use *Next = UseList->Next;
- UseList->Next = nullptr;
+ Use *Next = Uses.List->Next;
+ Uses.List->Next = nullptr;
unsigned NumSlots = 1;
- Slots[0] = UseList;
+ Slots[0] = Uses.List;
// Collect all but the last use.
while (Next->Next) {
@@ -958,15 +1028,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
// Merge all the lists together.
assert(Next && "Expected one more Use");
assert(!Next->Next && "Expected only one Use");
- UseList = Next;
+ Uses.List = Next;
for (unsigned I = 0; I < NumSlots; ++I)
if (Slots[I])
- // Since the uses in Slots[I] originally preceded those in UseList, send
+ // Since the uses in Slots[I] originally preceded those in Uses.List, send
// Slots[I] in as the left parameter to maintain a stable sort.
- UseList = mergeUseLists(Slots[I], UseList, Cmp);
+ Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
// Fix the Prev pointers.
- for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
+ for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
I->Prev = Prev;
Prev = &I->Next;
}
diff --git a/llvm/lib/Analysis/TypeMetadataUtils.cpp b/llvm/lib/Analysis/TypeMetadataUtils.cpp
index 9ec0785eb5034..8099fbc3daeda 100644
--- a/llvm/lib/Analysis/TypeMetadataUtils.cpp
+++ b/llvm/lib/Analysis/TypeMetadataUtils.cpp
@@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
static void findLoadCallsAtConstantOffset(
const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
int64_t Offset, const CallInst *CI, DominatorTree &DT) {
+ if (!VPtr->hasUseList())
+ return;
+
for (const Use &U : VPtr->uses()) {
Value *User = U.getUser();
if (isa<BitCastInst>(User)) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index af0422f09bc4f..e806fdcdd7cad 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
//===----------------------------------------------------------------------===//
bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
SMLoc Loc) {
+ if (isa<ConstantData>(V))
+ return false;
if (V->use_empty())
return error(Loc, "value has no uses");
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5c62ef4ad8e4e..de734aef0073e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3856,6 +3856,10 @@ Error BitcodeReader::parseUseLists() {
V = FunctionBBs[ID];
} else
V = ValueList[ID];
+
+ if (isa<ConstantData>(V))
+ break;
+
unsigned NumUses = 0;
SmallDenseMap<const Use *, unsigned, 16> Order;
for (const Use &U : V->materialized_uses()) {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 9f735f77d29dc..0c81a99f2235b 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
static void predictValueUseListOrder(const Value *V, const Function *F,
OrderMap &OM, UseListOrderStack &Stack) {
+ if (isa<ConstantData>(V))
+ return;
+
auto &IDPair = OM[V];
assert(IDPair.first && "Unmapped value");
if (IDPair.second)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index cf8f1c878ea5a..2fa949773ccd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3953,7 +3953,7 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
// Globals with sub-elements such as combinations of arrays and structs
// are handled recursively by emitGlobalConstantImpl. Keep track of the
// constant symbol base and the current position with BaseCV and Offset.
- if (!BaseCV && CV->hasOneUse())
+ if (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
BaseCV = dyn_cast<Constant>(CV->user_back());
if (isa<ConstantAggregateZero>(CV)) {
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 1bbd1b6f71b14..b728ac1c7c38a 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -8547,6 +8547,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
return false;
Value *X = Cmp->getOperand(0);
+ if (!X->hasUseList())
+ return false;
+
APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();
for (auto *U : X->users()) {
diff --git a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
index 4cd378f9aa595..43c9c4836e207 100644
--- a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
+++ b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
@@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
return nullptr;
+ if (!R->hasUseList() || !I->hasUseList())
+ return nullptr;
+
auto CommonUser =
findCommonBetweenCollections<Value *>(R->users(), I->users());
if (!CommonUser)
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ac8aa0d35ea30..fedad23066084 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
if (OM.lookup(V))
return;
- if (const Constant *C = dyn_cast<Constant>(V))
+ if (const Constant *C = dyn_cast<Constant>(V)) {
+ if (isa<ConstantData>(C))
+ return;
+
if (C->getNumOperands() && !isa<GlobalValue>(C))
for (const Value *Op : C->operands())
if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
orderValue(Op, OM);
+ }
// Note: we cannot cache this lookup above, since inserting into the map
// changes the map's size, and thus affects the other IDs.
@@ -275,7 +279,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
UseListOrderMap ULOM;
for (const auto &Pair : OM) {
const Value *V = Pair.first;
- if (V->use_empty() || std::next(V->use_begin()) == V->use_end())
+ if (!V->hasUseList() || V->use_empty() ||
+ std::next(V->use_begin()) == V->use_end())
continue;
std::vector<unsigned> Shuffle =
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4eadecb54feb5..19dd68b3a011d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
}
bool Instruction::isOnlyUserOfAnyOperand() {
- return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
+ return any_of(operands(), [](const Value *V) {
+ return V->hasUseList() && V->hasOneUser();
+ });
}
void Instruction::setHasNoUnsignedWrap(bool b) {
diff --git a/llvm/lib/IR/Use.cpp b/llvm/lib/IR/Use.cpp
index 99a89386d75f9..67882ba0144b4 100644
--- a/llvm/lib/IR/Use.cpp
+++ b/llvm/lib/IR/Use.cpp
@@ -19,11 +19,15 @@ void Use::swap(Use &RHS) {
std::swap(Next, RHS.Next);
std::swap(Prev, RHS.Prev);
- *Prev = this;
+ if (Prev)
+ *Prev = this;
+
if (Next)
Next->Prev = &Next;
- *RHS.Prev = &RHS;
+ if (RHS.Prev)
+ *RHS.Prev = &RHS;
+
if (RHS.Next)
RHS.Next->Prev = &RHS.Next;
}
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 6c52ced5f73b2..025cdece7d9fb 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -53,7 +53,7 @@ static inline Type *checkType(Type *Ty) {
Value::Value(Type *ty, unsigned scid)
: SubclassID(scid), HasValueHandle(0), SubclassOptionalData(0),
SubclassData(0), NumUserOperands(0), IsUsedByMD(false), HasName(false),
- HasMetadata(false), VTy(checkType(ty)), UseList(nullptr) {
+ HasMetadata(false), VTy(checkType(ty)) {
static_assert(ConstantFirstVal == 0, "!(SubclassID < ConstantFirstVal)");
// FIXME: Why isn't this in the subclass gunk??
// Note, we cannot call isa<CallInst> before the CallInst has been
@@ -147,10 +147,14 @@ void Value::destroyValueName() {
}
bool Value::hasNUses(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count == N;
return hasNItems(use_begin(), use_end(), N);
}
bool Value::hasNUsesOrMore(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count >= N;
return hasNItemsOrMore(use_begin(), use_end(), N);
}
@@ -231,6 +235,8 @@ void Value::dropDroppableUse(Use &U) {
}
bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
+ assert(!isa<ConstantData>(this) && "ConstantData has no use-list");
+
// This can be computed either by scanning the instructions in BB, or by
// scanning the use list of this Value. Both lists can be very long, but
// usually one is quite short.
@@ -252,6 +258,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
}
unsigned Value::getNumUses() const {
+ if (!hasUseList())
+ return Uses.Count;
+
return (unsigned)std::distance(use_begin(), use_end());
}
@@ -500,6 +509,7 @@ static bool contains(Value *Expr, Value *V) {
#endif // NDEBUG
void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
+ assert(hasUseList() && "Cannot replace constant data");
assert(New && "Value::replaceAllUsesWith(<null>) is invalid!");
assert(!contains(New, this) &&
"this->replaceAllUsesWith(expr(this)) is NOT valid!");
@@ -513,7 +523,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
ValueAsMetadata::handleRAUW(this, New);
while (!materialized_use_empty()) {
- Use &U = *UseList;
+ Use &U = *Uses.List;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -845,7 +855,7 @@ bool Value::canBeFreed() const {
// which is why we need the explicit opt in on a per collector basis.
if (!F->hasGC())
return true;
-
+
const auto &GCName = F->getGC();
if (GCName == "statepoint-example") {
auto *PT = cast<PointerType>(this->getType());
@@ -1093,12 +1103,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
LLVMContext &Value::getContext() const { return VTy->getContext(); }
void Value::reverseUseList() {
- if (!UseList || !UseList->Next)
+ if (!Uses.List || !Uses.List->Next || !hasUseList())
// No need to reverse 0 or 1 uses.
return;
- Use *Head = UseList;
- Use *Current = UseList->Next;
+ Use *Head = Uses.List;
+ Use *Current = Uses.List->Next;
Head->Next = nullptr;
while (Current) {
Use *Next = Current->Next;
@@ -1107,8 +1117,8 @@ void Value::reverseUseList() {
Head = Current;
Current = Next;
}
- UseList = Head;
- Head->Prev = &UseList;
+ Uses.List = Head;
+ Head->Prev = &Uses.List;
}
bool Value::isSwiftError() const {
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 8d83fef265e6f..06d7c415e070c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -633,7 +633,7 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
// Look at the first element of the array to determine its type
if (isa<ArrayType>(EltTy))
EltTy = EltTy->getArrayElementType();
- } else {
+ } else if (LdVal->hasUseList()) {
// FIXME: grubbing around uses is pretty ugly, but with no more
// `getPointerElementType` there's not much else we can do.
for (const auto *LdUser : LdVal->users()) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 0067d2400529a..15d80de960741 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1104,7 +1104,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
IRBuilder<> B(Ctx);
for (auto &OpIt : Ops) {
Value *Op = OpIt.first;
- if (Op->use_empty())
+ if (!Op->hasUseList() || Op->use_empty())
continue;
if (AskOps && !AskOps->contains(Op))
continue;
@@ -1470,34 +1470,36 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
// Do not emit new spv_ptrcast if equivalent one already exists or when
// spv_assign_ptr_type already targets this pointer with the same element
// type.
- for (auto User : Pointer->users()) {
- auto *II = dyn_cast<IntrinsicInst>(User);
- if (!II ||
- (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
- II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
- II->getOperand(0) != Pointer)
- continue;
+ if (Pointer->hasUseList()) {
+ for (auto User : Pointer->users()) {
+ auto *II = dyn_cast<IntrinsicInst>(User);
+ if (!II ||
+ (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
+ II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
+ II->getOperand(0) != Pointer)
+ continue;
- // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
- // pointer.
- FirstPtrCastOrAssignPtrType = false;
- if (II->getOperand(1) != VMD ||
- dyn_cast<ConstantInt>(II->getOperand(2))->getSExtValue() !=
- AddressSpace)
- continue;
+ // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
+ // pointer.
+ FirstPtrCastOrAssignPtrType = false;
+ if (II->getOperand(1) != VMD ||
+ dyn_cast<ConstantInt...
[truncated]
|
|
@llvm/pr-subscribers-llvm-transforms Author: Matt Arsenault (arsenm) ChangesThis is a resurrected version of the patch attached to this RFC: https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606 In this adaptation, there are a few differences. In the original patch, the Use's Remove use-lists from instances of ConstantData (which are shared To continue supporting most of the use-list API, store a ref-count in This change has three benefits:
The downside is that you can't look at all the users of ConstantData, Possible follow-ups:
Fixes #58629 Patch is 29.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134692.diff 24 Files Affected:
diff --git a/llvm/include/llvm/IR/Use.h b/llvm/include/llvm/IR/Use.h
index a86b9c46c1f69..bcd1fd6677497 100644
--- a/llvm/include/llvm/IR/Use.h
+++ b/llvm/include/llvm/IR/Use.h
@@ -23,6 +23,7 @@
namespace llvm {
template <typename> struct simplify_type;
+class ConstantData;
class User;
class Value;
@@ -42,10 +43,7 @@ class Use {
private:
/// Destructor - Only for zap()
- ~Use() {
- if (Val)
- removeFromList();
- }
+ ~Use();
/// Constructor
Use(User *Parent) : Parent(Parent) {}
@@ -87,19 +85,10 @@ class Use {
Use **Prev = nullptr;
User *Parent = nullptr;
- void addToList(Use **List) {
- Next = *List;
- if (Next)
- Next->Prev = &Next;
- Prev = List;
- *Prev = this;
- }
-
- void removeFromList() {
- *Prev = Next;
- if (Next)
- Next->Prev = Prev;
- }
+ inline void addToList(unsigned &Count);
+ inline void addToList(Use *&List);
+ inline void removeFromList(unsigned &Count);
+ inline void removeFromList(Use *&List);
};
/// Allow clients to treat uses just like values when using
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index cfed12e2f5f8d..96e4cf446cb93 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -116,7 +116,10 @@ class Value {
private:
Type *VTy;
- Use *UseList;
+ union {
+ Use *List = nullptr;
+ unsigned Count;
+ } Uses;
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -341,21 +344,28 @@ class Value {
#endif
}
+ /// Check if this Value has a use-list.
+ bool hasUseList() const { return !isa<ConstantData>(this); }
+
bool use_empty() const {
assertModuleIsMaterialized();
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
}
bool materialized_use_empty() const {
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : !Uses.Count;
}
using use_iterator = use_iterator_impl<Use>;
using const_use_iterator = use_iterator_impl<const Use>;
- use_iterator materialized_use_begin() { return use_iterator(UseList); }
+ use_iterator materialized_use_begin() {
+ assert(hasUseList());
+ return use_iterator(Uses.List);
+ }
const_use_iterator materialized_use_begin() const {
- return const_use_iterator(UseList);
+ assert(hasUseList());
+ return const_use_iterator(Uses.List);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
@@ -382,17 +392,18 @@ class Value {
return materialized_uses();
}
- bool user_empty() const {
- assertModuleIsMaterialized();
- return UseList == nullptr;
- }
+ bool user_empty() const { return use_empty(); }
using user_iterator = user_iterator_impl<User>;
using const_user_iterator = user_iterator_impl<const User>;
- user_iterator materialized_user_begin() { return user_iterator(UseList); }
+ user_iterator materialized_user_begin() {
+ assert(hasUseList());
+ return user_iterator(Uses.List);
+ }
const_user_iterator materialized_user_begin() const {
- return const_user_iterator(UseList);
+ assert(hasUseList());
+ return const_user_iterator(Uses.List);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
@@ -431,7 +442,11 @@ class Value {
///
/// This is specialized because it is a common request and does not require
/// traversing the whole use list.
- bool hasOneUse() const { return hasSingleElement(uses()); }
+ bool hasOneUse() const {
+ if (!hasUseList())
+ return Uses.Count == 1;
+ return hasSingleElement(uses());
+ }
/// Return true if this Value has exactly N uses.
bool hasNUses(unsigned N) const;
@@ -493,6 +508,8 @@ class Value {
static void dropDroppableUse(Use &U);
/// Check if this value is used in the specified basic block.
+ ///
+ /// Not supported for ConstantData.
bool isUsedInBasicBlock(const BasicBlock *BB) const;
/// This method computes the number of uses of this Value.
@@ -502,7 +519,19 @@ class Value {
unsigned getNumUses() const;
/// This method should only be used by the Use class.
- void addUse(Use &U) { U.addToList(&UseList); }
+ void addUse(Use &U) {
+ if (hasUseList())
+ U.addToList(Uses.List);
+ else
+ U.addToList(Uses.Count);
+ }
+
+ void removeUse(Use &U) {
+ if (hasUseList())
+ U.removeFromList(Uses.List);
+ else
+ U.removeFromList(Uses.Count);
+ }
/// Concrete subclass of this.
///
@@ -843,7 +872,8 @@ class Value {
///
/// \return the first element in the list.
///
- /// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
+ /// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
+ /// update).
template <class Compare>
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
Use *Merged;
@@ -889,10 +919,50 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
return OS;
}
+inline Use::~Use() {
+ if (Val)
+ Val->removeUse(*this);
+}
+
+void Use::addToList(unsigned &Count) {
+ assert(isa<ConstantData>(Val) && "Only ConstantData is ref-counted");
+ ++Count;
+
+ // We don't have a uselist - clear the remnant if we are replacing a
+ // non-constant value.
+ Prev = nullptr;
+ Next = nullptr;
+}
+
+void Use::addToList(Use *&List) {
+ assert(!isa<ConstantData>(Val) && "ConstantData has no use-list");
+
+ Next = List;
+ if (Next)
+ Next->Prev = &Next;
+ Prev = &List;
+ List = this;
+}
+
+void Use::removeFromList(unsigned &Count) {
+ assert(isa<ConstantData>(Val));
+ assert(Count > 0 && "reference count underflow");
+ assert(!Prev && !Next && "should not have uselist remnant");
+ --Count;
+}
+
+void Use::removeFromList(Use *&List) {
+ *Prev = Next;
+ if (Next)
+ Next->Prev = Prev;
+}
+
void Use::set(Value *V) {
- if (Val) removeFromList();
+ if (Val)
+ Val->removeUse(*this);
Val = V;
- if (V) V->addUse(*this);
+ if (V)
+ V->addUse(*this);
}
Value *Use::operator=(Value *RHS) {
@@ -906,7 +976,7 @@ const Use &Use::operator=(const Use &RHS) {
}
template <class Compare> void Value::sortUseList(Compare Cmp) {
- if (!UseList || !UseList->Next)
+ if (!hasUseList() || !Uses.List || !Uses.List->Next)
// No need to sort 0 or 1 uses.
return;
@@ -919,10 +989,10 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
Use *Slots[MaxSlots];
// Collect the first use, turning it into a single-item list.
- Use *Next = UseList->Next;
- UseList->Next = nullptr;
+ Use *Next = Uses.List->Next;
+ Uses.List->Next = nullptr;
unsigned NumSlots = 1;
- Slots[0] = UseList;
+ Slots[0] = Uses.List;
// Collect all but the last use.
while (Next->Next) {
@@ -958,15 +1028,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
// Merge all the lists together.
assert(Next && "Expected one more Use");
assert(!Next->Next && "Expected only one Use");
- UseList = Next;
+ Uses.List = Next;
for (unsigned I = 0; I < NumSlots; ++I)
if (Slots[I])
- // Since the uses in Slots[I] originally preceded those in UseList, send
+ // Since the uses in Slots[I] originally preceded those in Uses.List, send
// Slots[I] in as the left parameter to maintain a stable sort.
- UseList = mergeUseLists(Slots[I], UseList, Cmp);
+ Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
// Fix the Prev pointers.
- for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
+ for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
I->Prev = Prev;
Prev = &I->Next;
}
diff --git a/llvm/lib/Analysis/TypeMetadataUtils.cpp b/llvm/lib/Analysis/TypeMetadataUtils.cpp
index 9ec0785eb5034..8099fbc3daeda 100644
--- a/llvm/lib/Analysis/TypeMetadataUtils.cpp
+++ b/llvm/lib/Analysis/TypeMetadataUtils.cpp
@@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
static void findLoadCallsAtConstantOffset(
const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
int64_t Offset, const CallInst *CI, DominatorTree &DT) {
+ if (!VPtr->hasUseList())
+ return;
+
for (const Use &U : VPtr->uses()) {
Value *User = U.getUser();
if (isa<BitCastInst>(User)) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index af0422f09bc4f..e806fdcdd7cad 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
//===----------------------------------------------------------------------===//
bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
SMLoc Loc) {
+ if (isa<ConstantData>(V))
+ return false;
if (V->use_empty())
return error(Loc, "value has no uses");
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5c62ef4ad8e4e..de734aef0073e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3856,6 +3856,10 @@ Error BitcodeReader::parseUseLists() {
V = FunctionBBs[ID];
} else
V = ValueList[ID];
+
+ if (isa<ConstantData>(V))
+ break;
+
unsigned NumUses = 0;
SmallDenseMap<const Use *, unsigned, 16> Order;
for (const Use &U : V->materialized_uses()) {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 9f735f77d29dc..0c81a99f2235b 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
static void predictValueUseListOrder(const Value *V, const Function *F,
OrderMap &OM, UseListOrderStack &Stack) {
+ if (isa<ConstantData>(V))
+ return;
+
auto &IDPair = OM[V];
assert(IDPair.first && "Unmapped value");
if (IDPair.second)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index cf8f1c878ea5a..2fa949773ccd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3953,7 +3953,7 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
// Globals with sub-elements such as combinations of arrays and structs
// are handled recursively by emitGlobalConstantImpl. Keep track of the
// constant symbol base and the current position with BaseCV and Offset.
- if (!BaseCV && CV->hasOneUse())
+ if (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
BaseCV = dyn_cast<Constant>(CV->user_back());
if (isa<ConstantAggregateZero>(CV)) {
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 1bbd1b6f71b14..b728ac1c7c38a 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -8547,6 +8547,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
return false;
Value *X = Cmp->getOperand(0);
+ if (!X->hasUseList())
+ return false;
+
APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();
for (auto *U : X->users()) {
diff --git a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
index 4cd378f9aa595..43c9c4836e207 100644
--- a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
+++ b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
@@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
return nullptr;
+ if (!R->hasUseList() || !I->hasUseList())
+ return nullptr;
+
auto CommonUser =
findCommonBetweenCollections<Value *>(R->users(), I->users());
if (!CommonUser)
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ac8aa0d35ea30..fedad23066084 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
if (OM.lookup(V))
return;
- if (const Constant *C = dyn_cast<Constant>(V))
+ if (const Constant *C = dyn_cast<Constant>(V)) {
+ if (isa<ConstantData>(C))
+ return;
+
if (C->getNumOperands() && !isa<GlobalValue>(C))
for (const Value *Op : C->operands())
if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
orderValue(Op, OM);
+ }
// Note: we cannot cache this lookup above, since inserting into the map
// changes the map's size, and thus affects the other IDs.
@@ -275,7 +279,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
UseListOrderMap ULOM;
for (const auto &Pair : OM) {
const Value *V = Pair.first;
- if (V->use_empty() || std::next(V->use_begin()) == V->use_end())
+ if (!V->hasUseList() || V->use_empty() ||
+ std::next(V->use_begin()) == V->use_end())
continue;
std::vector<unsigned> Shuffle =
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4eadecb54feb5..19dd68b3a011d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
}
bool Instruction::isOnlyUserOfAnyOperand() {
- return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
+ return any_of(operands(), [](const Value *V) {
+ return V->hasUseList() && V->hasOneUser();
+ });
}
void Instruction::setHasNoUnsignedWrap(bool b) {
diff --git a/llvm/lib/IR/Use.cpp b/llvm/lib/IR/Use.cpp
index 99a89386d75f9..67882ba0144b4 100644
--- a/llvm/lib/IR/Use.cpp
+++ b/llvm/lib/IR/Use.cpp
@@ -19,11 +19,15 @@ void Use::swap(Use &RHS) {
std::swap(Next, RHS.Next);
std::swap(Prev, RHS.Prev);
- *Prev = this;
+ if (Prev)
+ *Prev = this;
+
if (Next)
Next->Prev = &Next;
- *RHS.Prev = &RHS;
+ if (RHS.Prev)
+ *RHS.Prev = &RHS;
+
if (RHS.Next)
RHS.Next->Prev = &RHS.Next;
}
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 6c52ced5f73b2..025cdece7d9fb 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -53,7 +53,7 @@ static inline Type *checkType(Type *Ty) {
Value::Value(Type *ty, unsigned scid)
: SubclassID(scid), HasValueHandle(0), SubclassOptionalData(0),
SubclassData(0), NumUserOperands(0), IsUsedByMD(false), HasName(false),
- HasMetadata(false), VTy(checkType(ty)), UseList(nullptr) {
+ HasMetadata(false), VTy(checkType(ty)) {
static_assert(ConstantFirstVal == 0, "!(SubclassID < ConstantFirstVal)");
// FIXME: Why isn't this in the subclass gunk??
// Note, we cannot call isa<CallInst> before the CallInst has been
@@ -147,10 +147,14 @@ void Value::destroyValueName() {
}
bool Value::hasNUses(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count == N;
return hasNItems(use_begin(), use_end(), N);
}
bool Value::hasNUsesOrMore(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count >= N;
return hasNItemsOrMore(use_begin(), use_end(), N);
}
@@ -231,6 +235,8 @@ void Value::dropDroppableUse(Use &U) {
}
bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
+ assert(!isa<ConstantData>(this) && "ConstantData has no use-list");
+
// This can be computed either by scanning the instructions in BB, or by
// scanning the use list of this Value. Both lists can be very long, but
// usually one is quite short.
@@ -252,6 +258,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
}
unsigned Value::getNumUses() const {
+ if (!hasUseList())
+ return Uses.Count;
+
return (unsigned)std::distance(use_begin(), use_end());
}
@@ -500,6 +509,7 @@ static bool contains(Value *Expr, Value *V) {
#endif // NDEBUG
void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
+ assert(hasUseList() && "Cannot replace constant data");
assert(New && "Value::replaceAllUsesWith(<null>) is invalid!");
assert(!contains(New, this) &&
"this->replaceAllUsesWith(expr(this)) is NOT valid!");
@@ -513,7 +523,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
ValueAsMetadata::handleRAUW(this, New);
while (!materialized_use_empty()) {
- Use &U = *UseList;
+ Use &U = *Uses.List;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -845,7 +855,7 @@ bool Value::canBeFreed() const {
// which is why we need the explicit opt in on a per collector basis.
if (!F->hasGC())
return true;
-
+
const auto &GCName = F->getGC();
if (GCName == "statepoint-example") {
auto *PT = cast<PointerType>(this->getType());
@@ -1093,12 +1103,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
LLVMContext &Value::getContext() const { return VTy->getContext(); }
void Value::reverseUseList() {
- if (!UseList || !UseList->Next)
+ if (!Uses.List || !Uses.List->Next || !hasUseList())
// No need to reverse 0 or 1 uses.
return;
- Use *Head = UseList;
- Use *Current = UseList->Next;
+ Use *Head = Uses.List;
+ Use *Current = Uses.List->Next;
Head->Next = nullptr;
while (Current) {
Use *Next = Current->Next;
@@ -1107,8 +1117,8 @@ void Value::reverseUseList() {
Head = Current;
Current = Next;
}
- UseList = Head;
- Head->Prev = &UseList;
+ Uses.List = Head;
+ Head->Prev = &Uses.List;
}
bool Value::isSwiftError() const {
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 8d83fef265e6f..06d7c415e070c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -633,7 +633,7 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
// Look at the first element of the array to determine its type
if (isa<ArrayType>(EltTy))
EltTy = EltTy->getArrayElementType();
- } else {
+ } else if (LdVal->hasUseList()) {
// FIXME: grubbing around uses is pretty ugly, but with no more
// `getPointerElementType` there's not much else we can do.
for (const auto *LdUser : LdVal->users()) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 0067d2400529a..15d80de960741 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1104,7 +1104,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
IRBuilder<> B(Ctx);
for (auto &OpIt : Ops) {
Value *Op = OpIt.first;
- if (Op->use_empty())
+ if (!Op->hasUseList() || Op->use_empty())
continue;
if (AskOps && !AskOps->contains(Op))
continue;
@@ -1470,34 +1470,36 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
// Do not emit new spv_ptrcast if equivalent one already exists or when
// spv_assign_ptr_type already targets this pointer with the same element
// type.
- for (auto User : Pointer->users()) {
- auto *II = dyn_cast<IntrinsicInst>(User);
- if (!II ||
- (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
- II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
- II->getOperand(0) != Pointer)
- continue;
+ if (Pointer->hasUseList()) {
+ for (auto User : Pointer->users()) {
+ auto *II = dyn_cast<IntrinsicInst>(User);
+ if (!II ||
+ (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
+ II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
+ II->getOperand(0) != Pointer)
+ continue;
- // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
- // pointer.
- FirstPtrCastOrAssignPtrType = false;
- if (II->getOperand(1) != VMD ||
- dyn_cast<ConstantInt>(II->getOperand(2))->getSExtValue() !=
- AddressSpace)
- continue;
+ // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
+ // pointer.
+ FirstPtrCastOrAssignPtrType = false;
+ if (II->getOperand(1) != VMD ||
+ dyn_cast<ConstantInt...
[truncated]
|
|
@llvm/pr-subscribers-llvm-analysis Author: Matt Arsenault (arsenm) ChangesThis is a resurrected version of the patch attached to this RFC: https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606 In this adaptation, there are a few differences. In the original patch, the Use's Remove use-lists from instances of ConstantData (which are shared To continue supporting most of the use-list API, store a ref-count in This change has three benefits:
The downside is that you can't look at all the users of ConstantData, Possible follow-ups:
Fixes #58629 Patch is 29.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134692.diff 24 Files Affected:
diff --git a/llvm/include/llvm/IR/Use.h b/llvm/include/llvm/IR/Use.h
index a86b9c46c1f69..bcd1fd6677497 100644
--- a/llvm/include/llvm/IR/Use.h
+++ b/llvm/include/llvm/IR/Use.h
@@ -23,6 +23,7 @@
namespace llvm {
template <typename> struct simplify_type;
+class ConstantData;
class User;
class Value;
@@ -42,10 +43,7 @@ class Use {
private:
/// Destructor - Only for zap()
- ~Use() {
- if (Val)
- removeFromList();
- }
+ ~Use();
/// Constructor
Use(User *Parent) : Parent(Parent) {}
@@ -87,19 +85,10 @@ class Use {
Use **Prev = nullptr;
User *Parent = nullptr;
- void addToList(Use **List) {
- Next = *List;
- if (Next)
- Next->Prev = &Next;
- Prev = List;
- *Prev = this;
- }
-
- void removeFromList() {
- *Prev = Next;
- if (Next)
- Next->Prev = Prev;
- }
+ inline void addToList(unsigned &Count);
+ inline void addToList(Use *&List);
+ inline void removeFromList(unsigned &Count);
+ inline void removeFromList(Use *&List);
};
/// Allow clients to treat uses just like values when using
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index cfed12e2f5f8d..96e4cf446cb93 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -116,7 +116,10 @@ class Value {
private:
Type *VTy;
- Use *UseList;
+ union {
+ Use *List = nullptr;
+ unsigned Count;
+ } Uses;
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -341,21 +344,28 @@ class Value {
#endif
}
+ /// Check if this Value has a use-list.
+ bool hasUseList() const { return !isa<ConstantData>(this); }
+
bool use_empty() const {
assertModuleIsMaterialized();
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
}
bool materialized_use_empty() const {
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : !Uses.Count;
}
using use_iterator = use_iterator_impl<Use>;
using const_use_iterator = use_iterator_impl<const Use>;
- use_iterator materialized_use_begin() { return use_iterator(UseList); }
+ use_iterator materialized_use_begin() {
+ assert(hasUseList());
+ return use_iterator(Uses.List);
+ }
const_use_iterator materialized_use_begin() const {
- return const_use_iterator(UseList);
+ assert(hasUseList());
+ return const_use_iterator(Uses.List);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
@@ -382,17 +392,18 @@ class Value {
return materialized_uses();
}
- bool user_empty() const {
- assertModuleIsMaterialized();
- return UseList == nullptr;
- }
+ bool user_empty() const { return use_empty(); }
using user_iterator = user_iterator_impl<User>;
using const_user_iterator = user_iterator_impl<const User>;
- user_iterator materialized_user_begin() { return user_iterator(UseList); }
+ user_iterator materialized_user_begin() {
+ assert(hasUseList());
+ return user_iterator(Uses.List);
+ }
const_user_iterator materialized_user_begin() const {
- return const_user_iterator(UseList);
+ assert(hasUseList());
+ return const_user_iterator(Uses.List);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
@@ -431,7 +442,11 @@ class Value {
///
/// This is specialized because it is a common request and does not require
/// traversing the whole use list.
- bool hasOneUse() const { return hasSingleElement(uses()); }
+ bool hasOneUse() const {
+ if (!hasUseList())
+ return Uses.Count == 1;
+ return hasSingleElement(uses());
+ }
/// Return true if this Value has exactly N uses.
bool hasNUses(unsigned N) const;
@@ -493,6 +508,8 @@ class Value {
static void dropDroppableUse(Use &U);
/// Check if this value is used in the specified basic block.
+ ///
+ /// Not supported for ConstantData.
bool isUsedInBasicBlock(const BasicBlock *BB) const;
/// This method computes the number of uses of this Value.
@@ -502,7 +519,19 @@ class Value {
unsigned getNumUses() const;
/// This method should only be used by the Use class.
- void addUse(Use &U) { U.addToList(&UseList); }
+ void addUse(Use &U) {
+ if (hasUseList())
+ U.addToList(Uses.List);
+ else
+ U.addToList(Uses.Count);
+ }
+
+ void removeUse(Use &U) {
+ if (hasUseList())
+ U.removeFromList(Uses.List);
+ else
+ U.removeFromList(Uses.Count);
+ }
/// Concrete subclass of this.
///
@@ -843,7 +872,8 @@ class Value {
///
/// \return the first element in the list.
///
- /// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
+ /// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
+ /// update).
template <class Compare>
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
Use *Merged;
@@ -889,10 +919,50 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
return OS;
}
+inline Use::~Use() {
+ if (Val)
+ Val->removeUse(*this);
+}
+
+void Use::addToList(unsigned &Count) {
+ assert(isa<ConstantData>(Val) && "Only ConstantData is ref-counted");
+ ++Count;
+
+ // We don't have a uselist - clear the remnant if we are replacing a
+ // non-constant value.
+ Prev = nullptr;
+ Next = nullptr;
+}
+
+void Use::addToList(Use *&List) {
+ assert(!isa<ConstantData>(Val) && "ConstantData has no use-list");
+
+ Next = List;
+ if (Next)
+ Next->Prev = &Next;
+ Prev = &List;
+ List = this;
+}
+
+void Use::removeFromList(unsigned &Count) {
+ assert(isa<ConstantData>(Val));
+ assert(Count > 0 && "reference count underflow");
+ assert(!Prev && !Next && "should not have uselist remnant");
+ --Count;
+}
+
+void Use::removeFromList(Use *&List) {
+ *Prev = Next;
+ if (Next)
+ Next->Prev = Prev;
+}
+
void Use::set(Value *V) {
- if (Val) removeFromList();
+ if (Val)
+ Val->removeUse(*this);
Val = V;
- if (V) V->addUse(*this);
+ if (V)
+ V->addUse(*this);
}
Value *Use::operator=(Value *RHS) {
@@ -906,7 +976,7 @@ const Use &Use::operator=(const Use &RHS) {
}
template <class Compare> void Value::sortUseList(Compare Cmp) {
- if (!UseList || !UseList->Next)
+ if (!hasUseList() || !Uses.List || !Uses.List->Next)
// No need to sort 0 or 1 uses.
return;
@@ -919,10 +989,10 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
Use *Slots[MaxSlots];
// Collect the first use, turning it into a single-item list.
- Use *Next = UseList->Next;
- UseList->Next = nullptr;
+ Use *Next = Uses.List->Next;
+ Uses.List->Next = nullptr;
unsigned NumSlots = 1;
- Slots[0] = UseList;
+ Slots[0] = Uses.List;
// Collect all but the last use.
while (Next->Next) {
@@ -958,15 +1028,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
// Merge all the lists together.
assert(Next && "Expected one more Use");
assert(!Next->Next && "Expected only one Use");
- UseList = Next;
+ Uses.List = Next;
for (unsigned I = 0; I < NumSlots; ++I)
if (Slots[I])
- // Since the uses in Slots[I] originally preceded those in UseList, send
+ // Since the uses in Slots[I] originally preceded those in Uses.List, send
// Slots[I] in as the left parameter to maintain a stable sort.
- UseList = mergeUseLists(Slots[I], UseList, Cmp);
+ Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
// Fix the Prev pointers.
- for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
+ for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
I->Prev = Prev;
Prev = &I->Next;
}
diff --git a/llvm/lib/Analysis/TypeMetadataUtils.cpp b/llvm/lib/Analysis/TypeMetadataUtils.cpp
index 9ec0785eb5034..8099fbc3daeda 100644
--- a/llvm/lib/Analysis/TypeMetadataUtils.cpp
+++ b/llvm/lib/Analysis/TypeMetadataUtils.cpp
@@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
static void findLoadCallsAtConstantOffset(
const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
int64_t Offset, const CallInst *CI, DominatorTree &DT) {
+ if (!VPtr->hasUseList())
+ return;
+
for (const Use &U : VPtr->uses()) {
Value *User = U.getUser();
if (isa<BitCastInst>(User)) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index af0422f09bc4f..e806fdcdd7cad 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
//===----------------------------------------------------------------------===//
bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
SMLoc Loc) {
+ if (isa<ConstantData>(V))
+ return false;
if (V->use_empty())
return error(Loc, "value has no uses");
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5c62ef4ad8e4e..de734aef0073e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3856,6 +3856,10 @@ Error BitcodeReader::parseUseLists() {
V = FunctionBBs[ID];
} else
V = ValueList[ID];
+
+ if (isa<ConstantData>(V))
+ break;
+
unsigned NumUses = 0;
SmallDenseMap<const Use *, unsigned, 16> Order;
for (const Use &U : V->materialized_uses()) {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 9f735f77d29dc..0c81a99f2235b 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
static void predictValueUseListOrder(const Value *V, const Function *F,
OrderMap &OM, UseListOrderStack &Stack) {
+ if (isa<ConstantData>(V))
+ return;
+
auto &IDPair = OM[V];
assert(IDPair.first && "Unmapped value");
if (IDPair.second)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index cf8f1c878ea5a..2fa949773ccd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3953,7 +3953,7 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
// Globals with sub-elements such as combinations of arrays and structs
// are handled recursively by emitGlobalConstantImpl. Keep track of the
// constant symbol base and the current position with BaseCV and Offset.
- if (!BaseCV && CV->hasOneUse())
+ if (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
BaseCV = dyn_cast<Constant>(CV->user_back());
if (isa<ConstantAggregateZero>(CV)) {
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 1bbd1b6f71b14..b728ac1c7c38a 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -8547,6 +8547,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
return false;
Value *X = Cmp->getOperand(0);
+ if (!X->hasUseList())
+ return false;
+
APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();
for (auto *U : X->users()) {
diff --git a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
index 4cd378f9aa595..43c9c4836e207 100644
--- a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
+++ b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
@@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
return nullptr;
+ if (!R->hasUseList() || !I->hasUseList())
+ return nullptr;
+
auto CommonUser =
findCommonBetweenCollections<Value *>(R->users(), I->users());
if (!CommonUser)
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ac8aa0d35ea30..fedad23066084 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
if (OM.lookup(V))
return;
- if (const Constant *C = dyn_cast<Constant>(V))
+ if (const Constant *C = dyn_cast<Constant>(V)) {
+ if (isa<ConstantData>(C))
+ return;
+
if (C->getNumOperands() && !isa<GlobalValue>(C))
for (const Value *Op : C->operands())
if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
orderValue(Op, OM);
+ }
// Note: we cannot cache this lookup above, since inserting into the map
// changes the map's size, and thus affects the other IDs.
@@ -275,7 +279,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
UseListOrderMap ULOM;
for (const auto &Pair : OM) {
const Value *V = Pair.first;
- if (V->use_empty() || std::next(V->use_begin()) == V->use_end())
+ if (!V->hasUseList() || V->use_empty() ||
+ std::next(V->use_begin()) == V->use_end())
continue;
std::vector<unsigned> Shuffle =
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4eadecb54feb5..19dd68b3a011d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
}
bool Instruction::isOnlyUserOfAnyOperand() {
- return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
+ return any_of(operands(), [](const Value *V) {
+ return V->hasUseList() && V->hasOneUser();
+ });
}
void Instruction::setHasNoUnsignedWrap(bool b) {
diff --git a/llvm/lib/IR/Use.cpp b/llvm/lib/IR/Use.cpp
index 99a89386d75f9..67882ba0144b4 100644
--- a/llvm/lib/IR/Use.cpp
+++ b/llvm/lib/IR/Use.cpp
@@ -19,11 +19,15 @@ void Use::swap(Use &RHS) {
std::swap(Next, RHS.Next);
std::swap(Prev, RHS.Prev);
- *Prev = this;
+ if (Prev)
+ *Prev = this;
+
if (Next)
Next->Prev = &Next;
- *RHS.Prev = &RHS;
+ if (RHS.Prev)
+ *RHS.Prev = &RHS;
+
if (RHS.Next)
RHS.Next->Prev = &RHS.Next;
}
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 6c52ced5f73b2..025cdece7d9fb 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -53,7 +53,7 @@ static inline Type *checkType(Type *Ty) {
Value::Value(Type *ty, unsigned scid)
: SubclassID(scid), HasValueHandle(0), SubclassOptionalData(0),
SubclassData(0), NumUserOperands(0), IsUsedByMD(false), HasName(false),
- HasMetadata(false), VTy(checkType(ty)), UseList(nullptr) {
+ HasMetadata(false), VTy(checkType(ty)) {
static_assert(ConstantFirstVal == 0, "!(SubclassID < ConstantFirstVal)");
// FIXME: Why isn't this in the subclass gunk??
// Note, we cannot call isa<CallInst> before the CallInst has been
@@ -147,10 +147,14 @@ void Value::destroyValueName() {
}
bool Value::hasNUses(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count == N;
return hasNItems(use_begin(), use_end(), N);
}
bool Value::hasNUsesOrMore(unsigned N) const {
+ if (!hasUseList())
+ return Uses.Count >= N;
return hasNItemsOrMore(use_begin(), use_end(), N);
}
@@ -231,6 +235,8 @@ void Value::dropDroppableUse(Use &U) {
}
bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
+ assert(!isa<ConstantData>(this) && "ConstantData has no use-list");
+
// This can be computed either by scanning the instructions in BB, or by
// scanning the use list of this Value. Both lists can be very long, but
// usually one is quite short.
@@ -252,6 +258,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
}
unsigned Value::getNumUses() const {
+ if (!hasUseList())
+ return Uses.Count;
+
return (unsigned)std::distance(use_begin(), use_end());
}
@@ -500,6 +509,7 @@ static bool contains(Value *Expr, Value *V) {
#endif // NDEBUG
void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
+ assert(hasUseList() && "Cannot replace constant data");
assert(New && "Value::replaceAllUsesWith(<null>) is invalid!");
assert(!contains(New, this) &&
"this->replaceAllUsesWith(expr(this)) is NOT valid!");
@@ -513,7 +523,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
ValueAsMetadata::handleRAUW(this, New);
while (!materialized_use_empty()) {
- Use &U = *UseList;
+ Use &U = *Uses.List;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -845,7 +855,7 @@ bool Value::canBeFreed() const {
// which is why we need the explicit opt in on a per collector basis.
if (!F->hasGC())
return true;
-
+
const auto &GCName = F->getGC();
if (GCName == "statepoint-example") {
auto *PT = cast<PointerType>(this->getType());
@@ -1093,12 +1103,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
LLVMContext &Value::getContext() const { return VTy->getContext(); }
void Value::reverseUseList() {
- if (!UseList || !UseList->Next)
+ if (!Uses.List || !Uses.List->Next || !hasUseList())
// No need to reverse 0 or 1 uses.
return;
- Use *Head = UseList;
- Use *Current = UseList->Next;
+ Use *Head = Uses.List;
+ Use *Current = Uses.List->Next;
Head->Next = nullptr;
while (Current) {
Use *Next = Current->Next;
@@ -1107,8 +1117,8 @@ void Value::reverseUseList() {
Head = Current;
Current = Next;
}
- UseList = Head;
- Head->Prev = &UseList;
+ Uses.List = Head;
+ Head->Prev = &Uses.List;
}
bool Value::isSwiftError() const {
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 8d83fef265e6f..06d7c415e070c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -633,7 +633,7 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
// Look at the first element of the array to determine its type
if (isa<ArrayType>(EltTy))
EltTy = EltTy->getArrayElementType();
- } else {
+ } else if (LdVal->hasUseList()) {
// FIXME: grubbing around uses is pretty ugly, but with no more
// `getPointerElementType` there's not much else we can do.
for (const auto *LdUser : LdVal->users()) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 0067d2400529a..15d80de960741 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1104,7 +1104,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
IRBuilder<> B(Ctx);
for (auto &OpIt : Ops) {
Value *Op = OpIt.first;
- if (Op->use_empty())
+ if (!Op->hasUseList() || Op->use_empty())
continue;
if (AskOps && !AskOps->contains(Op))
continue;
@@ -1470,34 +1470,36 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
// Do not emit new spv_ptrcast if equivalent one already exists or when
// spv_assign_ptr_type already targets this pointer with the same element
// type.
- for (auto User : Pointer->users()) {
- auto *II = dyn_cast<IntrinsicInst>(User);
- if (!II ||
- (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
- II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
- II->getOperand(0) != Pointer)
- continue;
+ if (Pointer->hasUseList()) {
+ for (auto User : Pointer->users()) {
+ auto *II = dyn_cast<IntrinsicInst>(User);
+ if (!II ||
+ (II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type &&
+ II->getIntrinsicID() != Intrinsic::spv_ptrcast) ||
+ II->getOperand(0) != Pointer)
+ continue;
- // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
- // pointer.
- FirstPtrCastOrAssignPtrType = false;
- if (II->getOperand(1) != VMD ||
- dyn_cast<ConstantInt>(II->getOperand(2))->getSExtValue() !=
- AddressSpace)
- continue;
+ // There is some spv_ptrcast/spv_assign_ptr_type already targeting this
+ // pointer.
+ FirstPtrCastOrAssignPtrType = false;
+ if (II->getOperand(1) != VMD ||
+ dyn_cast<ConstantInt...
[truncated]
|
nikic
reviewed
Apr 13, 2025
| // We don't have a uselist - clear the remnant if we are replacing a | ||
| // non-constant value. | ||
| Prev = nullptr; | ||
| Next = nullptr; |
There was a problem hiding this comment.
Can you explain in more detail when this is necessary? It seems odd to me that addToList() should be responsible for nulling out these pointers.
There was a problem hiding this comment.
I meant to revisit this. I think this was only a problem in cases where Use::swap was used between a register and a constant
| @@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) { | |||
| //===----------------------------------------------------------------------===// | |||
| bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes, | |||
| SMLoc Loc) { | |||
| if (isa<ConstantData>(V)) | |||
There was a problem hiding this comment.
hasUseList() more natural here and next two files?
| @@ -437,7 +437,8 @@ static bool LinearizeExprTree(Instruction *I, | |||
| for (unsigned OpIdx = 0; OpIdx < I->getNumOperands(); ++OpIdx) { // Visit operands. | |||
| Value *Op = I->getOperand(OpIdx); | |||
| LLVM_DEBUG(dbgs() << "OPERAND: " << *Op << " (" << Weight << ")\n"); | |||
| assert(!Op->use_empty() && "No uses, so how did we get to it?!"); | |||
| assert((isa<ConstantData>(Op) || !Op->use_empty()) && | |||
There was a problem hiding this comment.
Why do we need this change? use_empty() should still work on ConstantData?
There was a problem hiding this comment.
I've started thinking maybe it shouldn't. The hasN* use cases don't really mean anything useful given the references are context wide
nikic
reviewed
Apr 13, 2025
There was a problem hiding this comment.
Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=09588e93bbe486ce782de9fba604f5cd184ec446&to=54260021fd4ffa41b5f20994af7b34653b4d4d42&stat=instructions%3Au It's possible that the wall-time picture may be different due to slightly less memory traffic, but I suspect the overhead of having two different code paths in a lot of common operations dominates.
This does make me wonder though if we wouldn't be better off keeping the actual use lists and just asserting that they aren't accessed (and excluding them from uselistorder, as they cannot matter). Given that we still have to keep the Next/Prev pointers in Use, we're not really gaining much from dropping the list.
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.
This is a resurrected version of the patch attached to this RFC:
https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606
In this adaptation, there are a few differences. In the original patch, the Use's
use list was replaced with an unsigned* to the reference count in the value. This
version leaves them as null and leaves the ref counting only in Value.
Remove use-lists from instances of ConstantData (which are shared
across modules and have no operands).
To continue supporting most of the use-list API, store a ref-count in
place of the use-list; this is for API like Value::use_empty and
Value::hasNUses. Operations that actually need the use-list -- like
Value::use_begin -- will assert.
This change has three benefits:
The compiler output cannot in any way depend on the use-list order
of instances of ConstantData.
There's no use-list traffic when adding and removing simple
constants from operand lists (although there is ref-count traffic;
YMMV).
It's cheaper to serialize use-lists (since we're no longer
serializing the use-list order of things like i32 0).
The downside is that you can't look at all the users of ConstantData,
but traversals of users of i32 0 are already ill-advised.
Possible follow-ups:
to have all ConstantData arguments, and drop the use-lists to
ref-counts in those cases. Callers need to check Value::hasUseList
before iterating through the use-list.
besides minimizing the scope of this commit, and maintaining the
counts is not free.
Fixes #58629