[PATCH] D67431: [PowerPC] [NFC] refactor PPCLoopPreIncPrep pass for further ds/dq form usage.

[Jinsong Ji via Phabricator via llvm-commits]

jsji accepted this revision as: jsji.
jsji added a comment.
This revision is now accepted and ready to land.

Looks mostly good to me , except for some nit formatting issues and comments.



================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:121
+
+    /// Collect condition matched( \p isValidCandidate() returns true)
+    /// candidates in Loop \p L.
----------------
extra space before \p.


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:142
+    /// Rewrite load/store instructions in \p BucketChain according to
+    /// praparation.
+    bool rewriteLoadStores(Loop *L, Bucket &BucketChain,
----------------
typo: preparation.


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:284
+// for load/stores are {0, 34772, 34776, 34780}. Though each offset now is a
+// multipler of 4, it cannot be represented by sint16. Need to improve this. 
+bool PPCLoopPreIncPrep::prepareBaseForUpdateFormChain(Bucket &BucketChain) {
----------------
Can we add `TODO:` or `FIXME` in beginning instead of `Need to improve this` at the end.


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:487
 // This function will check to see if that PHI already exists and will return
 //  true if it found an existing PHI with the same start and increment as the
 //  one we wanted to create.
----------------
clang-formatted? why extra space in last two lines?


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:524
     if (CurrentPHINode->getNumIncomingValues() == 2) {
       if ( (CurrentPHINode->getIncomingBlock(0) == LatchBB &&
             CurrentPHINode->getIncomingBlock(1) == PredBB) ||
----------------
Is this clang-formatted? Format looks weird to me here.


================
Comment at: llvm/lib/Target/PowerPC/PPCLoopPreIncPrep.cpp:564
+  }
+  auto isUpdateFormCandidate = [&] (const Instruction *I,
+                                    const Value *PtrValue) {
----------------
Some comments before this lamda please.


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

https://reviews.llvm.org/D67431