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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7119
   SDValue N1 = N->getOperand(1);
   SDValue N2 = N->getOperand(2);
   bool IsFSHL = N->getOpcode() == ISD::FSHL;
----------------
lebedev.ri wrote:
> What should be done if N2 is `undef`?
> Pretend that it is `0`, or replace the entire op with `undef`?
Pretending it is zero would be consistent with InstSimplify: https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/InstructionSimplify.cpp#L5129

Replacing with undef is not legal (consider for example N0=0, N1=0, which has zero as the only possible result, regardless of N2).


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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58009