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 all commits
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: 58 additions & 23 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 @@ -806,9 +806,6 @@ END_TWO_BYTE_PACK()
return !operator==(x);
}

/// Return true if this iterator is at the end of uses list.
bool atEnd() const { return Op == nullptr; }

// Iterator traversal: forward iteration only.
use_iterator &operator++() { // Preincrement
assert(Op && "Cannot increment end iterator!");
Expand All @@ -821,14 +818,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 +832,69 @@ 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); }

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

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

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 +1054,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

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
Loading
Loading