[llvm] 4d60c6d - Reapply "IR: Remove reference counts from ConstantData (#137314)" (#138962)
via llvm-commits
llvm-commits at lists.llvm.org
Wed May 7 23:02:57 PDT 2025
Author: Matt Arsenault
Date: 2025-05-08T08:02:54+02:00
New Revision: 4d60c6d9b2c863d773aac9b59af8780e5ba23fcd
URL: https://github.com/llvm/llvm-project/commit/4d60c6d9b2c863d773aac9b59af8780e5ba23fcd
DIFF: https://github.com/llvm/llvm-project/commit/4d60c6d9b2c863d773aac9b59af8780e5ba23fcd.diff
LOG: Reapply "IR: Remove reference counts from ConstantData (#137314)" (#138962)
This reverts commit 0274232b87177779e5c985eca06df22bf140f6cb.
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 504db733308c1..05318362b99c9 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -56,7 +56,9 @@ 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
+* 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).
* 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 ff51f59b6ec68..76efa9bd63522 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -51,7 +51,8 @@ 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 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).
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 bcd1fd6677497..0d5d878e4689f 100644
--- a/llvm/include/llvm/IR/Use.h
+++ b/llvm/include/llvm/IR/Use.h
@@ -23,7 +23,6 @@
namespace llvm {
template <typename> struct simplify_type;
-class ConstantData;
class User;
class Value;
@@ -43,7 +42,7 @@ class Use {
private:
/// Destructor - Only for zap()
- ~Use();
+ ~Use() { removeFromList(); }
/// Constructor
Use(User *Parent) : Parent(Parent) {}
@@ -85,10 +84,25 @@ class Use {
Use **Prev = nullptr;
User *Parent = nullptr;
- inline void addToList(unsigned &Count);
- inline void addToList(Use *&List);
- inline void removeFromList(unsigned &Count);
- inline void removeFromList(Use *&List);
+ 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;
+ }
+ }
};
/// 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 180b6238eda6c..241b9e2860c4c 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -116,10 +116,7 @@ class Value {
private:
Type *VTy;
- union {
- Use *List = nullptr;
- unsigned Count;
- } Uses;
+ Use *UseList = nullptr;
friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
@@ -347,23 +344,21 @@ class Value {
bool use_empty() const {
assertModuleIsMaterialized();
- return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
+ return UseList == nullptr;
}
- bool materialized_use_empty() const {
- return hasUseList() ? Uses.List == nullptr : !Uses.Count;
- }
+ bool materialized_use_empty() const { return UseList == nullptr; }
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(Uses.List);
+ return use_iterator(UseList);
}
const_use_iterator materialized_use_begin() const {
assert(hasUseList());
- return const_use_iterator(Uses.List);
+ return const_use_iterator(UseList);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
@@ -397,11 +392,11 @@ class Value {
user_iterator materialized_user_begin() {
assert(hasUseList());
- return user_iterator(Uses.List);
+ return user_iterator(UseList);
}
const_user_iterator materialized_user_begin() const {
assert(hasUseList());
- return const_user_iterator(Uses.List);
+ return const_user_iterator(UseList);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
@@ -440,11 +435,7 @@ class Value {
///
/// This is specialized because it is a common request and does not require
/// traversing the whole use list.
- bool hasOneUse() const {
- if (!hasUseList())
- return Uses.Count == 1;
- return hasSingleElement(uses());
- }
+ bool hasOneUse() const { return UseList && hasSingleElement(uses()); }
/// Return true if this Value has exactly N uses.
bool hasNUses(unsigned N) const;
@@ -518,17 +509,8 @@ class Value {
/// This method should only be used by the Use class.
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);
+ if (UseList || hasUseList())
+ U.addToList(&UseList);
}
/// Concrete subclass of this.
@@ -870,8 +852,7 @@ class Value {
///
/// \return the first element in the list.
///
- /// \note Completely ignores \a Use::PrevOrCount (doesn't read, doesn't
- /// update).
+ /// \note Completely ignores \a Use::Prev (doesn't read, doesn't update).
template <class Compare>
static Use *mergeUseLists(Use *L, Use *R, Compare Cmp) {
Use *Merged;
@@ -917,47 +898,8 @@ 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)
- Val->removeUse(*this);
+ removeFromList();
Val = V;
if (V)
V->addUse(*this);
@@ -974,7 +916,7 @@ const Use &Use::operator=(const Use &RHS) {
}
template <class Compare> void Value::sortUseList(Compare Cmp) {
- if (!hasUseList() || !Uses.List || !Uses.List->Next)
+ if (!UseList || !UseList->Next)
// No need to sort 0 or 1 uses.
return;
@@ -987,10 +929,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 = Uses.List->Next;
- Uses.List->Next = nullptr;
+ Use *Next = UseList->Next;
+ UseList->Next = nullptr;
unsigned NumSlots = 1;
- Slots[0] = Uses.List;
+ Slots[0] = UseList;
// Collect all but the last use.
while (Next->Next) {
@@ -1026,15 +968,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");
- Uses.List = Next;
+ UseList = Next;
for (unsigned I = 0; I < NumSlots; ++I)
if (Slots[I])
- // Since the uses in Slots[I] originally preceded those in Uses.List, send
+ // Since the uses in Slots[I] originally preceded those in UseList, send
// Slots[I] in as the left parameter to maintain a stable sort.
- Uses.List = mergeUseLists(Slots[I], Uses.List, Cmp);
+ UseList = mergeUseLists(Slots[I], UseList, Cmp);
// Fix the Prev pointers.
- for (Use *I = Uses.List, **Prev = &Uses.List; I; I = I->Next) {
+ for (Use *I = UseList, **Prev = &UseList; 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 889e24a3f70ad..eb076960a5def 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -4004,7 +4004,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 (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
+ if (!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 610cbcb1a9b6b..7223dd845d18d 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -279,8 +279,7 @@ static UseListOrderMap predictUseListOrder(const Module *M) {
UseListOrderMap ULOM;
for (const auto &Pair : OM) {
const Value *V = Pair.first;
- if (!V->hasUseList() || V->use_empty() ||
- std::next(V->use_begin()) == V->use_end())
+ if (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 258681382f9e5..54e5e6d53e791 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -373,9 +373,7 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
}
bool Instruction::isOnlyUserOfAnyOperand() {
- return any_of(operands(), [](const Value *V) {
- return V->hasUseList() && V->hasOneUser();
- });
+ return any_of(operands(), [](const Value *V) { return V->hasOneUser(); });
}
void Instruction::setHasNoUnsignedWrap(bool b) {
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 74a96051f33af..d6cb65d94a11d 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -148,14 +148,18 @@ void Value::destroyValueName() {
}
bool Value::hasNUses(unsigned N) const {
- if (!hasUseList())
- return Uses.Count == N;
+ if (!UseList)
+ return N == 0;
+
+ // TODO: Disallow for ConstantData and remove !UseList check?
return hasNItems(use_begin(), use_end(), N);
}
bool Value::hasNUsesOrMore(unsigned N) const {
- if (!hasUseList())
- return Uses.Count >= N;
+ // TODO: Disallow for ConstantData and remove !UseList check?
+ if (!UseList)
+ return N == 0;
+
return hasNItemsOrMore(use_begin(), use_end(), N);
}
@@ -259,9 +263,9 @@ bool Value::isUsedInBasicBlock(const BasicBlock *BB) const {
}
unsigned Value::getNumUses() const {
- if (!hasUseList())
- return Uses.Count;
-
+ // TODO: Disallow for ConstantData and remove !UseList check?
+ if (!UseList)
+ return 0;
return (unsigned)std::distance(use_begin(), use_end());
}
@@ -522,7 +526,7 @@ void Value::doRAUW(Value *New, ReplaceMetadataUses ReplaceMetaUses) {
ValueAsMetadata::handleRAUW(this, New);
while (!materialized_use_empty()) {
- Use &U = *Uses.List;
+ Use &U = *UseList;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
@@ -1102,12 +1106,12 @@ const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
LLVMContext &Value::getContext() const { return VTy->getContext(); }
void Value::reverseUseList() {
- if (!Uses.List || !Uses.List->Next || !hasUseList())
+ if (!UseList || !UseList->Next)
// No need to reverse 0 or 1 uses.
return;
- Use *Head = Uses.List;
- Use *Current = Uses.List->Next;
+ Use *Head = UseList;
+ Use *Current = UseList->Next;
Head->Next = nullptr;
while (Current) {
Use *Next = Current->Next;
@@ -1116,8 +1120,8 @@ void Value::reverseUseList() {
Head = Current;
Current = Next;
}
- Uses.List = Head;
- Head->Prev = &Uses.List;
+ UseList = Head;
+ Head->Prev = &UseList;
}
bool Value::isSwiftError() const {
diff --git a/llvm/unittests/IR/ConstantsTest.cpp b/llvm/unittests/IR/ConstantsTest.cpp
index a46178abd9066..41cc212f00de6 100644
--- a/llvm/unittests/IR/ConstantsTest.cpp
+++ b/llvm/unittests/IR/ConstantsTest.cpp
@@ -21,6 +21,44 @@
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