Skip to content

IR: Remove uselist for constantdata #134692

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

Closed
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
23 changes: 6 additions & 17 deletions llvm/include/llvm/IR/Use.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
namespace llvm {

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

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

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

/// Constructor
Use(User *Parent) : Parent(Parent) {}
Expand Down Expand Up @@ -87,19 +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() {
*Prev = Next;
if (Next)
Next->Prev = Prev;
}
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
Expand Down
118 changes: 94 additions & 24 deletions llvm/include/llvm/IR/Value.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ class Value {

private:
Type *VTy;
Use *UseList;
union {
Use *List = nullptr;
unsigned Count;
} Uses;

friend class ValueAsMetadata; // Allow access to IsUsedByMD.
friend class ValueHandleBase; // Allow access to HasValueHandle.
Expand Down Expand Up @@ -341,21 +344,28 @@ class Value {
#endif
}

/// Check if this Value has a use-list.
bool hasUseList() const { return !isa<ConstantData>(this); }

bool use_empty() const {
assertModuleIsMaterialized();
return UseList == nullptr;
return hasUseList() ? Uses.List == nullptr : Uses.Count == 0;
}

bool materialized_use_empty() const {
return UseList == nullptr;
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() { return use_iterator(UseList); }
use_iterator materialized_use_begin() {
assert(hasUseList());
return use_iterator(Uses.List);
}
const_use_iterator materialized_use_begin() const {
return const_use_iterator(UseList);
assert(hasUseList());
return const_use_iterator(Uses.List);
}
use_iterator use_begin() {
assertModuleIsMaterialized();
Expand All @@ -382,17 +392,18 @@ class Value {
return materialized_uses();
}

bool user_empty() const {
assertModuleIsMaterialized();
return UseList == nullptr;
}
bool user_empty() const { return use_empty(); }

using user_iterator = user_iterator_impl<User>;
using const_user_iterator = user_iterator_impl<const User>;

user_iterator materialized_user_begin() { return user_iterator(UseList); }
user_iterator materialized_user_begin() {
assert(hasUseList());
return user_iterator(Uses.List);
}
const_user_iterator materialized_user_begin() const {
return const_user_iterator(UseList);
assert(hasUseList());
return const_user_iterator(Uses.List);
}
user_iterator user_begin() {
assertModuleIsMaterialized();
Expand Down Expand Up @@ -431,7 +442,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 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;
Expand Down Expand Up @@ -493,6 +508,8 @@ class Value {
static void dropDroppableUse(Use &U);

/// Check if this value is used in the specified basic block.
///
/// Not supported for ConstantData.
bool isUsedInBasicBlock(const BasicBlock *BB) const;

/// This method computes the number of uses of this Value.
Expand All @@ -502,7 +519,19 @@ class Value {
unsigned getNumUses() const;

/// This method should only be used by the Use class.
void addUse(Use &U) { U.addToList(&UseList); }
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);
}

/// Concrete subclass of this.
///
Expand Down Expand Up @@ -843,7 +872,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;
Expand Down Expand Up @@ -889,10 +919,50 @@ 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain in more detail when this is necessary? It seems odd to me that addToList() should be responsible for nulling out these pointers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to revisit this. I think this was only a problem in cases where Use::swap was used between a register and a constant

}

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) removeFromList();
if (Val)
Val->removeUse(*this);
Val = V;
if (V) V->addUse(*this);
if (V)
V->addUse(*this);
}

Value *Use::operator=(Value *RHS) {
Expand All @@ -906,7 +976,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;

Expand All @@ -919,10 +989,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) {
Expand Down Expand Up @@ -958,15 +1028,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;
}
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Analysis/TypeMetadataUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
static void findLoadCallsAtConstantOffset(
const Module *M, SmallVectorImpl<DevirtCallSite> &DevirtCalls, Value *VPtr,
int64_t Offset, const CallInst *CI, DominatorTree &DT) {
if (!VPtr->hasUseList())
return;

for (const Use &U : VPtr->uses()) {
Value *User = U.getUser();
if (isa<BitCastInst>(User)) {
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8857,6 +8857,8 @@ bool LLParser::parseMDNodeVector(SmallVectorImpl<Metadata *> &Elts) {
//===----------------------------------------------------------------------===//
bool LLParser::sortUseListOrder(Value *V, ArrayRef<unsigned> Indexes,
SMLoc Loc) {
if (isa<ConstantData>(V))
Copy link
Contributor

Choose a reason for hiding this comment

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

hasUseList() more natural here and next two files?

return false;
if (V->use_empty())
return error(Loc, "value has no uses");

Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3856,6 +3856,10 @@ Error BitcodeReader::parseUseLists() {
V = FunctionBBs[ID];
} else
V = ValueList[ID];

if (isa<ConstantData>(V))
break;

unsigned NumUses = 0;
SmallDenseMap<const Use *, unsigned, 16> Order;
for (const Use &U : V->materialized_uses()) {
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,

static void predictValueUseListOrder(const Value *V, const Function *F,
OrderMap &OM, UseListOrderStack &Stack) {
if (isa<ConstantData>(V))
return;

auto &IDPair = OM[V];
assert(IDPair.first && "Unmapped value");
if (IDPair.second)
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 @@ -3953,7 +3953,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)) {
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/CodeGen/CodeGenPrepare.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8547,6 +8547,9 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
return false;

Value *X = Cmp->getOperand(0);
if (!X->hasUseList())
return false;

APInt CmpC = cast<ConstantInt>(Cmp->getOperand(1))->getValue();

for (auto *U : X->users()) {
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,9 @@ ComplexDeinterleavingGraph::identifyPartialReduction(Value *R, Value *I) {
if (!isa<VectorType>(R->getType()) || !isa<VectorType>(I->getType()))
return nullptr;

if (!R->hasUseList() || !I->hasUseList())
return nullptr;

auto CommonUser =
findCommonBetweenCollections<Value *>(R->users(), I->users());
if (!CommonUser)
Expand Down
9 changes: 7 additions & 2 deletions llvm/lib/IR/AsmWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,15 @@ static void orderValue(const Value *V, OrderMap &OM) {
if (OM.lookup(V))
return;

if (const Constant *C = dyn_cast<Constant>(V))
if (const Constant *C = dyn_cast<Constant>(V)) {
if (isa<ConstantData>(C))
return;

if (C->getNumOperands() && !isa<GlobalValue>(C))
for (const Value *Op : C->operands())
if (!isa<BasicBlock>(Op) && !isa<GlobalValue>(Op))
orderValue(Op, OM);
}

// Note: we cannot cache this lookup above, since inserting into the map
// changes the map's size, and thus affects the other IDs.
Expand Down Expand Up @@ -275,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 =
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/IR/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ std::optional<BasicBlock::iterator> Instruction::getInsertionPointAfterDef() {
}

bool Instruction::isOnlyUserOfAnyOperand() {
return any_of(operands(), [](Value *V) { return V->hasOneUser(); });
return any_of(operands(), [](const Value *V) {
return V->hasUseList() && V->hasOneUser();
});
}

void Instruction::setHasNoUnsignedWrap(bool b) {
Expand Down
8 changes: 6 additions & 2 deletions llvm/lib/IR/Use.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ void Use::swap(Use &RHS) {
std::swap(Next, RHS.Next);
std::swap(Prev, RHS.Prev);

*Prev = this;
if (Prev)
*Prev = this;

if (Next)
Next->Prev = &Next;

*RHS.Prev = &RHS;
if (RHS.Prev)
*RHS.Prev = &RHS;

if (RHS.Next)
RHS.Next->Prev = &RHS.Next;
}
Expand Down
Loading
Loading