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

EsmeYi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 17 01:44:13 PST 2020


Esme marked an inline comment as not done.
Esme added a comment.

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?



================
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.
----------------
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.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3360
+    if (MRI->isSSA())
+      CanErase = !SrcMI->hasImplicitDef() && MRI->use_nodbg_empty(FoldingReg);
+    if (CanErase) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89855



More information about the llvm-commits mailing list