[PATCH] D58009: [DAGCombine] Simplify funnel shifts with undef args to bitshifts

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7158-7159
+
+  // fold fshr(undef, N1, N2) -> lshr(N1, N2)
+  // fold fshl(N0, undef, N2) -> shl(N0, N2)
+  // TODO: when is it worth doing SUB(BW, N2) as well?
----------------
RKSimon wrote:
> lebedev.ri wrote:
> > Do `ISD::SRL` / `ISD::SHL` implicitly take the modulo of the shift amount?
> > 
> Ah good point! Will limit this to PowerOf2 cases that have passed the maskediszero above
By modulo meant that funnel shift implicitly `urem`'s the shift amount by the bitwidth.
So
```
%r = fshl %a, 0, %c
  =>
%r = shl %a, %c
```
is a miscompile.

I.e. this should be folded to
```
%r = fshl i32 %a, 0, %c
  =>
%cmodwidth = and i32 %c, 31 ; <- !!!
%r = shl i32 %a, %cmodwidth
```



Repository:
  rL LLVM

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

https://reviews.llvm.org/D58009