Skip to content

[SelectionDAG] Split SDNode::use_iterator into user_iterator and use_iterator. #120531

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 19, 2024
Merged

[SelectionDAG] Split SDNode::use_iterator into user_iterator and use_iterator. #120531

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18e1928
[SelectionDAG] Split sd_use_iterator into sd_user_iterator and sd_use…
topperc Dec 19, 2024
e4962ca
Update llvm/include/llvm/CodeGen/SelectionDAGNodes.h
topperc Dec 19, 2024
856a879
fixup! Remove atEnd.
topperc Dec 19, 2024
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
81 changes: 61 additions & 20 deletions llvm/include/llvm/CodeGen/SelectionDAGNodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ END_TWO_BYTE_PACK()
bool use_empty() const { return UseList == nullptr; }

/// Return true if there is exactly one use of this node.
bool hasOneUse() const { return hasSingleElement(users()); }
bool hasOneUse() const { return hasSingleElement(uses()); }

/// Return the number of uses of this node. This method takes
/// time proportional to the number of uses.
Expand Down Expand Up @@ -821,14 +821,12 @@ END_TWO_BYTE_PACK()
}

/// Retrieve a pointer to the current user node.
SDNode *operator*() const {
SDUse &operator*() const {
assert(Op && "Cannot dereference end iterator!");
return Op->getUser();
return *Op;
}

SDNode *operator->() const { return operator*(); }

SDUse &getUse() const { return *Op; }
SDUse *operator->() const { return &operator*(); }

/// Retrieve the operand # of this use in its user.
unsigned getOperandNo() const {
Expand All @@ -837,29 +835,72 @@ END_TWO_BYTE_PACK()
}
};

class user_iterator {
friend class SDNode;
use_iterator UI;

explicit user_iterator(SDUse *op) : UI(op) {};

public:
using iterator_category = std::forward_iterator_tag;
using value_type = SDNode *;
using difference_type = std::ptrdiff_t;
using pointer = value_type *;
using reference = value_type &;

user_iterator() = default;

bool operator==(const user_iterator &x) const { return UI == x.UI; }
bool operator!=(const user_iterator &x) const { return !operator==(x); }

// /// Return true if this iterator is at the end of the uses list.
bool atEnd() const { return *this == user_iterator(); }
Copy link
Contributor

@jayfoad jayfoad Dec 19, 2024

Choose a reason for hiding this comment

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

Do we need this? Seems like a weird thing for an iterator to provide. If we do, the implementation could just delegate to UI.atEnd().

Copy link
Contributor

@s-barannikov s-barannikov Dec 19, 2024

Choose a reason for hiding this comment

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

Is it used somewhere?

Copy link
Collaborator Author

@topperc topperc Dec 19, 2024

Choose a reason for hiding this comment

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

It may not be used anywhere. It exists in use_iterator and the IR equivalent of use/user_iterator. The implementation is based on IR's user_iterator which defers to use_iterator.

Copy link
Collaborator Author

@topperc topperc Dec 19, 2024

Choose a reason for hiding this comment

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

I just noticed that IR only has this on user_iterator not on use_iterator. Not sure what its used for. Looks unused for SelectionDAG so I'll just delete.


user_iterator &operator++() { // Preincrement
++UI;
return *this;
}

user_iterator operator++(int) { // Postincrement
auto tmp = *this;
++*this;
return tmp;
}

// Retrieve a pointer to the current User.
SDNode *operator*() const { return UI->getUser(); }

SDNode *operator->() const { return operator*(); }

SDUse &getUse() const { return *UI; }
Copy link
Contributor

@jayfoad jayfoad Dec 19, 2024

Choose a reason for hiding this comment

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

Do we need this? If someone wants the Use, shouldn't they use use_iterator in the first place?

Copy link
Collaborator Author

@topperc topperc Dec 19, 2024

Choose a reason for hiding this comment

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

I think I used it in one place where I changed the iterator from use_iterator to user_iterator, but I could change that. I put it in the interface because it exists in the IR equivalent.

};

