[PATCH] Add DAG combine for shl + add of constants.

[Matt Arsenault]

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4146-4157
@@ -4145,2 +4145,14 @@
 
+  // fold (shl (add x, c1), c2) -> (add (shl x, c2), c1 << c2)
+  // Variant of version done on multiply, except mul by a power of 2 is turned
+  // into a shift.
+  APInt Val;
+  if (N1C && N0.getOpcode() == ISD::ADD && N0.getNode()->hasOneUse() &&
+      (isConstantSplatVector(N0.getOperand(1).getNode(), Val) ||
+       isa<ConstantSDNode>(N0.getOperand(1)))) {
+    SDValue Shl0 = DAG.getNode(ISD::SHL, SDLoc(N0), VT, N0.getOperand(0), N1);
+    SDValue Shl1 = DAG.getNode(ISD::SHL, SDLoc(N1), VT, N0.getOperand(1), N1);
+    return DAG.getNode(ISD::ADD, SDLoc(N), VT, Shl0, Shl1);
+  }
+
   if (N1C) {
----------------
andreadb wrote:
> If I understand correctly, your rule doesn't try to fold the case where C2 is a build_vector of all ConstantSDNode or Undef. Is this intentional?
> If so, then why at line 4151 you check for 'isConstantSplatVector'? I am asking this because you specifically check for N1C; however, (unless I misread the code) N1C is not null only if N1 is a ConstantSDNode.
> 
> Also, I noticed that the dag combiner already implements a similar (but less poweful) rule in 'visitADD' at around line 1658 (see also function 'combineShlAddConstant'); the rule is:
> // fold (add (shl (add x, c1), c2), ) -> (add (add (shl x, c2), c1<<c2), )
> 
> I think that your patch would make that rule redundant. I would do an experiment and see if that is true; in case, I suggest to get rid of it (and also the static function 'combineShlAddConstant').
> 
> I hope this helps.
> Andrea
It's sort of hard to follow, but N1C is replaced with the constant splat value under the VT.isVector() at the beginning of the function. I've added a test for vectors in the new version to make sure that works.

The combineShlAddConstant transform actually made the opposite change way back in r33361 / 2007 with the weak justification that it "fixes test/CodeGen/ARM/smul.ll" so I assume there isn't actually a real reason to avoid doing this. I've also removed it in the new version.

http://reviews.llvm.org/D4768