[PATCH] D133103: [PowerPC] Improve kill flag computation and add verification after MI peephole

[Lei Huang via Phabricator via llvm-commits]

lei added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:4852
 
   // Get killed info in case fixup needed after transformation.
   MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
----------------
this should be removed as well?


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:133
                       MachineInstr *MI);
+  void addDummyDef(MachineBasicBlock &MBB, MachineInstr *At, Register Reg) {
+    BuildMI(MBB, At, At->getDebugLoc(), TII->get(PPC::IMPLICIT_DEF), Reg);
----------------
nit: I knnow you have doc above, but it would be good if we can add a note here about valid usage of this new function.  When it should be used etc..


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:142
+    AU.addRequired<LiveVariables>();
+    AU.addPreserved<LiveVariables>();
     AU.addRequired<MachinePostDominatorTree>();
----------------
nit: seems prev code list all required first and then preserved?  


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1109
     // Reset TrapOpt to false at the end of the basic block.
-    if (EnableTrapOptimization)
-      TrapOpt = false;
----------------
Why was this removed?


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1234
 
+  for (Register Reg : RegsToUpdate) {
+    if (!MRI->reg_empty(Reg) && Register::isVirtualRegister(Reg))
----------------
nit: add doc as to why we are doing this?


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1736
       LLVM_DEBUG(CMPI2->dump());
-    }
+    } else
 
----------------
Is this change on purpose to fix a previous bug?  Seems unrelated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133103