[llvm-commits] [llvm] r71196 - in /llvm/trunk: lib/CodeGen/RegAllocLocal.cpp test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll

[Evan Cheng]

Well, I can think of an alternative solution. Why not inhibit  
reloading of reg1028? It's easy to tell since the only use is a copy  
(use is killed right away) and its def is dead.

Evan

On May 7, 2009, at 7:03 PM, Evan Cheng wrote:

> This is fine for local regalloc since performance is not critical.
> Thanks.
>
> Evan
>
> On May 7, 2009, at 4:47 PM, Bob Wilson wrote:
>
>> Author: bwilson
>> Date: Thu May  7 18:47:03 2009
>> New Revision: 71196
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=71196&view=rev
>> Log:
>> Fix pr4100.  Do not remove no-op copies when they are dead.  The
>> register
>> scavenger gets confused about register liveness if it doesn't see
>> them.
>> I'm not thrilled with this solution, but it only comes up when there
>> are dead
>> copies in the code, which is something that hopefully doesn't happen
>> much.
>>
>> Here is what happens in pr4100: As shown in the following excerpt
>> from the
>> debug output of llc, the source of a move gets reloaded from the
>> stack,
>> inserting a new load instruction before the move.  Since that source
>> operand
>> is a kill, the physical register is free to be reused for the
>> destination
>> of the move.  The move ends up being a no-op, copying R3 to R3, so
>> it is
>> deleted.  But, it leaves behind the load to reload %reg1028 into R3,
>> and
>> that load is not updated to show that it's destination operand (R3)
>> is dead.
>> The scavenger gets confused by that load because it thinks that R3
>> is live.
>>
>> Starting RegAlloc of: %reg1025<def,dead> = MOVr %reg1028<kill>, 14,
>> %reg0, %reg0
>> Regs have values:
>> Reloading %reg1028 into R3
>> Last use of R3[%reg1028], removing it from live set
>> Assigning R3 to %reg1025
>> Register R3 [%reg1025] is never used, removing it from live set
>>
>> Alternative solutions might be either marking the load as dead, or
>> zapping
>> the load along with the no-op copy.  I couldn't see an easy way to do
>> either of those, though.
>>
>> Added:
>>   llvm/trunk/test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll
>> Modified:
>>   llvm/trunk/lib/CodeGen/RegAllocLocal.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/RegAllocLocal.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocLocal.cpp?rev=71196&r1=71195&r2=71196&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> = 
>> =====================================================================
>> --- llvm/trunk/lib/CodeGen/RegAllocLocal.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/RegAllocLocal.cpp Thu May  7 18:47:03 2009
>> @@ -981,10 +981,12 @@
>>      }
>>    }
>>
>> -    // Finally, if this is a noop copy instruction, zap it.
>> +    // Finally, if this is a noop copy instruction, zap it.
>> (Except that if
>> +    // the copy is dead, it must be kept to avoid messing up
>> liveness info for
>> +    // the register scavenger.  See pr4100.)
>>    unsigned SrcReg, DstReg, SrcSubReg, DstSubReg;
>>    if (TII->isMoveInstr(*MI, SrcReg, DstReg, SrcSubReg, DstSubReg) &&
>> -        SrcReg == DstReg)
>> +        SrcReg == DstReg && DeadDefs.empty())
>>      MBB.erase(MI);
>>  }
>>
>>
>> Added: llvm/trunk/test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll?rev=71196&view=auto
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> = 
>> =====================================================================
>> --- llvm/trunk/test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll (added)
>> +++ llvm/trunk/test/CodeGen/ARM/2009-05-07-RegAllocLocal.ll Thu May
>> 7 18:47:03 2009
>> @@ -0,0 +1,12 @@
>> +; RUN: llvm-as < %s | llc -mtriple=armv5-unknown-linux-gnueabi -O0 -
>> regalloc=local
>> +; PR4100
>> + at .str = external constant [30 x i8]		; <[30 x i8]*> [#uses=1]
>> +
>> +define i16 @fn16(i16 %arg0.0, <2 x i16> %arg1, i16 %arg2.0)
>> nounwind {
>> +entry:
>> +	store <2 x i16> %arg1, <2 x i16>* null
>> +	%0 = call i32 (i8*, ...)* @printf(i8* getelementptr ([30 x i8]*
>> @.str, i32 0, i32 0), i32 0) nounwind		; <i32> [#uses=0]
>> +	ret i16 0
>> +}
>> +
>> +declare i32 @printf(i8*, ...) nounwind
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits