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

[ChenZheng via Phabricator via llvm-commits]

shchenz accepted this revision.
shchenz added a comment.

LGTM too. Please remember to rebase this patch after you commit NFC patch D89846 <https://reviews.llvm.org/D89846>



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3229
+  case PPC::RLWINM_rec:
+    if (Is64Bit)
+      return false;
----------------
Esme wrote:
> shchenz wrote:
> > 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`.
> > 
> > 
> Thanks! I will mark this as a follow-up work after this patch submitted.
sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89855