[PATCH] D77850: [PowerPC] Exploit rldimi for ori with large immediates

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM. Please add a test case where the input value has multiple uses for a test of where we don't do this even when the constant is conducive to doing it. I don't think such an addition requires another review.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4556
+    return false;
+
+  unsigned SH = 63 - ME;
----------------
qiucf wrote:
> nemanjai wrote:
> > There are probably some more conditions under which you don't want to do this. What comes to mind is checking for a single use of the input to the OR (since `rldimi` is destructive) and that the output is not used in a SETCC to compare against zero.
> > 
> > Here's a test case:
> > ```
> > long test(long a, long b) {
> >   return ((a | 4294967296L) > 0) ? a : b;
> > }
> > ```
> > Prior to this patch, the codegen is:
> > ```
> >         li 5, 1
> >         sldi 5, 5, 32
> >         or. 5, 3, 5
> >         isel 3, 3, 4, 1
> > ```
> > With this patch, the codegen is:
> > ```
> >         li 5, -1
> >         mr 6, 3
> >         rldimi. 6, 5, 32, 31
> >         isel 3, 3, 4, 1
> > ```
> > 
> > At first glance, the new codegen looks no worse. However it is worse for two reasons:
> > 1. Increased register pressure to move the value due to `rldimi` being destructive
> > 2. Record-form rotate instead of record-form `or` is worse because the former is a two-way cracked instruction.
> Thanks for pointing this out. I'm not sure about: after checking the input has only one use, is it still profitable to use `or.` instead of `rldimi.` for comparison against zero? Like:
> 
> ```
> long test(long a, long b, long c) {
>   return ((a | 4294967296L) > 0) ? c : b;
> }
> 
> # Original result
> li 6, 1
> sldi 6, 6, 32
> or. 3, 3, 6
> isel 3, 5, 4, 1
> blr
> 
> # After patch
> li 6, -1
> rldimi. 3, 6, 32, 31
> isel 3, 5, 4, 1
> blr
> ```
Well, we could check something like `N->use_begin()->getOpcode() == ISD::SETCC`, but I don't think it is necessary. The cracked `rldimi.` is no worse than the increased path length of the `or.` sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77850