[PATCH] D37896: [DAGCombine] Resolving PR34474 by transforming mul(x, 2^c +/- 1) -> sub/add(shl(x, c) x) for any type including vector types

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Please can you commit the new tests you've added to vector-mul.ll with trunk's current codegen and then update this patch to show the diff?



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2623
+
+  if (TLI.isTypeLegal(VT)){
+    // FIXME: There is a possible regression in x86.
----------------
This should be an early-out
```
if (!TLI.isTypeLegal(VT))
  return SDValue();
```


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2643
+    unsigned TrailingZeroes = 0;
+    bool match = matchUnaryPredicate(N1, [&](ConstantSDNode *C) {
+      const APInt &ConstantValue = C->getAPIntValue();
----------------
bool Match


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2659
+        SignDirection = ConstantValue.isNonNegative() ? 1 : -1;
+      }
+      // Avoid getting poisoned through shifts > bitsize.
----------------
(style) Remove braces around single lines


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2670
+    });
+
+    if (match && static_cast<unsigned>(abs(AllArePow2)) == (VT.isVector() ? VT.getVectorNumElements() : 1)) {
----------------
```
if (!Match)
  return SDValue();
```


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2671
+
+    if (match && static_cast<unsigned>(abs(AllArePow2)) == (VT.isVector() ? VT.getVectorNumElements() : 1)) {
+      SDLoc DL(N);
----------------
Isn't VT is guaranteed to be a vector?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2696
+
+      SDValue *LHS = &Shl, *RHS = &N0;
+      if (SignDirection < 0) {
----------------
Is there a need for these to be pointers?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2699
+        std::swap(LHS, RHS);
+      }
+      auto Res = DAG.getNode(AllArePow2 > 0 ? ISD::SUB : ISD::ADD, DL, VT, *LHS, *RHS);
----------------
braces


https://reviews.llvm.org/D37896