[PATCH] [X86] Integer multiplication improvements

[Chandler Carruth]

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23991-23992
@@ +23990,4 @@
+  
+  // The sequences we want to produce are larger than in imul, 
+  // disable this for -Oz
+  if (DAG.getMachineFunction().getFunction()->getAttributes().hasAttribute(
----------------
s/in/an

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24019-24023
@@ +24018,7 @@
+
+  // How many steps will we have to peform to replace the
+  // MUL. We limit this to 3 steps, based on imul latency of 4
+  // (If the latency is equal, this is probably not a win)
+  unsigned Steps = 0;
+  const unsigned MAX_STEPS = 3;
+  // Having to negate or shift is a step
----------------
This doesn't really make sense. We shouldn't be hard coding these things here.

- LEA as slower on Atom than on other X86 chips.
- IMUL can be anything from 6 to 14 cycles on some X86 chips.

We have a schedule, we should consult it for the expected latency of these and use those to drive the limits. (And we should fix the schedule to be correct if it is currently wrong.)

Also, I wouldn't use capitals here. This isn't a macro.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24025-24026
@@ +24024,4 @@
+  // Having to negate or shift is a step
+  Steps += (IsNeg ?  1 : 0);
+  Steps += (ShiftAmt ?  1 : 0);
+  
----------------
Just use the boolean?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24060-24061
@@ -24032,1 +24059,4 @@
+  // Do not add new nodes to DAG combiner worklist.
+  if (MulAmt == 1)
     DCI.CombineTo(N, NewMul, false);
+
----------------
It's pretty hostile to the DAG to create nodes unless they are absolutely going to be used. It's almost certainly better to do the math in a tight loop first to verify that we'll actually combine this node.

That will also let you use early exit.

http://reviews.llvm.org/D7320

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/