[PATCH] D89855: [PowerPC] Extend folding RLWINM + RLWINM to post-RA.

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3218
+  } else {
+    if (!Register::isPhysicalRegister(FoldingReg))
+      return false;
----------------
This is redundant or can be changed to an assert? 


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3229
+  case PPC::RLWINM_rec:
+    if (Is64Bit)
+      return false;
----------------
Esme wrote:
> shchenz wrote:
> > What case will have rlwinm input is rlwinm8 or rlwinm8 input is rlwinm? Just curious. 
> There is a case in llvm/test/CodeGen/PowerPC/popcnt-zext.ll
> ```
> declare i16 @llvm.ctpop.i16(i16) nounwind readnone
> define i64 @popa_i16_i64(i16 %x) {
>   %pop = call i16 @llvm.ctpop.i16(i16 %x)
>   %z = zext i16 %pop to i64 ; SimplifyDemandedBits may turn zext (or sext) into aext
>   %a = and i64 %z, 16
>   ret i64 %a
> }
> ```
> `llc -verify-machineinstrs -mtriple=powerpc64-- -mattr=+slow-popcntd < cnt16.ll -debug`
> We will see such transformation:
> From
> ```
> 368B	  %22:gprc = RLWINM %21:gprc, 8, 24, 31
> 384B	  undef %23.sub_32:g8rc = COPY %22:gprc
> 400B	  %25:g8rc = RLWINM8 %23:g8rc, 0, 27, 27
> 416B	  $x3 = COPY %25:g8rc
> ```
> To
> 
> ```
>   undef %23.sub_32:g8rc = RLWINM %21:gprc, 8, 24, 31
>   %25:g8rc = RLWINM8 %23:g8rc, 0, 27, 27
>   $x3 = COPY %25:g8rc
> ```
> After RA
> 
> ```
>   renamable $r3 = RLWINM killed renamable $r3, 8, 24, 31, implicit-def $x3
>   renamable $x3 = RLWINM8 killed renamable $x3, 0, 27, 27
> ```
> We will treat them as DefMI and MI after RA, but they shouldn't be folded. Please correct me if I'm wrong.
Thanks, I see, this is just a post-ra issue. Before RA, there are always register copy instructions between `RLWINM` and `RLWINM8`.

After Post-RA, 
1: for `RLWINM8(RLWINM)`, it should be ok to fold it to single `RLWINM8`,
2: for `RLWINM(RLWINM8)`, it should be ok to fold it to a single `RLWINM`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89855