[X86] Disable mul -> shl + lea combine when compiling for minsize

[RKSimon]

Slightly off at a tangent: are we ensuring that we keep to the small (8-bit immediate) imul implementation when possible? I seem to recall there's been a problem with unnecessary 32-bit immediate imul instructions causing code bloat.

[spatel]

lib/Target/X86/X86ISelLowering.cpp

23521 ↗

(On Diff #31667)

Missing period.

test/CodeGen/X86/imul.ll

112–119 ↗

(On Diff #31667)

Does this test change with the patch? Is there a reason i386 should be different than x86-64?

[mkuper]

Thanks, Simon, Sanjay.

Simon:
It should be all right now. E.g.:

echo imull $12, %eax, %eax | llvm-mc --show-encoding

.text
imull   $12, %eax, %eax         # encoding: [0x6b,0xc0,0x0c]

The issues you were seeing were most likely fixed in r241152.

test/CodeGen/X86/imul.ll

112–119 ↗

(On Diff #31667)

This is just a copy of a test from above, but with minsize added. I wanted to verify that we still produce a single shift/lea (as opposed to a sequence with > 1 instructions) even under minsize.
Do you think this is redundant?

[spatel]

test/CodeGen/X86/imul.ll

112–119 ↗

(On Diff #31667)

Ah, I didn't notice the earlier test. I got tired of running into the FIXMEs that I added with r243994, so I've been adding minsize tests myself. In some cases, it has exposed functionality with no regression tests, so I like having the test.

Not a problem then, although I'm still curious why this is different for 32-bit. The answer may be related to that "But why?!" comment above. :)

[spatel]

LGTM.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

static SDValue PerformCMOVCombine(SDNode *N, SelectionDAG &DAG,
   return SDValue();
 }
 
 /// PerformMulCombine - Optimize a single multiply with constant into two
 /// in order to implement it with two cheaper instructions, e.g.
 /// LEA + SHL, LEA + LEA.
 static SDValue PerformMulCombine(SDNode *N, SelectionDAG &DAG,
                                  TargetLowering::DAGCombinerInfo &DCI) {
+  // An imul is usually smaller than the alternative sequence.
+  if (DAG.getMachineFunction().getFunction()->optForMinSize())
+    return SDValue();
+
   if (DCI.isBeforeLegalize() || DCI.isCalledByLegalizer())
     return SDValue();
 
   EVT VT = N->getValueType(0);
   if (VT != MVT::i64 && VT != MVT::i32)
     return SDValue();
 
   ConstantSDNode *C = dyn_cast<ConstantSDNode>(N->getOperand(1));

llvm/trunk/test/CodeGen/X86/imul.ll

 ; X86-LABEL: mul40_64:
 ; X86: leal
 ; X86-NEXT: movl
 ; X86-NEXT: mull
 ; X86-NEXT: leal
     %mul = mul i64 %A, 40
     ret i64 %mul
 }
+
+define i32 @mul4_32_minsize(i32 %A) minsize {
+; X64-LABEL: mul4_32_minsize:
+; X64: leal
+; X86-LABEL: mul4_32_minsize:
+; X86: shll
+    %mul = mul i32 %A, 4
+    ret i32 %mul
+}
+
+define i32 @mul40_32_minsize(i32 %A) minsize {
+; X64-LABEL: mul40_32_minsize:
+; X64: imull
+; X86-LABEL: mul40_32_minsize:
+; X86: imull
+    %mul = mul i32 %A, 40
+    ret i32 %mul
+}