[llvm] 0274232 - Revert "IR: Remove reference counts from ConstantData (#137314)"
Kirill Stoimenov via llvm-commits
llvm-commits at lists.llvm.org
Tue May 6 17:08:12 PDT 2025
Author: Kirill Stoimenov
Date: 2025-05-07T00:07:55Z
New Revision: 0274232b87177779e5c985eca06df22bf140f6cb
URL: https://github.com/llvm/llvm-project/commit/0274232b87177779e5c985eca06df22bf140f6cb
DIFF: https://github.com/llvm/llvm-project/commit/0274232b87177779e5c985eca06df22bf140f6cb.diff
LOG: Revert "IR: Remove reference counts from ConstantData (#137314)"
This reverts commit 51a3bd919d68a8fb1b026377d6e86b1523d37433.
Possible breaks the build: https://lab.llvm.org/buildbot/#/builders/24/builds/8119/steps/9/logs/stdio
Added:
Modified:
llvm/docs/ReleaseNotes.md
llvm/include/llvm/IR/Constants.h
llvm/include/llvm/IR/Use.h
llvm/include/llvm/IR/Value.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/IR/Value.cpp
llvm/unittests/IR/ConstantsTest.cpp
Removed:
################################################################################
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index 05318362b99c9..504db733308c1 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -56,9 +56,7 @@ Makes programs 10x faster by doing Special New Thing.
Changes to the LLVM IR
----------------------
-* It is no longer permitted to inspect the uses of ConstantData. Use
- count APIs will behave as if they have no uses (i.e. use_empty() is
- always true).
+* It is no longer permitted to inspect the uses of ConstantData
* The `nocapture` attribute has been replaced by `captures(none)`.
* The constant expression variants of the following instructions have been
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index 76efa9bd63522..ff51f59b6ec68 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -51,8 +51,7 @@ template <class ConstantClass> struct ConstantAggrKeyType;
/// Since they can be in use by unrelated modules (and are never based on
/// GlobalValues), it never makes sense to RAUW them.
///
-/// These do not have use lists. It is illegal to inspect the uses. These behave
-/// as if they have no uses (i.e. use_empty() is always true).
+/// These do not have use lists. It is illegal to inspect the uses.
class ConstantData : public Constant {
constexpr static IntrusiveOperandsAllocMarker AllocMarker{0};
diff --git a/llvm/include/llvm/IR/Use.h b/llvm/include/llvm/IR/Use.h
index 0d5d878e4689f..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,7 +43,7 @@ class Use {
private:
/// Destructor - Only for zap()
- ~Use() { removeFromList(); }
+ ~Use();
/// Constructor
Use(User *Parent) : Parent(Parent) {}
@@ -84,25 +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() {
- if (Prev) {
- *Prev = Next;
- if (Next) {
- Next->Prev = Prev;
- Next = nullptr;
- }
-
- Prev = nullptr;
- }
- }
+ 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 241b9e2860c4c..180b6238eda6c 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 = nullptr;
+ union {
+ Use *List = nullptr;
+ unsigned Count;
+ } Uses;
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -344,21 +347,23 @@ class Value {
bool use_empty() const {
assertModuleIsMaterialized();
- return UseList == nullptr;
+ return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
}
- bool materialized_use_empty() const { return UseList == nullptr; }
+ bool materialized_use_empty() const {
+ 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() {
assert(hasUseList());
- return use_iterator(UseList);
+ return use_iterator(Uses.List);
}
const_use_iterator materialized_use_begin() const {
assert(hasUseList());
- return const_use_iterator(UseList);
+ return const_use_iterator(Uses.List);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
@@ -392,11 +397,11 @@ class Value {
user_iterator materialized_user_begin() {
assert(hasUseList());
- return user_iterator(UseList);
+ return user_iterator(Uses.List);
}
const_user_iterator materialized_user_begin() const {
assert(hasUseList());
- return const_user_iterator(UseList);
+ return const_user_iterator(Uses.List);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
@@ -435,7 +440,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 UseList && 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;
@@ -509,8 +518,17 @@ class Value {
/// This method should only be used by the Use class.
void addUse(Use &U) {
- if (UseList || hasUseList())
- U.addToList(&UseList);
+ 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.
@@ -852,7 +870,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;
@@ -898,8 +917,47 @@ 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) {
- removeFromList();
+ if (Val)
+ Val->removeUse(*this);
Val = V;
if (V)
V->addUse(*this);
@@ -916,7 +974,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;
@@ -929,10 +987,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) {
@@ -968,15 +1026,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/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index bdcd54a135da9..4b0e5893b34e0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -4002,7 +4002,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/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 7223dd845d18d..610cbcb1a9b6b 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -279,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 54e5e6d53e791..258681382f9e5 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(), [](const 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/Value.cpp b/llvm/lib/IR/Value.cpp
index d6cb65d94a11d..74a96051f33af 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -148,18 +148,14 @@ void Value::destroyValueName() {
}
bool Value::hasNUses(unsigned N) const {
- if (!UseList)
- return N == 0;
-
- // TODO: Disallow for ConstantData and remove !UseList check?
+ if (!hasUseList())
+ return Uses.Count == N;
return hasNItems(use_begin(), use_end(), N);
}
bool Value::hasNUsesOrMore(unsigned N) const {
- // TODO: Disallow for ConstantData and remove !UseList check?
- if (!UseList)
- return N == 0;
-
+ if (!hasUseList())
+ return Uses.Count >= N;
return hasNItemsOrMore(use_begin(), use_end(), N);
}
@@ -263,9 +259,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
}
unsigned Value::getNumUses() const {
- // TODO: Disallow for ConstantData and remove !UseList check?
- if (!UseList)
- return 0;
+ if (!hasUseList())
+ return Uses.Count;
+
return (unsigned)std::distance(use_begin(), use_end());
}
@@ -526,7 +522,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())) {
@@ -1106,12 +1102,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;
@@ -1120,8 +1116,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/unittests/IR/ConstantsTest.cpp b/llvm/unittests/IR/ConstantsTest.cpp
index 41cc212f00de6..a46178abd9066 100644
--- a/llvm/unittests/IR/ConstantsTest.cpp
+++ b/llvm/unittests/IR/ConstantsTest.cpp
@@ -21,44 +21,6 @@
namespace llvm {
namespace {
-// Check that use count checks treat ConstantData like they have no uses.
-TEST(ConstantsTest, UseCounts) {
- LLVMContext Context;
- Type *Int32Ty = Type::getInt32Ty(Context);
- Constant *Zero = ConstantInt::get(Int32Ty, 0);
-
- EXPECT_TRUE(Zero->use_empty());
- EXPECT_EQ(Zero->getNumUses(), 0u);
- EXPECT_TRUE(Zero->hasNUses(0));
- EXPECT_FALSE(Zero->hasOneUse());
- EXPECT_FALSE(Zero->hasOneUser());
- EXPECT_FALSE(Zero->hasNUses(1));
- EXPECT_FALSE(Zero->hasNUsesOrMore(1));
- EXPECT_FALSE(Zero->hasNUses(2));
- EXPECT_FALSE(Zero->hasNUsesOrMore(2));
-
- std::unique_ptr<Module> M(new Module("MyModule", Context));
-
- // Introduce some uses
- new GlobalVariable(*M, Int32Ty, /*isConstant=*/false,
- GlobalValue::ExternalLinkage, /*Initializer=*/Zero,
- "gv_user0");
- new GlobalVariable(*M, Int32Ty, /*isConstant=*/false,
- GlobalValue::ExternalLinkage, /*Initializer=*/Zero,
- "gv_user1");
-
- // Still looks like use_empty with uses.
- EXPECT_TRUE(Zero->use_empty());
- EXPECT_EQ(Zero->getNumUses(), 0u);
- EXPECT_TRUE(Zero->hasNUses(0));
- EXPECT_FALSE(Zero->hasOneUse());
- EXPECT_FALSE(Zero->hasOneUser());
- EXPECT_FALSE(Zero->hasNUses(1));
- EXPECT_FALSE(Zero->hasNUsesOrMore(1));
- EXPECT_FALSE(Zero->hasNUses(2));
- EXPECT_FALSE(Zero->hasNUsesOrMore(2));
-}
-
TEST(ConstantsTest, Integer_i1) {
LLVMContext Context;
IntegerType *Int1 = IntegerType::get(Context, 1);
More information about the llvm-commits
mailing list