[PATCH] D69168: [PowerPC] Fold redundant load immediates of zero and delete if possible

Kamau Bridgeman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 13 11:51:06 PST 2019


kamaub added a comment.

All comments have been address, thank you.



================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:329
+        Simplified |= Folded;
+        if (MRI->use_nodbg_empty(MIDestReg) && Folded) {
+          ++NumLoadImmZeroFoldedAndRemoved;
----------------
nemanjai wrote:
> stefanp wrote:
> > Why do you need to check `&& Folded` here? The output register of this `LI` instruction has no non-debug uses so you can probably just schedule it to be deleted anyway. If you do that you also don't need to keep `Folded` around and just do what Nemanja mentioned:
> > ```
> > Simplified |=
> >   TII->FoldImmediateWithoutDelete(UseMI, MI, MIDestReg, MRI);
> > ```
> In fact, as implemented, `Folded` will be set to whatever `TII->onlyFoldImmediate()` returns for the **last** use which may be `false` even if you have folded it in some previous uses. This needs to be fixed.
That right, I shouldn't used `Folded |=` instead, I've gone with your (earlier suggestion however, thank you.


================
Comment at: llvm/test/CodeGen/PowerPC/save-crbp-ppc32svr4.ll:17
-; CHECK: stw 28, -24(29)
-; CHECK: stw 12, -28(29)
 
----------------
stefanp wrote:
> Question: 
> Why do we lose a store here? Is it because we remove an LI instruction and then that reduces register pressure? 
Yes, an Li is removed which does reduce register pressure but that isn't why this test cases changes. `r28` is spilled because zero is loaded into it after the prologue but, with this patch, it is folded into and `addi` and removed so the stack is reduced from 28 -> 24


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69168





More information about the llvm-commits mailing list