/// Provide iteration support to walk over all uses of an SDNode.
use_iterator use_begin() const {
return use_iterator(UseList);
}

static use_iterator use_end() { return use_iterator(nullptr); }

/// Provide iteration support to walk over all users of an SDNode.
/// For now, this should only be used to get a pointer to the first user.
/// FIXME: Rename use_iterator to user_iterator. Add user_end().
use_iterator user_begin() const { return use_iterator(UseList); }

// Dereferencing use_iterator returns the user SDNode* making it closer to a
// user_iterator thus this function is called users() to reflect that.
// FIXME: Rename to user_iterator and introduce a use_iterator that returns
// SDUse*.
inline iterator_range<use_iterator> users() {
inline iterator_range<use_iterator> uses() {
return make_range(use_begin(), use_end());
}
inline iterator_range<use_iterator> users() const {
inline iterator_range<use_iterator> uses() const {
return make_range(use_begin(), use_end());
}

/// Provide iteration support to walk over all users of an SDNode.
user_iterator user_begin() const { return user_iterator(UseList); }

static user_iterator user_end() { return user_iterator(nullptr); }

inline iterator_range<user_iterator> users() {
return make_range(user_begin(), user_end());
}
inline iterator_range<user_iterator> users() const {
return make_range(user_begin(), user_end());
}

/// Return true if there are exactly NUSES uses of the indicated value.
/// This method ignores uses of other values defined by this operation.
bool hasNUsesOfValue(unsigned NUses, unsigned Value) const;
Expand Down Expand Up @@ -1019,9 +1060,9 @@ END_TWO_BYTE_PACK()
/// If this node has a glue value with a user, return
/// the user (there is at most one). Otherwise return NULL.
SDNode *getGluedUser() const {
for (use_iterator UI = use_begin(), UE = use_end(); UI != UE; ++UI)
if (UI.getUse().get().getValueType() == MVT::Glue)
return *UI;
for (SDUse &U : uses())
if (U.getValueType() == MVT::Glue)
return U.getUser();
return nullptr;
}

Expand Down
39 changes: 17 additions & 22 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13229,12 +13229,11 @@ static bool ExtendUsesToFormExtLoad(EVT VT, SDNode *N, SDValue N0,
const TargetLowering &TLI) {
bool HasCopyToRegUses = false;
bool isTruncFree = TLI.isTruncateFree(VT, N0.getValueType());
for (SDNode::use_iterator UI = N0->use_begin(), UE = N0->use_end(); UI != UE;
++UI) {
SDNode *User = *UI;
for (SDUse &Use : N0->uses()) {
SDNode *User = Use.getUser();
if (User == N)
continue;
if (UI.getUse().getResNo() != N0.getResNo())
if (Use.getResNo() != N0.getResNo())
continue;
// FIXME: Only extend SETCC N, N and SETCC N, c for now.
if (ExtOpc != ISD::ANY_EXTEND && User->getOpcode() == ISD::SETCC) {
Expand Down Expand Up @@ -13266,9 +13265,7 @@ static bool ExtendUsesToFormExtLoad(EVT VT, SDNode *N, SDValue N0,

if (HasCopyToRegUses) {
bool BothLiveOut = false;
for (SDNode::use_iterator UI = N->use_begin(), UE = N->use_end();
UI != UE; ++UI) {
SDUse &Use = UI.getUse();
for (SDUse &Use : N->uses()) {
if (Use.getResNo() == 0 && Use.getUser()->getOpcode() == ISD::CopyToReg) {
BothLiveOut = true;
break;
Expand Down Expand Up @@ -13780,11 +13777,10 @@ SDValue DAGCombiner::foldSextSetcc(SDNode *N) {

// Non-chain users of this value must either be the setcc in this
// sequence or extends that can be folded into the new {z/s}ext-load.
for (SDNode::use_iterator UI = V->use_begin(), UE = V->use_end();
UI != UE; ++UI) {
for (SDUse &Use : V->uses()) {
Copy link
Contributor

@s-barannikov s-barannikov Dec 19, 2024

Choose a reason for hiding this comment

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

It would be great if SDUse was hidden from users (=callers) and use_iterator::operator* returned a pair of SDNode* and unsigned instead (possibly wrapped in another struct). It looks like Next and Prev members are implementation detail that shouldn't be exposed. Also, they increase the size of SDUse that makes us use & in range-based for loops.
We could use structured bindings with the returned pair to further simplify the loops.
(We may still be able to use structured bindings if SDUse supports std::get<>).

Copy link
Collaborator Author

@topperc topperc Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Even without next/prev there are 3 pieces of information not 2. The SDNode * being used, the result number being used, and the SDNode * of the user instruction.

op_begin/end also return SDUse and SDUse has an implicit conversion to SDValue which is used in many places.

// Skip uses of the chain and the setcc.
SDNode *User = *UI;
if (UI.getUse().getResNo() != 0 || User == N0.getNode())
SDNode *User = Use.getUser();
if (Use.getResNo() != 0 || User == N0.getNode())
continue;
// Extra users must have exactly the same cast we are about to create.
// TODO: This restriction could be eased if ExtendUsesToFormExtLoad()
Expand Down Expand Up @@ -18928,7 +18924,7 @@ bool DAGCombiner::CombineToPreIndexedLoadStore(SDNode *N) {
for (SDNode::use_iterator UI = BasePtr->use_begin(),
UE = BasePtr->use_end();
UI != UE; ++UI) {
SDUse &Use = UI.getUse();
SDUse &Use = *UI;
// Skip the use that is Ptr and uses of other results from BasePtr's
// node (important for nodes that return multiple results).
if (Use.getUser() == Ptr.getNode() || Use != BasePtr)
Expand Down Expand Up @@ -20056,13 +20052,12 @@ bool DAGCombiner::SliceUpLoad(SDNode *N) {
// Check if this load is used as several smaller chunks of bits.
// Basically, look for uses in trunc or trunc(lshr) and record a new chain
// of computation for each trunc.
for (SDNode::use_iterator UI = LD->use_begin(), UIEnd = LD->use_end();
UI != UIEnd; ++UI) {
for (SDUse &U : LD->uses()) {
// Skip the uses of the chain.
if (UI.getUse().getResNo() != 0)
if (U.getResNo() != 0)
continue;

SDNode *User = *UI;
SDNode *User = U.getUser();
unsigned Shift = 0;

// Check if this is a trunc(lshr).
Expand Down Expand Up @@ -20940,7 +20935,7 @@ DAGCombiner::getStoreMergeCandidates(StoreSDNode *St,
// This must be a chain use.
if (UseIter.getOperandNo() != 0)
return;
if (auto *OtherStore = dyn_cast<StoreSDNode>(*UseIter)) {
if (auto *OtherStore = dyn_cast<StoreSDNode>(UseIter->getUser())) {
BaseIndexOffset Ptr;
int64_t PtrDiff;
if (CandidateMatch(OtherStore, Ptr, PtrDiff) &&
Expand All @@ -20958,12 +20953,13 @@ DAGCombiner::getStoreMergeCandidates(StoreSDNode *St,
return nullptr;
for (auto I = RootNode->use_begin(), E = RootNode->use_end();
I != E && NumNodesExplored < MaxSearchNodes; ++I, ++NumNodesExplored) {
if (I.getOperandNo() == 0 && isa<LoadSDNode>(*I)) { // walk down chain
for (auto I2 = (*I)->use_begin(), E2 = (*I)->use_end(); I2 != E2; ++I2)
SDNode *User = I->getUser();
if (I.getOperandNo() == 0 && isa<LoadSDNode>(User)) { // walk down chain
for (auto I2 = User->use_begin(), E2 = User->use_end(); I2 != E2; ++I2)
TryToAddCandidate(I2);
}
// Check stores that depend on the root (e.g. Store 3 in the chart above).
if (I.getOperandNo() == 0 && isa<StoreSDNode>(*I)) {
if (I.getOperandNo() == 0 && isa<StoreSDNode>(User)) {
TryToAddCandidate(I);
}
}
Expand Down Expand Up @@ -27320,8 +27316,7 @@ SDValue DAGCombiner::visitGET_FPENV_MEM(SDNode *N) {

// Check if the loaded value is used only in a store operation.
StoreSDNode *StNode = nullptr;
for (auto I = LdNode->use_begin(), E = LdNode->use_end(); I != E; ++I) {
SDUse &U = I.getUse();
for (SDUse &U : LdNode->uses()) {
if (U.getResNo() == 0) {
if (auto *St = dyn_cast<StoreSDNode>(U.getUser())) {
if (StNode)
Expand Down
7 changes: 3 additions & 4 deletions llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ void DAGTypeLegalizer::PerformExpensiveChecks() {
if (I != ReplacedValues.end()) {
Mapped |= 1;
// Check that remapped values are only used by nodes marked NewNode.
for (SDNode::use_iterator UI = Node.use_begin(), UE = Node.use_end();
UI != UE; ++UI)
if (UI.getUse().getResNo() == i)
assert(UI->getNodeId() == NewNode &&
for (SDUse &U : Node.uses())
if (U.getResNo() == i)
assert(U.getUser()->getNodeId() == NewNode &&
"Remapped value has non-trivial use!");

// Check that the final result of applying ReplacedValues is not
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ void ScheduleDAGSDNodes::ClusterNeighboringLoads(SDNode *Node) {
// This algorithm requires a reasonably low use count before finding a match
// to avoid uselessly blowing up compile time in large blocks.
unsigned UseCount = 0;
for (SDNode::use_iterator I = Chain->use_begin(), E = Chain->use_end();
for (SDNode::user_iterator I = Chain->user_begin(), E = Chain->user_end();
I != E && UseCount < 100; ++I, ++UseCount) {
if (I.getUse().getResNo() != Chain.getResNo())
continue;
Expand Down
40 changes: 19 additions & 21 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11611,7 +11611,7 @@ class RAUWUpdateListener : public SelectionDAG::DAGUpdateListener {

void NodeDeleted(SDNode *N, SDNode *E) override {
// Increment the iterator as needed.
while (UI != UE && N == *UI)
while (UI != UE && N == UI->getUser())
++UI;
}

Expand Down Expand Up @@ -11650,7 +11650,7 @@ void SelectionDAG::ReplaceAllUsesWith(SDValue FromN, SDValue To) {
SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
RAUWUpdateListener Listener(*this, UI, UE);
while (UI != UE) {
SDNode *User = *UI;
SDNode *User = UI->getUser();

// This node is about to morph, remove its old self from the CSE maps.
RemoveNodeFromCSEMaps(User);
Expand All @@ -11660,12 +11660,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDValue FromN, SDValue To) {
// To help reduce the number of CSE recomputations, process all
// the uses of this user that we can find this way.
do {
SDUse &Use = UI.getUse();
SDUse &Use = *UI;
++UI;
Use.set(To);
if (To->isDivergent() != From->isDivergent())
updateDivergence(User);
} while (UI != UE && *UI == User);
} while (UI != UE && UI->getUser() == User);
// Now that we have modified User, add it back to the CSE maps. If it
// already exists there, recursively merge the results together.
AddModifiedNodeToCSEMaps(User);
Expand Down Expand Up @@ -11708,7 +11708,7 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, SDNode *To) {
SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
RAUWUpdateListener Listener(*this, UI, UE);
while (UI != UE) {
SDNode *User = *UI;
SDNode *User = UI->getUser();

// This node is about to morph, remove its old self from the CSE maps.
RemoveNodeFromCSEMaps(User);
Expand All @@ -11718,12 +11718,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, SDNode *To) {
// To help reduce the number of CSE recomputations, process all
// the uses of this user that we can find this way.
do {
SDUse &Use = UI.getUse();
SDUse &Use = *UI;
++UI;
Use.setNode(To);
if (To->isDivergent() != From->isDivergent())
updateDivergence(User);
} while (UI != UE && *UI == User);
} while (UI != UE && UI->getUser() == User);

// Now that we have modified User, add it back to the CSE maps. If it
// already exists there, recursively merge the results together.
Expand Down Expand Up @@ -11756,7 +11756,7 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, const SDValue *To) {
SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
RAUWUpdateListener Listener(*this, UI, UE);
while (UI != UE) {
SDNode *User = *UI;
SDNode *User = UI->getUser();

// This node is about to morph, remove its old self from the CSE maps.
RemoveNodeFromCSEMaps(User);
Expand All @@ -11767,12 +11767,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, const SDValue *To) {
// user that we can find this way.
bool To_IsDivergent = false;
do {
SDUse &Use = UI.getUse();
SDUse &Use = *UI;
const SDValue &ToOp = To[Use.getResNo()];
++UI;
Use.set(ToOp);
To_IsDivergent |= ToOp->isDivergent();
} while (UI != UE && *UI == User);
} while (UI != UE && UI->getUser() == User);

if (To_IsDivergent != From->isDivergent())
updateDivergence(User);
Expand Down Expand Up @@ -11810,15 +11810,15 @@ void SelectionDAG::ReplaceAllUsesOfValueWith(SDValue From, SDValue To){
UE = From.getNode()->use_end();
RAUWUpdateListener Listener(*this, UI, UE);
while (UI != UE) {
SDNode *User = *UI;
SDNode *User = UI->getUser();
bool UserRemovedFromCSEMaps = false;

// A user can appear in a use list multiple times, and when this
// happens the uses are usually next to each other in the list.
// To help reduce the number of CSE recomputations, process all
// the uses of this user that we can find this way.
do {
SDUse &Use = UI.getUse();
SDUse &Use = *UI;

// Skip uses of different values from the same node.
if (Use.getResNo() != From.getResNo()) {
Expand All @@ -11837,7 +11837,7 @@ void SelectionDAG::ReplaceAllUsesOfValueWith(SDValue From, SDValue To){
Use.set(To);
if (To->isDivergent() != From->isDivergent())
updateDivergence(User);
} while (UI != UE && *UI == User);
} while (UI != UE && UI->getUser() == User);
// We are iterating over all uses of the From node, so if a use
// doesn't use the specific value, no changes are made.
if (!UserRemovedFromCSEMaps)
Expand Down Expand Up @@ -11982,11 +11982,9 @@ void SelectionDAG::ReplaceAllUsesOfValuesWith(const SDValue *From,
for (unsigned i = 0; i != Num; ++i) {
unsigned FromResNo = From[i].getResNo();
SDNode *FromNode = From[i].getNode();
for (SDNode::use_iterator UI = FromNode->use_begin(),
E = FromNode->use_end(); UI != E; ++UI) {
SDUse &Use = UI.getUse();
for (SDUse &Use : FromNode->uses()) {
if (Use.getResNo() == FromResNo) {
UseMemo Memo = { *UI, i, &Use };
UseMemo Memo = {Use.getUser(), i, &Use};
Uses.push_back(Memo);
}
}
Expand Down Expand Up @@ -12462,8 +12460,8 @@ bool SDNode::hasNUsesOfValue(unsigned NUses, unsigned Value) const {
assert(Value < getNumValues() && "Bad value!");

// TODO: Only iterate over uses of a given value of the node
for (SDNode::use_iterator UI = use_begin(), E = use_end(); UI != E; ++UI) {
if (UI.getUse().getResNo() == Value) {
for (SDUse &U : uses()) {
if (U.getResNo() == Value) {
if (NUses == 0)
return false;
--NUses;
Expand All @@ -12479,8 +12477,8 @@ bool SDNode::hasNUsesOfValue(unsigned NUses, unsigned Value) const {
bool SDNode::hasAnyUseOfValue(unsigned Value) const {
assert(Value < getNumValues() && "Bad value!");

for (SDNode::use_iterator UI = use_begin(), E = use_end(); UI != E; ++UI)
if (UI.getUse().getResNo() == Value)
for (SDUse &U : uses())
if (U.getResNo() == Value)
return true;

return false;
Expand Down
Loading
Loading