[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
llvm-commits at lists.llvm.org
Wed Sep 27 09:23:58 PDT 2017
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
More information about the llvm-commits
mailing list