[llvm-commits] [llvm] r77904 - in /llvm/trunk: include/llvm/CodeGen/RegisterScavenging.h lib/CodeGen/RegisterScavenging.cpp test/CodeGen/Blackfin/load-intr.ll

[Evan Cheng]

On Aug 2, 2009, at 11:53 PM, Jakob Stoklund Olesen wrote:

>
> On 03/08/2009, at 07.42, Evan Cheng wrote:
>
>>>
>>> But this part is OK, right? <imp-def> can redefine a live register.
>>
>> It shouldn't. Are you talking about re-defining part of a larger
>> register? In that case, it should first kill the larger register and
>> then redefine it. At least that's what livevariable does. It's  
>> unclear
>> if it's realistic to enforce this in register scavenger. This might  
>> be
>> too difficult to maintain. I need to see an example.
>
> At least that makes the rules clearer - no special treatment for
> implicit operands. This means that you must always know if a register
> is dead or alive before use use of define it in any way. It could  
> work.

Right.

>
> I don't think the scavenger should enforce this rule through
> assertions. A double define does not break the scavenger in any way,
> it just makes it a bit less efficient - it is a missed available
> register, that is all. I think the machine code verifier should catch
> these bugs, and the scavenger should only assert when stuff would  
> break.

You are absolutely right. But we're not yet running the machine  
verifier by default that means we are using the scavenger for that  
purpose. It caught a lot of corner cases like these. On more than one  
occasion I'd thought about just removing the assertions. :-) Note the  
assertions are turned off in release mode so these bugs are being left  
through.

>
> Use of undefined registers is another matter. It means that a register
> was killed too soon, and the scavenger could mistakenly overwrite it.
> That would be bad, and an assert is in order.

Right. We ought to use the newly introduced error reporting mechanism  
instead. Assertions are not strong enough here. The only concern I  
have is the compile time cost. Some of the assertions checks are  
implemented less than optimally.

>
> I will implement the strict rules in the verifier. Then we can see how
> bad the status quo is and get some examples of breakage.

Ok thanks. This is really really hard to get right. I really  
appreciate that someone else is hacking on it. :-)

  Please see my replies to
[PATCH] Fix Bug 4657: register scavenger	asserts with subreg lowering
and
  [PATCH] Accidental <kill> on two-address operand
for more discussion on these issues.

Evan

>
> /jakob
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090803/cb9fd9ae/attachment.html>