Skip to content

IR: Remove reference counts from ConstantData #137314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: users/arsenm/ir/remove-uselist-for-constantdata
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llvm/docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
9 changes: 3 additions & 6 deletions llvm/include/llvm/IR/Use.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
namespace llvm {

template <typename> struct simplify_type;
class ConstantData;
class User;
class Value;

Expand All @@ -43,7 +42,7 @@ class Use {

private:
/// Destructor - Only for zap()
~Use();
~Use() { removeFromList(); }

/// Constructor
Use(User *Parent) : Parent(Parent) {}
Expand Down Expand Up @@ -85,10 +84,8 @@ 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);
inline void addToList(Use **List);
inline void removeFromList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous PR moved the method away from here -- should move them back again? I think it makes more sense to define them here than in Value.h.

};

/// Allow clients to treat uses just like values when using
Expand Down
118 changes: 40 additions & 78 deletions llvm/include/llvm/IR/Value.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the UseList || here? Isn't hasUseList() sufficient? Or is this just an optimization?

U.addToList(&UseList);
}

/// Concrete subclass of this.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -973,8 +915,28 @@ const Use &Use::operator=(const Use &RHS) {
return *this;
}

void Use::addToList(Use **List) {
Next = *List;
if (Next)
Next->Prev = &Next;
Prev = List;
*Prev = this;
}

void Use::removeFromList() {
if (Prev) {
*Prev = Next;
if (Next) {
Next->Prev = Prev;
Next = nullptr;
}

Prev = nullptr;
}
}

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;

Expand All @@ -987,10 +949,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) {
Expand Down Expand Up @@ -1026,15 +988,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;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 (!isa<ConstantData>(CV) && !BaseCV && CV->hasOneUse())
if (!BaseCV && CV->hasOneUse())
BaseCV = dyn_cast<Constant>(CV->user_back());

if (isa<ConstantAggregateZero>(CV)) {
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/IR/AsmWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/IR/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
30 changes: 17 additions & 13 deletions llvm/lib/IR/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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())) {
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
Loading
Loading