[PATCH] D42479: DAGCombiner: Combine SDIV with non-splat vector pow2 divisor

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2905
+  SmallBitVector KnownNegatives(
+      (N1C || !VT.isVector()) ? 1 : VT.getVectorNumElements(), false);
+  unsigned EltIndex = 0;
----------------
RKSimon wrote:
> Why is it important for splat vectors to use a single entry in KnownNegatives? And won't it cause issues when matchBinaryPredicate enumerates across the entire vector?
Still worried about this - matchUnaryPredicate doesn't check for a splat vector (I can't remember why not) - it will check every element. with IsPowerOfTwo so EltIndex will go to number of vector elements.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2925
+  // For non-splat vector shift amounts, it is profitable if the target can
+  // lower efficiently vector shifts with vector variable shift amounts.
+  if (!N->getFlags().hasExact() && matchUnaryPredicate(N1, IsPowerOfTwo)) {
----------------
This comment needs updating.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2950
+    SDValue C1 = DAG.getNode(ISD::CTTZ, DL, VT, N1);
+    C1 = DAG.getZExtOrTrunc(C1, DL, ShiftAmtTy);
+    SDValue INEXACT = DAG.getNode(ISD::SUB, DL, ShiftAmtTy, BITS, C1);
----------------
Why can't LG2 and C1 be merged?


Repository:
  rL LLVM

https://reviews.llvm.org/D42479