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

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

In D89855#2399063 <https://reviews.llvm.org/D89855#2399063>, @Esme wrote:

> In D89855#2398960 <https://reviews.llvm.org/D89855#2398960>, @shchenz wrote:
>
>> I saw some testcases are removed compared with your committed version. Is there any reason?
>
> Every specific folding patterns with different masks have been tested in llvm/test/CodeGen/PowerPC/fold-rlwinm.mir
> This patch didn't change the patterns, so I supposed it would be clearer to remove some redundant test cases.
> Do you think they should be added back？

Understanding your concerns here. I prefer to keep that testcases as well. If new added post ra peephole in pre-emit-peephole pass changes the case, the pre-ra cases will not be aware of? This is up to you.^^



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3257
+      SrcMI->definesRegister(SrcMI->getOperand(1).getReg()))
+    return false;
   // If MBMI is bigger than MEMI, we always can not get run of ones.
----------------
Esme wrote:
> shchenz wrote:
> > Can these lines be moved after line 3221? So we will bail out early.
> It's safe to call `SrcMI->getOperand(1).getReg()` after filtering the opcode of SrcMI.
Is it safe if we add `SrcMI->getOperand(1).isReg()`?. We can bail out early if the operand 1 of SrcMI is not a register.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3360
+    if (MRI->isSSA())
+      CanErase = !SrcMI->hasImplicitDef() && MRI->use_nodbg_empty(FoldingReg);
+    if (CanErase) {
----------------
Esme wrote:
> shchenz wrote:
> > Can we move `CanErase` assignment for SSA to the place where we assign it for post SSA?
> The condition `MRI->use_nodbg_empty(FoldingReg)` has to be checked after the folding of MI and SrcMI.
Is it ok if we change `use_nodbg_empty` to `hasOneNonDBGUse()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89855