llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-ir Author: Matt Arsenault (arsenm) <details> <summary>Changes</summary> This reverts commit 0274232b87177779e5c985eca06df22bf140f6cb. --- Full diff: https://github.com/llvm/llvm-project/pull/138962.diff 9 Files Affected: - (modified) llvm/docs/ReleaseNotes.md (+3-1) - (modified) llvm/include/llvm/IR/Constants.h (+2-1) - (modified) llvm/include/llvm/IR/Use.h (+20-6) - (modified) llvm/include/llvm/IR/Value.h (+20-78) - (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+1-1) - (modified) llvm/lib/IR/AsmWriter.cpp (+1-2) - (modified) llvm/lib/IR/Instruction.cpp (+1-3) - (modified) llvm/lib/IR/Value.cpp (+17-13) - (modified) llvm/unittests/IR/ConstantsTest.cpp (+38) ``````````diff 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); `````````` </details> https://github.com/llvm/llvm-project/pull/138962 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
