[PATCH] D105872: [PowerPC] prepare more loop load/store instructions

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

@jsji thanks very much for the review and also for the NFC patch https://reviews.llvm.org/D109083. Updated and rebased accordingly.



================
Comment at: llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp:172
     PPCTargetMachine *TM = nullptr;
-    const PPCSubtarget *ST; 
+    const PPCSubtarget *ST;
     DominatorTree *DT;
----------------
jsji wrote:
> Please commit these unrelated formatting changes directly then rebase. Thanks.
Done in NFC commit 259612019980ebc28f7c4f214a5016cb6cefec73


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp:771
+
+  auto getExistingNode = [&](Instruction *I) -> Value * {
+    Value *StrippedBasePtr = I;
----------------
jsji wrote:
> Comments about this lambda please.
I simplify the logic a little bit, so the lambda function is not needed now.


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp:776
+      // time considering.
+      if (BC->hasOneUser())
+        StrippedBasePtr = *BC->users().begin();
----------------
jsji wrote:
> Not sure why we need this check? Do you have examples that may cost us compile time?
Now I use PHI node's operand instead of its user, so we don't have such issue any more.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105872/new/

https://reviews.llvm.org/D105872