Skip to content

Commit

Permalink
[x86] machine combiner reassociation: mark EFLAGS operand as 'dead'
Browse files Browse the repository at this point in the history
In the commentary for D11660, I wasn't sure if it was alright to create new
integer machine instructions without also creating the implicit EFLAGS operand. 
From what I can see, the implicit operand is always created by the MachineInstrBuilder
based on the instruction type, so we don't have to do that explicitly. However, in
reviewing the debug output, I noticed that the operand was not marked as 'dead'. 
The machine combiner should do that to preserve future optimization opportunities 
that may be checking for that dead EFLAGS operand themselves.

Differential Revision: http://reviews.llvm.org/D11696

llvm-svn: 243990
  • Loading branch information
rotateright committed Aug 4, 2015
1 parent a00e997 commit 75ced27
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 5 deletions.
47 changes: 43 additions & 4 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
Expand Up @@ -6437,6 +6437,44 @@ bool X86InstrInfo::getMachineCombinerPatterns(MachineInstr &Root,
return false;
}

/// This is an architecture-specific helper function of reassociateOps.
/// Set special operand attributes for new instructions after reassociation.
static void setSpecialOperandAttr(MachineInstr &OldMI1, MachineInstr &OldMI2,
MachineInstr &NewMI1, MachineInstr &NewMI2) {
// Integer instructions define an implicit EFLAGS source register operand as
// the third source (fourth total) operand.
if (OldMI1.getNumOperands() != 4 || OldMI2.getNumOperands() != 4)
return;

assert(NewMI1.getNumOperands() == 4 && NewMI2.getNumOperands() == 4 &&
"Unexpected instruction type for reassociation");

MachineOperand &OldOp1 = OldMI1.getOperand(3);
MachineOperand &OldOp2 = OldMI2.getOperand(3);
MachineOperand &NewOp1 = NewMI1.getOperand(3);
MachineOperand &NewOp2 = NewMI2.getOperand(3);

assert(OldOp1.isReg() && OldOp1.getReg() == X86::EFLAGS && OldOp1.isDead() &&
"Must have dead EFLAGS operand in reassociable instruction");
assert(OldOp2.isReg() && OldOp2.getReg() == X86::EFLAGS && OldOp2.isDead() &&
"Must have dead EFLAGS operand in reassociable instruction");

(void)OldOp1;
(void)OldOp2;

assert(NewOp1.isReg() && NewOp1.getReg() == X86::EFLAGS &&
"Unexpected operand in reassociable instruction");
assert(NewOp2.isReg() && NewOp2.getReg() == X86::EFLAGS &&
"Unexpected operand in reassociable instruction");

// Mark the new EFLAGS operands as dead to be helpful to subsequent iterations
// of this pass or other passes. The EFLAGS operands must be dead in these new
// instructions because the EFLAGS operands in the original instructions must
// be dead in order for reassociation to occur.
NewOp1.setIsDead();
NewOp2.setIsDead();
}

/// Attempt the following reassociation to reduce critical path length:
/// B = A op X (Prev)
/// C = B op Y (Root)
Expand Down Expand Up @@ -6503,15 +6541,16 @@ static void reassociateOps(MachineInstr &Root, MachineInstr &Prev,
BuildMI(*MF, Prev.getDebugLoc(), TII->get(Opcode), NewVR)
.addReg(RegX, getKillRegState(KillX))
.addReg(RegY, getKillRegState(KillY));
InsInstrs.push_back(MIB1);

MachineInstrBuilder MIB2 =
BuildMI(*MF, Root.getDebugLoc(), TII->get(Opcode), RegC)
.addReg(RegA, getKillRegState(KillA))
.addReg(NewVR, getKillRegState(true));
InsInstrs.push_back(MIB2);

// Record old instructions for deletion.
setSpecialOperandAttr(Root, Prev, *MIB1, *MIB2);

// Record new instructions for insertion and old instructions for deletion.
InsInstrs.push_back(MIB1);
InsInstrs.push_back(MIB2);
DelInstrs.push_back(&Prev);
DelInstrs.push_back(&Root);
}
Expand Down
8 changes: 7 additions & 1 deletion llvm/test/CodeGen/X86/machine-combiner-int.ll
@@ -1,4 +1,5 @@
; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=x86-64 < %s | FileCheck %s
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=x86-64 | FileCheck %s
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=x86-64 -stop-after machine-combiner -o /dev/null 2>&1 | FileCheck %s --check-prefix=DEAD

; Verify that integer multiplies are reassociated. The first multiply in
; each test should be independent of the result of the preceding add (lea).
Expand All @@ -23,6 +24,11 @@ define i32 @reassociate_muls_i32(i32 %x0, i32 %x1, i32 %x2, i32 %x3) {
; CHECK-NEXT: imull %ecx, %edx
; CHECK-NEXT: imull %edx, %eax
; CHECK-NEXT: retq

; DEAD: ADD32rr
; DEAD-NEXT: IMUL32rr{{.*}}implicit-def dead %eflags
; DEAD-NEXT: IMUL32rr{{.*}}implicit-def dead %eflags

%t0 = add i32 %x0, %x1
%t1 = mul i32 %x2, %t0
%t2 = mul i32 %x3, %t1
Expand Down

0 comments on commit 75ced27

Please sign in to comment.