[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
Thu Nov 17 19:20:01 PST 2022


amyk added a comment.

Just had a couple comments I wanted to submit prior to the rebase of this patch.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3632
          "The forwarding operand needs to be valid at this point");
+
   bool IsForwardingOperandKilled = MI.getOperand(ForwardingOperand).isKill();
----------------
Nit: unrelated whitespace change.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3651
           PPC::INSTRUCTION_LIST_END &&
-      transformToNewImmFormFedByAdd(MI, *DefMI, ForwardingOperand))
+      transformToNewImmFormFedByAdd(MI, *DefMI, ForwardingOperand)) {
     return true;
----------------
Isn't this an unrelated change, as well?


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:468
+        if (MO.isReg() && MO.isDef() && RegsToUpdate.count(MO.getReg())) {
+          RegsToUpdate.erase(MO.getReg());
+          LV->recomputeForSingleDefVirtReg(MO.getReg());
----------------
We're accessing the register both in the cases when we're removing it from `RegsToUpdate` and when we're re-running LiveVariables.
It might make sense to pull out `MO.isReg()` into a separate variable.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1817
   ++NumEXTSWAndSLDICombined;
+
   ToErase = &MI;
----------------
Nit: unrelated whitespace change.


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