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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon marked 2 inline comments as done.
RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7148
+    // fold fshr(N0, undef, C) -> lshr(N0, BW-C)
+    if (N0.isUndef())
+      return DAG.getNode(ISD::SRL, SDLoc(N), VT, N1,
----------------
nikic wrote:
> lebedev.ri wrote:
> > Since we can replace `undef` with something, e.g. `0`, we should do the same if it's `0` too.
> This would also be consistent with InstCombine: https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/InstCombine/InstCombineCalls.cpp#L1996
Sure, I can add support for zero as well.


================
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?
----------------
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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58009