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

[Bob Wilson]

I agree that would be better.  After looking at for a while today,  
though, I think it may be beyond my current understanding of the code  
to implement that.  Maybe I can come back to it later.

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

> 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
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits