[PATCH] D11660: [x86] reassociate integer multiplies using machine combiner pass

[Sanjay Patel]

spatel created this revision.
spatel added reviewers: Gerolf, qcolombet, mkuper.
spatel added a subscriber: llvm-commits.

If I've done this correctly, then the patch is nearly trivial. :)
Is MachineOperand.isDead() the proper predicate for the EFLAGS operand of an integer math op that we want to reassociate?

This is part of fixing:
https://llvm.org/bugs/show_bug.cgi?id=21768

http://reviews.llvm.org/D11660

Files:
  lib/Target/X86/X86InstrInfo.cpp
  test/CodeGen/X86/machine-combiner-int.ll

Index: test/CodeGen/X86/machine-combiner-int.ll
===================================================================
--- test/CodeGen/X86/machine-combiner-int.ll
+++ test/CodeGen/X86/machine-combiner-int.ll
@@ -0,0 +1,43 @@
+; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=x86-64 < %s | FileCheck %s
+
+; Verify that integer multiplies are reassociated. The first multiply in 
+; each test should be independent of the result of the preceding add (lea).
+
+define i16 @reassociate_muls_i16(i16 %x0, i16 %x1, i16 %x2, i16 %x3) {
+; CHECK-LABEL: reassociate_muls_i16:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    leal   (%rdi,%rsi), %eax
+; CHECK-NEXT:    imull  %ecx, %edx
+; CHECK-NEXT:    imull  %edx, %eax
+; CHECK-NEXT:    retq
+  %t0 = add i16 %x0, %x1
+  %t1 = mul i16 %x2, %t0
+  %t2 = mul i16 %x3, %t1
+  ret i16 %t2
+}
+
+define i32 @reassociate_muls_i32(i32 %x0, i32 %x1, i32 %x2, i32 %x3) {
+; CHECK-LABEL: reassociate_muls_i32:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    leal   (%rdi,%rsi), %eax
+; CHECK-NEXT:    imull  %ecx, %edx
+; CHECK-NEXT:    imull  %edx, %eax
+; CHECK-NEXT:    retq
+  %t0 = add i32 %x0, %x1
+  %t1 = mul i32 %x2, %t0
+  %t2 = mul i32 %x3, %t1
+  ret i32 %t2
+}
+
+define i64 @reassociate_muls_i64(i64 %x0, i64 %x1, i64 %x2, i64 %x3) {
+; CHECK-LABEL: reassociate_muls_i64:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    leaq   (%rdi,%rsi), %rax
+; CHECK-NEXT:    imulq  %rcx, %rdx
+; CHECK-NEXT:    imulq  %rdx, %rax
+; CHECK-NEXT:    retq
+  %t0 = add i64 %x0, %x1
+  %t1 = mul i64 %x2, %t0
+  %t2 = mul i64 %x3, %t1
+  ret i64 %t2
+}
Index: lib/Target/X86/X86InstrInfo.cpp
===================================================================
--- lib/Target/X86/X86InstrInfo.cpp
+++ lib/Target/X86/X86InstrInfo.cpp
@@ -6300,11 +6300,18 @@
 
 static bool hasVirtualRegDefsInBasicBlock(const MachineInstr &Inst,
                                           const MachineBasicBlock *MBB) {
-  assert(Inst.getNumOperands() == 3 && "Reassociation needs binary operators");
+  assert((Inst.getNumOperands() == 3 || Inst.getNumOperands() == 4) &&
+         "Reassociation needs binary operators");
   const MachineOperand &Op1 = Inst.getOperand(1);
   const MachineOperand &Op2 = Inst.getOperand(2);
   const MachineRegisterInfo &MRI = MBB->getParent()->getRegInfo();
 
+  // For binary instructions that have a third source operand (integer ops have
+  // EFLAGS), that operand must be dead.
+  if (Inst.getNumOperands() == 4)
+    if (!Inst.getOperand(3).isDead())
+      return false;
+  
   // We need virtual register definitions.
   MachineInstr *MI1 = nullptr;
   MachineInstr *MI2 = nullptr;
@@ -6350,6 +6357,10 @@
 //       2. Other math / logic operations (and, or)
 static bool isAssociativeAndCommutative(const MachineInstr &Inst) {
   switch (Inst.getOpcode()) {
+  case X86::IMUL16rr:
+  case X86::IMUL32rr:
+  case X86::IMUL64rr:
+    return true;
   case X86::ADDSDrr:
   case X86::ADDSSrr:
   case X86::VADDSDrr:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11660.31066.patch
Type: text/x-patch
Size: 2949 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150730/70ceda8c/attachment.bin>