[PATCH] D89855: [PowerPC] Extend folding RLWINM + RLWINM to post-RA.

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3192
 
-bool PPCInstrInfo::combineRLWINM(MachineInstr &MI,
-                                 MachineInstr **ToErase) const {
+// This function tries to combine two RLWINMs. We not only perform such
+// optimization in SSA, but also after RA, since some RLWINM is generated after
----------------
The strange character before "We"


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3197
+                                              MachineInstr *&ToErase) const {
+  bool Is64Bit;
+  switch (MI.getOpcode()) {
----------------
Can we initial Is64Bit to true or false and then we only need to handle 32-bit or 64-bit opcodes in following switch?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3222
+    if (OtherIntermediateUse)
+      return false;
+  }
----------------
Any special reason that we don't do the folding after RA when there are multiple uses for SrcMI? Before RA, we do such kind of transformation as it is still benefit. We eliminate the dependancy between SrcMI and MI.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3229
+  case PPC::RLWINM_rec:
+    if (Is64Bit)
+      return false;
----------------
What case will have rlwinm input is rlwinm8 or rlwinm8 input is rlwinm? Just curious. 


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3356
+    if ((!MRI->isSSA() && IsFoldingRegKilled) ||
+        (MRI->isSSA() && MRI->use_nodbg_empty(FoldingReg))) {
+      ToErase = SrcMI;
----------------
Same as above, after RA, even SrcMI has multiple uses, we still should do the transform?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89855