[PATCH] D83948: [PowerPC] Custom lowering for funnel shifts

[Jay Foad via Phabricator via llvm-commits]

foad marked 3 inline comments as done.
foad added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8998
+  // fshr: (X << (BW - (Z % BW))) | (Y >> (Z % BW))
+  // This is simpler than TargetLowering::expandFunnelShift because we can rely
+  // on PowerPC shift by BW being well defined.
----------------
RKSimon wrote:
> Maybe also show the pseudocode as used by PPC for comparison?
This //is// pseudocode for the PPC expansion. Did you mean, show the more complicated expansion that TargetLowering would have used?


================
Comment at: llvm/test/CodeGen/PowerPC/umulfixsat.ll:12
 ; CHECK-NEXT:    mullw 3, 3, 4
+; CHECK-NEXT:    ori 3, 3, 0
 ; CHECK-NEXT:    bclr 12, 2, 0
----------------
I think the extra instruction here is just due to bad luck in the register allocation or scheduling.


================
Comment at: llvm/test/CodeGen/PowerPC/umulfixsat.ll:29
+; CHECK-NEXT:    slwi 6, 3, 31
+; CHECK-NEXT:    or 4, 6, 4
+; CHECK-NEXT:    cmplwi 3, 1
----------------
It's hard to see because of the register allocation and scheduling differences but there is a slight regression here, from:
```
       rotlwi  3, 3, 31
       rlwimi 3, 5, 31, 0, 0
```
to:
```
       srwi 5, 5, 1
       slwi 4, 3, 31
       or 4, 4, 5
```
I suppose I have regressed funnel shifts by a constant amount. I'll see if I can fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83948