Skip to content

Commit 0274232

Browse files
committed
Revert "IR: Remove reference counts from ConstantData (#137314)"
This reverts commit 51a3bd9. Possible breaks the build: https://lab.llvm.org/buildbot/#/builders/24/builds/8119/steps/9/logs/stdio
1 parent 1897023 commit 0274232

File tree

9 files changed

+105
-103
lines changed

9 files changed

+105
-103
lines changed

llvm/docs/ReleaseNotes.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ 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. Use
60-
count APIs will behave as if they have no uses (i.e. use_empty() is
61-
always true).
59+
* It is no longer permitted to inspect the uses of ConstantData
6260

6361
* The `nocapture` attribute has been replaced by `captures(none)`.
6462
* The constant expression variants of the following instructions have been

llvm/include/llvm/IR/Constants.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ template <class ConstantClass> struct ConstantAggrKeyType;
5151
/// Since they can be in use by unrelated modules (and are never based on
5252
/// GlobalValues), it never makes sense to RAUW them.
5353
///
54-
/// These do not have use lists. It is illegal to inspect the uses. These behave
55-
/// as if they have no uses (i.e. use_empty() is always true).
54+
/// These do not have use lists. It is illegal to inspect the uses.
5655
class ConstantData : public Constant {
5756
constexpr static IntrusiveOperandsAllocMarker AllocMarker{0};
5857

llvm/include/llvm/IR/Use.h

Lines changed: 6 additions & 20 deletions
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,7 +43,7 @@ class Use {
4243

4344
private:
4445
/// Destructor - Only for zap()
45-
~Use() { removeFromList(); }
46+
~Use();
4647

4748
/// Constructor
4849
Use(User *Parent) : Parent(Parent) {}
@@ -84,25 +85,10 @@ class Use {
8485
Use **Prev = nullptr;
8586
User *Parent = nullptr;
8687

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

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

llvm/include/llvm/IR/Value.h

Lines changed: 78 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,10 @@ class Value {
116116

117117
private:
118118
Type *VTy;
119-
Use *UseList = nullptr;
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.
@@ -344,21 +347,23 @@ class Value {
344347

345348
bool use_empty() const {
346349
assertModuleIsMaterialized();
347-
return UseList == nullptr;
350+
return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
348351
}
349352

350-
bool materialized_use_empty() const { return UseList == nullptr; }
353+
bool materialized_use_empty() const {
354+
return hasUseList() ? Uses.List == nullptr : !Uses.Count;
355+
}
351356

352357
using use_iterator = use_iterator_impl<Use>;
353358
using const_use_iterator = use_iterator_impl<const Use>;
354359

355360
use_iterator materialized_use_begin() {
356361
assert(hasUseList());
357-
return use_iterator(UseList);
362+
return use_iterator(Uses.List);
358363
}
359364
const_use_iterator materialized_use_begin() const {
360365
assert(hasUseList());
361-
return const_use_iterator(UseList);
366+
return const_use_iterator(Uses.List);
362367
}
363368
use_iterator use_begin() {
364369
assertModuleIsMaterialized();
@@ -392,11 +397,11 @@ class Value {
392397

393398
user_iterator materialized_user_begin() {
394399
assert(hasUseList());
395-
return user_iterator(UseList);
400+
return user_iterator(Uses.List);
396401
}
397402
const_user_iterator materialized_user_begin() const {
398403
assert(hasUseList());
399-
return const_user_iterator(UseList);
404+
return const_user_iterator(Uses.List);
400405
}
401406
user_iterator user_begin() {
402407
assertModuleIsMaterialized();
@@ -435,7 +440,11 @@ class Value {
435440
///
436441
/// This is specialized because it is a common request and does not require
437442
/// traversing the whole use list.
438-
bool hasOneUse() const { return UseList && hasSingleElement(uses()); }
443+
bool hasOneUse() const {
444+
if (!hasUseList())
445+
return Uses.Count == 1;
446+
return hasSingleElement(uses());
447+
}
439448

440449
/// Return true if this Value has exactly N uses.
441450
bool hasNUses(unsigned N) const;
@@ -509,8 +518,17 @@ class Value {
509518

510519
/// This method should only be used by the Use class.
511520
void addUse(Use &U) {
512-
if (UseList || hasUseList())
513-
U.addToList(&UseList);
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);
514532
}
515533

516534
/// Concrete subclass of this.
@@ -852,7 +870,8 @@ class Value {
852870
///
853871
/// \return the first element in the list.
854872
///
855-
/// \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).
856875
template <class Compare>
857876
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
858877
Use *Merged;
@@ -898,8 +917,47 @@ inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {
898917
return OS;
899918
}
900919

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+
901958
void Use::set(Value *V) {
902-
removeFromList();
959+
if (Val)
960+
Val->removeUse(*this);
903961
Val = V;
904962
if (V)
905963
V->addUse(*this);
@@ -916,7 +974,7 @@ const Use &Use::operator=(const Use &RHS) {
916974
}
917975

918976
template <class Compare> void Value::sortUseList(Compare Cmp) {
919-
if (!UseList || !UseList->Next)
977+
if (!hasUseList() || !Uses.List || !Uses.List->Next)
920978
// No need to sort 0 or 1 uses.
921979
return;
922980

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

931989
// Collect the first use, turning it into a single-item list.
932-
Use *Next = UseList->Next;
933-
UseList->Next = nullptr;
990+
Use *Next = Uses.List->Next;
991+
Uses.List->Next = nullptr;
934992
unsigned NumSlots = 1;
935-
Slots[0] = UseList;
993+
Slots[0] = Uses.List;
936994

937995
// Collect all but the last use.
938996
while (Next->Next) {
@@ -968,15 +1026,15 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
9681026
// Merge all the lists together.
9691027
assert(Next && "Expected one more Use");
9701028
assert(!Next->Next && "Expected only one Use");
971-
UseList = Next;
1029+
Uses.List = Next;
9721030
for (unsigned I = 0; I < NumSlots; ++I)
9731031
if (Slots[I])
974-
// 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
9751033
// Slots[I] in as the left parameter to maintain a stable sort.
976-
UseList = mergeUseLists(Slots[I], UseList, Cmp);
1034+
Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
9771035

9781036
// Fix the Prev pointers.
979-
for (Use *I = UseList, **Prev = &UseList; I; I = I->Next) {
1037+
for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
9801038
I->Prev = Prev;
9811039
Prev = &I->Next;
9821040
}

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

Lines changed: 1 addition & 1 deletion
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/IR/AsmWriter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
279279
UseListOrderMap ULOM;
280280
for (const auto &Pair : OM) {
281281
const Value *V = Pair.first;
282-
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())
283284
continue;
284285

285286
std::vector<unsigned> Shuffle =

llvm/lib/IR/Instruction.cpp

Lines changed: 3 additions & 1 deletion
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(), [](const 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) {

llvm/lib/IR/Value.cpp

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,18 +148,14 @@ void Value::destroyValueName() {
148148
}
149149

150150
bool Value::hasNUses(unsigned N) const {
151-
if (!UseList)
152-
return N == 0;
153-
154-
// TODO: Disallow for ConstantData and remove !UseList check?
151+
if (!hasUseList())
152+
return Uses.Count == N;
155153
return hasNItems(use_begin(), use_end(), N);
156154
}
157155

158156
bool Value::hasNUsesOrMore(unsigned N) const {
159-
// TODO: Disallow for ConstantData and remove !UseList check?
160-
if (!UseList)
161-
return N == 0;
162-
157+
if (!hasUseList())
158+
return Uses.Count >= N;
163159
return hasNItemsOrMore(use_begin(), use_end(), N);
164160
}
165161

@@ -263,9 +259,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
263259
}
264260

265261
unsigned Value::getNumUses() const {
266-
// TODO: Disallow for ConstantData and remove !UseList check?
267-
if (!UseList)
268-
return 0;
262+
if (!hasUseList())
263+
return Uses.Count;
264+
269265
return (unsigned)std::distance(use_begin(), use_end());
270266
}
271267

@@ -526,7 +522,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
526522
ValueAsMetadata::handleRAUW(this, New);
527523

528524
while (!materialized_use_empty()) {
529-
Use &U = *UseList;
525+
Use &U = *Uses.List;
530526
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
531527
// constant because they are uniqued.
532528
if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -1106,12 +1102,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
11061102
LLVMContext &Value::getContext() const { return VTy->getContext(); }
11071103

11081104
void Value::reverseUseList() {
1109-
if (!UseList || !UseList->Next)
1105+
if (!Uses.List || !Uses.List->Next || !hasUseList())
11101106
// No need to reverse 0 or 1 uses.
11111107
return;
11121108

1113-
Use *Head = UseList;
1114-
Use *Current = UseList->Next;
1109+
Use *Head = Uses.List;
1110+
Use *Current = Uses.List->Next;
11151111
Head->Next = nullptr;
11161112
while (Current) {
11171113
Use *Next = Current->Next;
@@ -1120,8 +1116,8 @@ void Value::reverseUseList() {
11201116
Head = Current;
11211117
Current = Next;
11221118
}
1123-
UseList = Head;
1124-
Head->Prev = &UseList;
1119+
Uses.List = Head;
1120+
Head->Prev = &Uses.List;
11251121
}
11261122

11271123
bool Value::isSwiftError() const {

0 commit comments

Comments
 (0)