[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 inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2626
+  // So we do nothing for non-vector types for now.
+  if (!TLI.isTypeLegal(VT) || !VT.isVector())
+    return SDValue();
----------------
Your reference to a broken lea-3.ll test is down right suspicious, do you have any more information?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2658
+    if (!SignDirection)
+      SignDirection = ConstantValue.isNonNegative() ? 1 : -1;
+    // Avoid getting poisoned through shifts > bitsize.
----------------
What happens when different lanes require different SignDirection values?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2694
+  SDValue Shl = DAG.getNode(ISD::SHL, DL, VT, N0, LogBase2);
+  Shl.getNode()->setFlags(N->getFlags());
+  AddToWorklist(Shl.getNode());
----------------
Won't this work?
```
SDValue Shl = DAG.getNode(ISD::SHL, DL, VT, N0, LogBase2, N->getFlags()); 
```




================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2714
+  return Res;
+}
+
----------------
Can you clang-format the whole of TransformMulWithPow2DisplacedBy1 -  its seems a bit off.


https://reviews.llvm.org/D37896