[llvm-commits] [llvm] r66976 - in /llvm/trunk: lib/CodeGen/PHIElimination.cpp lib/Transforms/Scalar/CodeGenPrepare.cpp test/CodeGen/X86/2009-03-13-PHIElimBug.ll test/CodeGen/X86/split-eh-lpad-edges.ll

[Evan Cheng]

On Mar 14, 2009, at 3:40 AM, Duncan Sands wrote:

> Hi Evan, thanks for fixing this.
>
>> +void PNE::WalkPassEHTryRange(MachineBasicBlock &MBB,
>> +                             MachineBasicBlock::iterator &I,  
>> unsigned SrcReg) {
>> +  if (I == MBB.begin())
>> +    return;
>> +  MachineBasicBlock::iterator PI = prior(I);
>> +  if (PI->getOpcode() != TargetInstrInfo::EH_LABEL)
>> +    return;
>
> If the invoke returns a result, then the block will not end with an  
> EH_LABEL,
> it will end with a register copy.  For example:
>
>        %reg1031<def> = MOV32rr %EAX
>        EH_LABEL 2
>        %reg1025<def> = MOV32rr %reg1031  <= Invoke result, not an  
> EH_LABEL
>        JMP mbb<entry.cont_crit_edge,0xa11af90>
>
> So in this case the logic bails out, putting the copy at the end of  
> the BB,
> even though it might still need to be before the invoke.

Something seems wrong with this. Who inserted the copy? The copy from  
EAX to reg1031 is the copy lowered by isel. Is PHI elimination  
inserting the copy from reg1031 to reg1025? Then it should have done  
the right thing.

>
>
>> +  // Trying to walk pass the EH try range. If we run into a use  
>> instruction,
>> +  // we want to insert the copy there.
>
> Why is that?  I don't understand why uses matter.  Can't you just  
> always put the
> copy immediately after the definition of SrcReg?  If so, you  
> wouldn't need to
> scan for EH_LABEL's etc.

For 2 reasons. 1. PHI elimination expects the copy to be the last use  
of the src register (unless the terminator is), it will mark it kill.  
If it marks it incorrectly bad things will happen. 2. It should be the  
last use to make it easier to coalesce away the copy.

Evan
>
>
> Ciao,
>
> Duncan.