[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 Jan 26 10:42:34 PST 2023


amyk added a comment.

Some additional comments and questions I thought of after our previous review/discussion.



================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1817
   ++NumEXTSWAndSLDICombined;
+
   ToErase = &MI;
----------------
amyk wrote:
> Nit: unrelated whitespace change.
Nit: Not sure if you missed this before, but this change seems unrelated.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:789
               LLVM_DEBUG(dbgs() << "Removing redundant shift: ");
               LLVM_DEBUG(DefMI->dump());
               ToErase = DefMI;
----------------
Do we need to `addRegToUpdate()` here?


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:997
               .addImm(PPC::sub_32);
           ToErase = &MI;
           Simplified = true;
----------------
Do we need to `addRegToUpdate()` here?


================
Comment at: llvm/test/CodeGen/PowerPC/convert-ri-addi-to-ri.mir:2
 # RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -verify-machineinstrs \
-# RUN:   -run-pass ppc-mi-peepholes -ppc-convert-rr-to-ri %s -o - | FileCheck %s
+# RUN:   -run-pass ppc-mi-peepholes,livevars -ppc-convert-rr-to-ri %s -o - | \
+# RUN:   FileCheck %s
----------------
Potentially silly question, but because we're adding `livevars` here explicitly, does this mean we are not running the live variables pass by default when we are adding live variables to PPCMIPeephole?


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