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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2932
+    return true;
+  };
   // fold (sdiv X, pow2) -> simple ops after legalize
----------------
zvi wrote:
> RKSimon wrote:
> > Did you look at using matchBinaryPredicate here?
> Thanks for suggesting. Will use it.
Oops, sorry that should have been matchUnaryPredicate...


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2905
+  SmallBitVector KnownNegatives(
+      (N1C || !VT.isVector()) ? 1 : VT.getVectorNumElements(), false);
+  unsigned EltIndex = 0;
----------------
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?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2955
+      INEXACT = DAG.FoldConstantArithmetic(ISD::SUB, DL, ShiftAmtTy,
+                                           BITS.getNode(), LG2.getNode());
 
----------------
Would it be easier to just do something like this which should work for scalars and vectors?
```
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);
if (!isConstantOrConstantVector(INEXACT))
  return SDValue()
```


Repository:
  rL LLVM

https://reviews.llvm.org/D42479