[llvm-commits] [llvm] r105387 - in /llvm/trunk: lib/CodeGen/MachineSink.cpp test/CodeGen/X86/MachineSink-CritEdge.ll test/CodeGen/X86/sink-hoist.ll
Bill Wendling
isanbard at gmail.com
Thu Jun 3 16:37:45 PDT 2010
Yeah, it's not the best. I can put a limit in. But this will only be run if we come across an "imp-def,dead" preg that we want to move. And it will stop at the instruction we're hoping to move, if that preg hasn't been imp-def or killed before...
-bw
On Jun 3, 2010, at 10:36 AM, Evan Cheng wrote:
> I am concerned about the compile time impact. Perhaps we should limit the scanning to a small number of instructions and return true conservatively when it reaches the limit?
>
> Evan
>
> On Jun 3, 2010, at 12:54 AM, Bill Wendling wrote:
>
>> Author: void
>> Date: Thu Jun 3 02:54:20 2010
>> New Revision: 105387
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=105387&view=rev
>> Log:
>> Machine sink could potentially sink instructions into a block where the physical
>> registers it defines then interfere with an existing preg live range.
>>
>> For instance, if we had something like these machine instructions:
>>
>> BB#0
>> ... = imul ... EFLAGS<imp-def,dead>
>> test ..., EFLAGS<imp-def>
>> jcc BB#2 EFLAGS<imp-use>
>>
>> BB#1
>> ... ; fallthrough to BB#2
>>
>> BB#2
>> ... ; No code that defines EFLAGS
>> jcc ... EFLAGS<imp-use>
>>
>> Machine sink will come along, see that imul implicitly defines EFLAGS, but
>> because it's "dead", it assumes that it can move imul into BB#2. But when it
>> does, imul's "dead" imp-def of EFLAGS is raised from the dead (a zombie) and
>> messes up the condition code for the jump (and pretty much anything else which
>> relies upon it being correct).
>>
>> The solution is to know which pregs are live going into a basic block. However,
>> that information isn't calculated at this point. Nor does the LiveVariables pass
>> take into account non-allocatable physical registers. In lieu of this, we do a
>> *very* conservative pass through the basic block to determine if a preg is live
>> coming out of it.
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/MachineSink.cpp
>> llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll
>> llvm/trunk/test/CodeGen/X86/sink-hoist.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=105387&r1=105386&r2=105387&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineSink.cpp Thu Jun 3 02:54:20 2010
>> @@ -25,6 +25,7 @@
>> #include "llvm/Target/TargetRegisterInfo.h"
>> #include "llvm/Target/TargetInstrInfo.h"
>> #include "llvm/Target/TargetMachine.h"
>> +#include "llvm/ADT/SmallSet.h"
>> #include "llvm/ADT/Statistic.h"
>> #include "llvm/Support/Debug.h"
>> #include "llvm/Support/raw_ostream.h"
>> @@ -61,6 +62,7 @@
>> bool ProcessBlock(MachineBasicBlock &MBB);
>> bool SinkInstruction(MachineInstr *MI, bool &SawStore);
>> bool AllUsesDominatedByBlock(unsigned Reg, MachineBasicBlock *MBB) const;
>> + bool LiveOutOfBasicBlock(const MachineInstr *MI, unsigned Reg) const;
>> };
>> } // end anonymous namespace
>>
>> @@ -166,6 +168,44 @@
>> return MadeChange;
>> }
>>
>> +/// LiveOutOfBasicBlock - Determine if the physical register, defined and dead
>> +/// in MI, is live on exit from the basic block.
>> +bool MachineSinking::LiveOutOfBasicBlock(const MachineInstr *MI,
>> + unsigned Reg) const {
>> + assert(TargetRegisterInfo::isPhysicalRegister(Reg) &&
>> + "Only want to determine if a physical register is live out of a BB!");
>> +
>> + const MachineBasicBlock *MBB = MI->getParent();
>> + SmallSet<unsigned, 8> KilledRegs;
>> + MachineBasicBlock::const_iterator I = MBB->end();
>> + MachineBasicBlock::const_iterator E = MBB->begin();
>> + assert(I != E && "How can there be an empty block at this point?!");
>> +
>> + // Loop through the instructions bottom-up. If we see a kill of the preg
>> + // first, then it's not live out of the BB. If we see a use or def first, then
>> + // we assume that it is live out of the BB.
>> + do {
>> + const MachineInstr &CurMI = *--I;
>> +
>> + for (unsigned i = 0, e = CurMI.getNumOperands(); i != e; ++i) {
>> + const MachineOperand &MO = CurMI.getOperand(i);
>> + if (!MO.isReg()) continue; // Ignore non-register operands.
>> +
>> + unsigned MOReg = MO.getReg();
>> + if (MOReg == 0) continue;
>> +
>> + if (MOReg == Reg) {
>> + if (MO.isKill())
>> + return false;
>> + if (MO.isUse() || MO.isDef())
>> + return true;
>> + }
>> + }
>> + } while (I != E);
>> +
>> + return false;
>> +}
>> +
>> /// SinkInstruction - Determine whether it is safe to sink the specified machine
>> /// instruction out of its current block into a successor.
>> bool MachineSinking::SinkInstruction(MachineInstr *MI, bool &SawStore) {
>> @@ -188,6 +228,7 @@
>> // SuccToSinkTo - This is the successor to sink this instruction to, once we
>> // decide.
>> MachineBasicBlock *SuccToSinkTo = 0;
>> + SmallVector<unsigned, 4> PhysRegs;
>>
>> for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
>> const MachineOperand &MO = MI->getOperand(i);
>> @@ -216,9 +257,12 @@
>> if (AllocatableSet.test(AliasReg))
>> return false;
>> }
>> - } else if (!MO.isDead()) {
>> - // A def that isn't dead. We can't move it.
>> - return false;
>> + } else {
>> + if (!MO.isDead())
>> + // A def that isn't dead. We can't move it.
>> + return false;
>> + else
>> + PhysRegs.push_back(Reg);
>> }
>> } else {
>> // Virtual register uses are always safe to sink.
>> @@ -281,7 +325,15 @@
>> // happen with loops.
>> if (MI->getParent() == SuccToSinkTo)
>> return false;
>> -
>> +
>> + // If the instruction to move defines a dead physical register which is live
>> + // when leaving the basic block, don't move it because it could turn into a
>> + // "zombie" define of that preg. E.g., EFLAGS. (<rdar://problem/8030636>)
>> + for (SmallVectorImpl<unsigned>::const_iterator
>> + I = PhysRegs.begin(), E = PhysRegs.end(); I != E; ++I)
>> + if (LiveOutOfBasicBlock(MI, *I))
>> + return false;
>> +
>> DEBUG(dbgs() << "Sink instr " << *MI << "\tinto block " << *SuccToSinkTo);
>>
>> // If the block has multiple predecessors, this would introduce computation on
>>
>> Modified: llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll?rev=105387&r1=105386&r2=105387&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/MachineSink-CritEdge.ll Thu Jun 3 02:54:20 2010
>> @@ -1,4 +1,10 @@
>> ; RUN: llc < %s | FileCheck %s
>> +; XFAIL: *
>> +;
>> +; See <rdar://problem/8030636>. This test isn't valid after we made machine
>> +; sinking more conservative about sinking instructions that define a preg into a
>> +; block when we don't know if the preg is killed within the current block.
>> +
>> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
>> target triple = "x86_64-apple-darwin10.0.0"
>>
>>
>> Modified: llvm/trunk/test/CodeGen/X86/sink-hoist.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sink-hoist.ll?rev=105387&r1=105386&r2=105387&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/sink-hoist.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/sink-hoist.ll Thu Jun 3 02:54:20 2010
>> @@ -44,26 +44,33 @@
>>
>> ; Sink instructions with dead EFLAGS defs.
>>
>> -; CHECK: zzz:
>> -; CHECK: je
>> -; CHECK-NEXT: orb
>> -
>> -define zeroext i8 @zzz(i8 zeroext %a, i8 zeroext %b) nounwind readnone {
>> -entry:
>> - %tmp = zext i8 %a to i32 ; <i32> [#uses=1]
>> - %tmp2 = icmp eq i8 %a, 0 ; <i1> [#uses=1]
>> - %tmp3 = or i8 %b, -128 ; <i8> [#uses=1]
>> - %tmp4 = and i8 %b, 127 ; <i8> [#uses=1]
>> - %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; <i8> [#uses=1]
>> - ret i8 %b_addr.0
>> -}
>> +; FIXME: Unfail the zzz test if we can correctly mark pregs with the kill flag.
>> +;
>> +; See <rdar://problem/8030636>. This test isn't valid after we made machine
>> +; sinking more conservative about sinking instructions that define a preg into a
>> +; block when we don't know if the preg is killed within the current block.
>> +
>> +
>> +; FIXMEHECK: zzz:
>> +; FIXMEHECK: je
>> +; FIXMEHECK-NEXT: orb
>> +
>> +; define zeroext i8 @zzz(i8 zeroext %a, i8 zeroext %b) nounwind readnone {
>> +; entry:
>> +; %tmp = zext i8 %a to i32 ; <i32> [#uses=1]
>> +; %tmp2 = icmp eq i8 %a, 0 ; <i1> [#uses=1]
>> +; %tmp3 = or i8 %b, -128 ; <i8> [#uses=1]
>> +; %tmp4 = and i8 %b, 127 ; <i8> [#uses=1]
>> +; %b_addr.0 = select i1 %tmp2, i8 %tmp4, i8 %tmp3 ; <i8> [#uses=1]
>> +; ret i8 %b_addr.0
>> +; }
>>
>> ; Codegen should hoist and CSE these constants.
>>
>> ; CHECK: vv:
>> -; CHECK: LCPI3_0(%rip), %xmm0
>> -; CHECK: LCPI3_1(%rip), %xmm1
>> -; CHECK: LCPI3_2(%rip), %xmm2
>> +; CHECK: LCPI2_0(%rip), %xmm0
>> +; CHECK: LCPI2_1(%rip), %xmm1
>> +; CHECK: LCPI2_2(%rip), %xmm2
>> ; CHECK: align
>> ; CHECK-NOT: LCPI
>> ; CHECK: ret
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list