[SelectionDAG] Split SDNode::use_iterator into user_iterator and use_iterator. #120531
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…_iterator. sd_use_iterator now returns an SDUse& when dereferenced. sd_user_iterator returns SDNode*.a SDNode::use_begin/use_end/uses work on sd_use_iterator. SDNode::user_begin/user_end/users work on sd_user_iterator. This allows us to write range based for loops using SDUse& and SDNode::uses(). I've converted many of these in this patch. I didn't update loops that have additional variables updated in their for statement. Some loops use sd_use_iterator::getOperandNo() which also prevents using range based for loops. I plan to move this into SDUse in a follow up patch.
llvmbot
added
backend:ARM
backend:AArch64
backend:AMDGPU
backend:Hexagon
backend:RISC-V
backend:PowerPC
backend:SystemZ
backend:X86
llvm:SelectionDAG
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-backend-aarch64 Author: Craig Topper (topperc) Changessd_use_iterator now returns an SDUse& when dereferenced. sd_user_iterator returns SDNode*. SDNode::use_begin/use_end/uses work on sd_use_iterator. SDNode::user_begin/user_end/users work on sd_user_iterator. We can now write range based for loops using SDUse& and SDNode::uses(). I've converted many of these in this patch. I didn't update loops that have additional variables updated in their for statement. Some loops use sd_use_iterator::getOperandNo() which also prevents using range based for loops. I plan to move this into SDUse in a follow up patch. Patch is 49.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120531.diff 18 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 77c04369f3e92d..ff6d4f5f18ed1e 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -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.
@@ -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 {
@@ -837,6 +835,46 @@ 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(); }
+
+ 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; }
+ };
+
/// Provide iteration support to walk over all uses of an SDNode.
use_iterator use_begin() const {
return use_iterator(UseList);
@@ -844,22 +882,25 @@ END_TWO_BYTE_PACK()
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;
@@ -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;
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 85009439c37b3a..7a458ff830ab46 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -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) {
@@ -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;
@@ -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()) {
// 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()
@@ -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)
@@ -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).
@@ -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) &&
@@ -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);
}
}
@@ -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)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
index c7d29ec1a836c1..b6abad830c371e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
@@ -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
diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
index 2e59dbf2f70280..26fc75c0578ec2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -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;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index bd9e5d4dce8ec6..07749ec87d0b20 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -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;
}
@@ -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);
@@ -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);
@@ -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);
@@ -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.
@@ -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);
@@ -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);
@@ -11810,7 +11810,7 @@ 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
@@ -11818,7 +11818,7 @@ void SelectionDAG::ReplaceAllUsesOfValueWith(SDValue From, 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;
// Skip uses of different values from the same node.
if (Use.getResNo() != From.getResNo()) {
@@ -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)
@@ -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);
}
}
@@ -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;
@@ -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;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index c4d0552ad55d39..c1dabe05452fb9 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -3792,8 +3792,8 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
for (unsigned i = 1, e = NodeStack.size()-1; i != e; ++i) {
unsigned NNonChainUses = 0;
SDNode *NS = NodeStack[i].getNode();
- for (auto UI = NS->use_begin(), UE = NS->use_end(); UI != UE; ++UI)
- if (UI.getUse().getValueType() != MVT::Other)
+ for (const SDUse &U : NS->uses())
+ if (U.getValueType() != MVT::Other)
if (++NNonChainUses > 1) {
HasMultipleUses = true;
break;
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 494506def33a35..257cb716bae430 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -23390,11 +23390,10 @@ static SDValue performPostLD1Combine(SDNode *N,
// Check if there are other uses. If so, do not combine as it will introduce
// an extra load.
- for (SDNode::use_iterator UI = LD->use_begin(), UE = LD->use_end(); UI != UE;
- ++UI) {
- if (UI.getUse().getResNo() == 1) // Ignore uses of the chain result.
+ for (SDUse &U : LD->uses()) {
+ if (U.getResNo() == 1) // Ignore uses of the chain result.
continue;
- if (*UI != N)
+ if (U.getUser() != N)
return SDValue();
}
@@ -23410,11 +23409,9 @@ static SDValue performPostLD1Combine(SDNode *N,
SDValue Addr = LD->getOperand(1);
SDValue Vector = N->getOperand(0);
// Search for a use of the address operand that is an increment.
- for (SDNode::use_iterator UI = Addr.getNode()->use_begin(), UE =
- Addr.getNode()->use_end(); UI != UE; ++UI) {
- SDNode *User = *UI;
- if (User->getOpcode() != ISD::ADD
- || UI.getUse().getResNo() != Addr.getResNo())
+ for (SDUse &Use : Addr->uses()) {
+ SDNode *User = Use.getUser();
+ if (User->getOpcode() != ISD::ADD || Use.getResNo() != Addr.getResNo())
continue;
// If the increment is a constant, it must match the memory ref size.
@@ -24128,11 +24125,9 @@ static SDValue performNEONPostLDSTCombine(SDNode *N,
SDValue Addr = N->getOperand(AddrOpIdx);
// Search for a use of the address operand that is an increment.
- for (SDNode::use_iterator UI = Addr.getNode()->use_begin(),
- UE = Addr.getNode()->use_end(); UI != UE; ++UI) {
- SDNode *User ...
[truncated]
|
arsenm
approved these changes
Dec 19, 2024
jayfoad
reviewed
Dec 19, 2024
| 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(); } |
There was a problem hiding this comment.
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().
There was a problem hiding this comment.
Is it used somewhere?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| SDNode *operator->() const { return operator*(); } | ||
|
|
||
| SDUse &getUse() const { return *UI; } |
There was a problem hiding this comment.
Do we need this? If someone wants the Use, shouldn't they use use_iterator in the first place?
There was a problem hiding this comment.
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.
The name |
| @@ -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()) { | |||
There was a problem hiding this comment.
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<>).
There was a problem hiding this comment.
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.
| 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(); } |
There was a problem hiding this comment.
Is it used somewhere?
Co-authored-by: Sergey Barannikov <[email protected]>
It should have been SDNode::use_iterator. I'll update. |
topperc
changed the title
topperc
changed the title
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/3721 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/6011 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SDNode::use_iterator now returns an SDUse& when dereferenced. SDNode::user_iterator returns SDNode*. SDNode::use_begin/use_end/uses work on use_iterator. SDNode::user_begin/user_end/users work on user_iterator.
We can now write range based for loops using SDUse& and SDNode::uses(). I've converted many of these in this patch. I didn't update loops that have additional variables updated in their for statement.
Some loops use SDNode::use_iterator::getOperandNo() which also prevents using range based for loops. I plan to move this into SDUse in a follow up patch.