Skip to content

Commit 9c95bd9

Browse files
committed
IR: Remove uselist for constantdata
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
1 parent 2d00c73 commit 9c95bd9

26 files changed

+246
-104
lines changed

llvm/docs/ReleaseNotes.md

+2
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ Makes programs 10x faster by doing Special New Thing.
5656
Changes to the LLVM IR
5757
----------------------
5858

59+
* It is no longer permitted to inspect the uses of ConstantData
60+
5961
* The `nocapture` attribute has been replaced by `captures(none)`.
6062
* The constant expression variants of the following instructions have been
6163
removed:

llvm/include/llvm/IR/Constants.h

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ template <class ConstantClass> struct ConstantAggrKeyType;
5050
/// These constants have no operands; they represent their data directly.
5151
/// Since they can be in use by unrelated modules (and are never based on
5252
/// GlobalValues), it never makes sense to RAUW them.
53+
///
54+
/// These do not have use lists. It is illegal to inspect the uses.
5355
class ConstantData : public Constant {
5456
constexpr static IntrusiveOperandsAllocMarker AllocMarker{0};
5557

llvm/include/llvm/IR/Use.h

+6-17
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
namespace llvm {
2424

2525
template <typename> struct simplify_type;
26+
class ConstantData;
2627
class User;
2728
class Value;
2829

@@ -42,10 +43,7 @@ class Use {
4243

4344
private:
4445
/// Destructor - Only for zap()
45-
~Use() {
46-
if (Val)
47-
removeFromList();
48-
}
46+
~Use();
4947

5048
/// Constructor
5149
Use(User *Parent) : Parent(Parent) {}
@@ -87,19 +85,10 @@ class Use {
8785
Use **Prev = nullptr;
8886
User *Parent = nullptr;
8987

90-
void addToList(Use **List) {
91-
Next = *List;
92-
if (Next)
93-
Next->Prev = &Next;
94-
Prev = List;
95-
*Prev = this;
96-
}
97-
98-
void removeFromList() {
99-
*Prev = Next;
100-
if (Next)
101-
Next->Prev = Prev;
102-
}
88+
inline void addToList(unsigned &Count);
89+
inline void addToList(Use *&List);
90+
inline void removeFromList(unsigned &Count);
91+
inline void removeFromList(Use *&List);
10392
};
10493

10594
/// Allow clients to treat uses just like values when using

llvm/include/llvm/IR/Value.h

+94-24
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,10 @@ class Value {
116116

117117
private:
118118
Type *VTy;
119-
Use *UseList;
119+
union {
120+
Use *List = nullptr;
121+
unsigned Count;
122+
} Uses;
120123

121124
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
122125
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -339,21 +342,28 @@ class Value {
339342
#endif
340343
}
341344

345+
/// Check if this Value has a use-list.
346+
bool hasUseList() const { return !isa<ConstantData>(this); }
347+
342348
bool use_empty() const {
343349
assertModuleIsMaterialized();
344-
return UseList == nullptr;
350+
return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
345351
}
346352

347353
bool materialized_use_empty() const {
348-
return UseList == nullptr;
354+
return hasUseList() ? Uses.List == nullptr : !Uses.Count;
349355
}
350356

351357
using use_iterator = use_iterator_impl<Use>;
352358
using const_use_iterator = use_iterator_impl<const Use>;
353359

354-
use_iterator materialized_use_begin() { return use_iterator(UseList); }
360+
use_iterator materialized_use_begin() {
361+
assert(hasUseList());
362+
return use_iterator(Uses.List);
363+
}
355364
const_use_iterator materialized_use_begin() const {
356-
return const_use_iterator(UseList);
365+
assert(hasUseList());
366+
return const_use_iterator(Uses.List);
357367
}
358368
use_iterator use_begin() {
359369
assertModuleIsMaterialized();
@@ -380,17 +390,18 @@ class Value {
380390
return materialized_uses();
381391
}
382392

383-
bool user_empty() const {
384-
assertModuleIsMaterialized();
385-
return UseList == nullptr;
386-
}
393+
bool user_empty() const { return use_empty(); }
387394

388395
using user_iterator = user_iterator_impl<User>;
389396
using const_user_iterator = user_iterator_impl<const User>;
390397

391-
user_iterator materialized_user_begin() { return user_iterator(UseList); }
398+
user_iterator materialized_user_begin() {
399+
assert(hasUseList());
400+
return user_iterator(Uses.List);
401+
}
392402
const_user_iterator materialized_user_begin() const {
393-
return const_user_iterator(UseList);
403+
assert(hasUseList());
404+
return const_user_iterator(Uses.List);
394405
}
395406
user_iterator user_begin() {
396407
assertModuleIsMaterialized();
@@ -429,7 +440,11 @@ class Value {
429440
///
430441
/// This is specialized because it is a common request and does not require
431442
/// traversing the whole use list.
432-
bool hasOneUse() const { return hasSingleElement(uses()); }
443+
bool hasOneUse() const {
444+
if (!hasUseList())
445+
return Uses.Count == 1;
446+
return hasSingleElement(uses());
447+
}
433448

434449
/// Return true if this Value has exactly N uses.
435450
bool hasNUses(unsigned N) const;
@@ -491,6 +506,8 @@ class Value {
491506
static void dropDroppableUse(Use &U);
492507

493508
/// Check if this value is used in the specified basic block.
509+
///
510+
/// Not supported for ConstantData.
494511
bool isUsedInBasicBlock(const BasicBlock *BB) const;
495512

496513
/// This method computes the number of uses of this Value.
@@ -500,7 +517,19 @@ class Value {
500517
unsigned getNumUses() const;
501518

502519
/// This method should only be used by the Use class.
503-
void addUse(Use &U) { U.addToList(&UseList); }
520+
void addUse(Use &U) {
521+
if (hasUseList())
522+
U.addToList(Uses.List);
523+
else
524+
U.addToList(Uses.Count);
525+
}
526+
527+
void removeUse(Use &U) {
528+
if (hasUseList())
529+
U.removeFromList(Uses.List);
530+
else
531+
U.removeFromList(Uses.Count);
532+
}
504533

505534
/// Concrete subclass of this.
506535
///
@@ -841,7 +870,8 @@ class Value {
841870
///
842871
/// \return the first element in the list.
843872
///
844-
/// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
873+
/// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
874+
/// update).
845875
template <class Compare>
846876
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
847877
Use *Merged;
@@ -887,10 +917,50 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
887917
return OS;
888918
}
889919

920+
inline Use::~Use() {
921+
if (Val)
922+
Val->removeUse(*this);
923+
}
924+
925+
void Use::addToList(unsigned &Count) {
926+
assert(isa<ConstantData>(Val) && "Only ConstantData is ref-counted");
927+
++Count;
928+
929+
// We don't have a uselist - clear the remnant if we are replacing a
930+
// non-constant value.
931+
Prev = nullptr;
932+
Next = nullptr;
933+
}
934+
935+
void Use::addToList(Use *&List) {
936+
assert(!isa<ConstantData>(Val) && "ConstantData has no use-list");
937+
938+
Next = List;
939+
if (Next)
940+
Next->Prev = &Next;
941+
Prev = &List;
942+
List = this;
943+
}
944+
945+
void Use::removeFromList(unsigned &Count) {
946+
assert(isa<ConstantData>(Val));
947+
assert(Count > 0 && "reference count underflow");
948+
assert(!Prev && !Next && "should not have uselist remnant");
949+
--Count;
950+
}
951+
952+
void Use::removeFromList(Use *&List) {
953+
*Prev = Next;
954+
if (Next)
955+
Next->Prev = Prev;
956+
}
957+
890958
void Use::set(Value *V) {
891-
if (Val) removeFromList();
959+
if (Val)
960+
Val->removeUse(*this);
892961
Val = V;
893-
if (V) V->addUse(*this);
962+
if (V)
963+
V->addUse(*this);
894964
}
895965

896966
Value *Use::operator=(Value *RHS) {
@@ -904,7 +974,7 @@ const Use &Use::operator=(const Use &RHS) {
904974
}
905975

906976
template <class Compare> void Value::sortUseList(Compare Cmp) {
907-
if (!UseList || !UseList->Next)
977+
if (!hasUseList() || !Uses.List || !Uses.List->Next)
908978
// No need to sort 0 or 1 uses.
909979
return;
910980

@@ -917,10 +987,10 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
917987
Use *Slots[MaxSlots];
918988

919989
// Collect the first use, turning it into a single-item list.
920-
Use *Next = UseList->Next;
921-
UseList->Next = nullptr;
990+
Use *Next = Uses.List->Next;
991+
Uses.List->Next = nullptr;
922992
unsigned NumSlots = 1;
923-
Slots[0] = UseList;
993+
Slots[0] = Uses.List;
924994

925995
// Collect all but the last use.
926996
while (Next->Next) {
@@ -956,15 +1026,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
9561026
// Merge all the lists together.
9571027
assert(Next && "Expected one more Use");
9581028
assert(!Next->Next && "Expected only one Use");
959-
UseList = Next;
1029+
Uses.List = Next;
9601030
for (unsigned I = 0; I < NumSlots; ++I)
9611031
if (Slots[I])
962-
// Since the uses in Slots[I] originally preceded those in UseList, send
1032+
// Since the uses in Slots[I] originally preceded those in Uses.List, send
9631033
// Slots[I] in as the left parameter to maintain a stable sort.
964-
UseList = mergeUseLists(Slots[I], UseList, Cmp);
1034+
Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
9651035

9661036
// Fix the Prev pointers.
967-
for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
1037+
for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
9681038
I->Prev = Prev;
9691039
Prev = &I->Next;
9701040
}

llvm/lib/Analysis/TypeMetadataUtils.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
5454
static void findLoadCallsAtConstantOffset(
5555
const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
5656
int64_t Offset, const CallInst *CI, DominatorTree &DT) {
57+
if (!VPtr->hasUseList())
58+
return;
59+
5760
for (const Use &U : VPtr->uses()) {
5861
Value *User = U.getUser();
5962
if (isa<BitCastInst>(User)) {

llvm/lib/AsmParser/LLParser.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
88578857
//===----------------------------------------------------------------------===//
88588858
bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
88598859
SMLoc Loc) {
8860+
if (!V->hasUseList())
8861+
return false;
88608862
if (V->use_empty())
88618863
return error(Loc, "value has no uses");
88628864

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -3856,6 +3856,10 @@ Error BitcodeReader::parseUseLists() {
38563856
V = FunctionBBs[ID];
38573857
} else
38583858
V = ValueList[ID];
3859+
3860+
if (isa<ConstantData>(V))
3861+
break;
3862+
38593863
unsigned NumUses = 0;
38603864
SmallDenseMap<const Use *, unsigned, 16> Order;
38613865
for (const Use &U : V->materialized_uses()) {

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
230230

231231
static void predictValueUseListOrder(const Value *V, const Function *F,
232232
OrderMap &OM, UseListOrderStack &Stack) {
233+
if (isa<ConstantData>(V))
234+
return;
235+
233236
auto &IDPair = OM[V];
234237
assert(IDPair.first && "Unmapped value");
235238
if (IDPair.second)

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4002,7 +4002,7 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
40024002
// Globals with sub-elements such as combinations of arrays and structs
40034003
// are handled recursively by emitGlobalConstantImpl. Keep track of the
40044004
// constant symbol base and the current position with BaseCV and Offset.
4005-
if (!BaseCV && CV->hasOneUse())
4005+
if (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
40064006
BaseCV = dyn_cast<Constant>(CV->user_back());
40074007

40084008
if (isa<ConstantAggregateZero>(CV)) {

llvm/lib/CodeGen/CodeGenPrepare.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -8591,6 +8591,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
85918591
return false;
85928592

85938593
Value *X = Cmp->getOperand(0);
8594+
if (!X->hasUseList())
8595+
return false;
8596+
85948597
APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();
85958598

85968599
for (auto *U : X->users()) {

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
10341034
if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
10351035
return nullptr;
10361036

1037+
if (!R->hasUseList() || !I->hasUseList())
1038+
return nullptr;
1039+
10371040
auto CommonUser =
10381041
findCommonBetweenCollections<Value *>(R->users(), I->users());
10391042
if (!CommonUser)

llvm/lib/IR/AsmWriter.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
125125
if (OM.lookup(V))
126126
return;
127127

128-
if (const Constant *C = dyn_cast<Constant>(V))
128+
if (const Constant *C = dyn_cast<Constant>(V)) {
129+
if (isa<ConstantData>(C))
130+
return;
131+
129132
if (C->getNumOperands() && !isa<GlobalValue>(C))
130133
for (const Value *Op : C->operands())
131134
if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
132135
orderValue(Op, OM);
136+
}
133137

134138
// Note: we cannot cache this lookup above, since inserting into the map
135139
// changes the map's size, and thus affects the other IDs.
@@ -275,7 +279,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
275279
UseListOrderMap ULOM;
276280
for (const auto &Pair : OM) {
277281
const Value *V = Pair.first;
278-
if (V->use_empty() || std::next(V->use_begin()) == V->use_end())
282+
if (!V->hasUseList() || V->use_empty() ||
283+
std::next(V->use_begin()) == V->use_end())
279284
continue;
280285

281286
std::vector<unsigned> Shuffle =

llvm/lib/IR/Instruction.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
373373
}
374374

375375
bool Instruction::isOnlyUserOfAnyOperand() {
376-
return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
376+
return any_of(operands(), [](const Value *V) {
377+
return V->hasUseList() && V->hasOneUser();
378+
});
377379
}
378380

379381
void Instruction::setHasNoUnsignedWrap(bool b) {

0 commit comments

Comments
 (0)