[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