[llvm-commits] [llvm] r106066 - in /llvm/trunk: lib/CodeGen/MachineSink.cpp lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/MachineSink-CritEdge.ll

[Bill Wendling]

On Jun 15, 2010, at 5:13 PM, Jakob Stoklund Olesen wrote:

> 
> On Jun 15, 2010, at 4:46 PM, Bill Wendling wrote:
> 
>> Author: void
>> Date: Tue Jun 15 18:46:31 2010
>> New Revision: 106066
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=106066&view=rev
>> Log:
>> Create a more targeted fix for not sinking instructions into a range where it
>> will conflict with another live range. The place which creates this scenerio is
>> the code in X86 that lowers a select instruction by splitting the MBBs. This
>> eliminates the need to check from the bottom up in an MBB for live pregs.
> 
> Looks good to me.
> 
>> @@ -329,10 +285,14 @@
>>  // 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))
>> +  for (unsigned I = 0, E = MI->getNumOperands(); I != E; ++I) {
>> +    const MachineOperand &MO = MI->getOperand(I);
>> +    if (!MO.isReg()) continue;
>> +    unsigned Reg = MO.getReg();
>> +    if (Reg == 0 || !TargetRegisterInfo::isPhysicalRegister(Reg)) continue;
>> +    if (SuccToSinkTo->isLiveIn(Reg))
>>      return false;
>> +  }
> 
> This loop catches both physreg defs and uses. That is on purpose, right? The comment doesn't mention uses.
> 
Yes. It would be bad to move either a def or use into the live range it's not meant to inhabit. I'll modify the comment.

Thanks!

-bw