Reapply "IR: Remove reference counts from ConstantData (#137314)" #138962
Conversation
This file contains hidden or 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 stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-llvm-ir Author: Matt Arsenault (arsenm) ChangesThis reverts commit 0274232. Full diff: https://github.com/llvm/llvm-project/pull/138962.diff 9 Files Affected:
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);
|
arsenm
force-pushed
the
users/arsenm/reapply/ir/remove-uselist-for-constantdata
branch
from
4fb5018 to
38540d2
Compare
Base automatically changed from
users/arsenm/reapply/ir/remove-uselist-for-constantdata
to
main
May 8, 2025 06:00
This reverts commit 0274232.
arsenm
force-pushed
the
users/arsenm/reapply/ir/remove-refcounts-constantdata
branch
from
8403b13 to
8d3b0f2
Compare
petrhosek
pushed a commit
to petrhosek/llvm-project
that referenced
this pull request
May 8, 2025
…llvm#138962) This reverts commit 0274232.
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 reverts commit 0274232.