[PATCH] D87046: [PPC] Do not emit extswsli in 32BIT mode when using -mcpu=pwr9

Wed Sep 23 20:11:10 PDT 2020

ZarkoCA marked 5 inline comments as done.
ZarkoCA added inline comments.

================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-shift.ll:15
+
+; CHECK-LABEL:     .a
+
----------------
sfertile wrote:
> Is the `.` character treated as a regex? Neither 32-bit or 64-bit assembly will produce `.a` in the assembly, both produce `a:`, but the test works, so I am clearly missing something.
I changed it to foo and removed the period for clarity. It's strange that it wasn't caught. 

================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-shift.ll:17
+
+; 32BIT:           srawi [[REG1:[0-9]+]], [[REG2:[0-9]+]]
+; 32BIT-NEXT:      rotlwi [[REG3:[0-9]+]], [[REG2]], [[REG4:[0-9]+]]
----------------
Xiangling_L wrote:
> It seems, you missed matching the third part of srawi instruction. Any reason of doing so?
I wanted to only check the register usage but leaving out the SH value makes it more confusing so I added it.  

================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-shift.ll:18
+; 32BIT:           srawi [[REG1:[0-9]+]], [[REG2:[0-9]+]]
+; 32BIT-NEXT:      rotlwi [[REG3:[0-9]+]], [[REG2]], [[REG4:[0-9]+]]
+; 32BIT-NEXT:      slwi [[REG2]], [[REG2]], [[REG4]]
----------------
sfertile wrote:
> Xiangling_L wrote:
> > I am suggesting to use eg. `[[REG3:[0-9]+]]` to match register, use `{{[0-9]+}}` to match numbers. Or since `8` is hardcoded in the IR, we don't need to worry about this value will change in the assembly .
> > 
> > For example, 
> > 
> > ```
> > rotlwi [[REG3:[0-9]+]], [[REG2]], 8
> > 
> > ```
> +1, and same for the ME/MB fields of the rlwimi instruction.
Thanks for catching that.  I left the actual values for the shifts and the mask end/begin, and used variables for the registers in the 32BIT case.  Is that better? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87046