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

Amy Kwan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 23 17:43:46 PST 2022


amyk added a comment.

A few more submitted comments that I realized I had left on the review.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3638
 
+  // Conservatively add defs from DefMI and uses from MI to the set of
+  // registers that need their kill flags updated.
----------------
We should clarify that for `MI`, we're actually adding both the defs and the uses. 


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3642
+    if (MO.isReg() && MO.isDef())
+      UpdatedRegs.push_back(MO.getReg());
+  for (const MachineOperand &MO : MI.operands())
----------------
It would be good to have a more descriptive name for `UpdateRegs`. Perhaps `RegsToUpdate`. 


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:439
+          SmallVector<Register, 2> UpdatedRegs;
+          if (TII->convertToImmediateForm(MI, UpdatedRegs)) {
+            for (Register R : UpdatedRegs)
----------------
Since you're adding a significant amount of code to a deeply nested block, consider flipping this condition to an early exit to reduce the nesting. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133103



More information about the llvm-commits mailing list