Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Jan 29, 2025

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jan 29, 2025

@llvm/pr-subscribers-backend-powerpc

Author: Craig Topper (topperc)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/124889.diff

1 Files Affected:

  • (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+1-10)
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 4ca328bd9a9ba2..1a325d92cb8145 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -8651,22 +8651,13 @@ bool PPCTargetLowering::canReuseLoadAddress(SDValue Op, EVT MemVT,
 // Given the head of the old chain, ResChain, insert a token factor containing
 // it and NewResChain, and make users of ResChain now be users of that token
 // factor.
-// TODO: Remove and use DAG::makeEquivalentMemoryOrdering() instead.
 void PPCTargetLowering::spliceIntoChain(SDValue ResChain,
                                         SDValue NewResChain,
                                         SelectionDAG &DAG) const {
   if (!ResChain)
     return;
 
-  SDLoc dl(NewResChain);
-
-  SDValue TF = DAG.getNode(ISD::TokenFactor, dl, MVT::Other,
-                           NewResChain, DAG.getUNDEF(MVT::Other));
-  assert(TF.getNode() != NewResChain.getNode() &&
-         "A new TF really is required here");
-
-  DAG.ReplaceAllUsesOfValueWith(ResChain, TF);
-  DAG.UpdateNodeOperands(TF.getNode(), ResChain, NewResChain);
+  DAG.makeEquivalentMemoryOrdering(ResChain, NewResChain);
 }
 
 /// Analyze profitability of direct move


DAG.ReplaceAllUsesOfValueWith(ResChain, TF);
DAG.UpdateNodeOperands(TF.getNode(), ResChain, NewResChain);
DAG.makeEquivalentMemoryOrdering(ResChain, NewResChain);
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow the TODO and remove the whole helper function?

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 tried but the null check on ResChain is needed for at least one of the 5 callers of this function. I'll look further to see how many really need it.

@topperc topperc merged commit 7fff252 into llvm:main Jan 29, 2025
8 checks passed
@topperc topperc deleted the pr/ppc-makeequiv branch January 29, 2025 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants