[llvm-commits] [llvm] r78650 - in /llvm/trunk: include/llvm/CodeGen/RegisterScavenging.h lib/CodeGen/RegisterScavenging.cpp test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll

[Evan Cheng]

On Aug 12, 2009, at 12:04 AM, Jakob Stoklund Olesen wrote:

>
> On 12/08/2009, at 07.46, Evan Cheng wrote:
>
>>
>> On Aug 11, 2009, at 12:17 PM, Jakob Stoklund Olesen wrote:
>>>
>>> I was thinking along similar lines, and I will be happy to code it
>>> up,
>>> but an issue must be addressed first: The scavengeRegister()  
>>> function
>>> may find an unused CSR, and it is not always clear how to handle it.
>>> That can happen for one of two reasons: 1) The CSR has been saved in
>>> the prologue and happens to be unused at this point, or 2) The CSR
>>> has
>>> not been spilled because it is not used in the function.
>>
>> Ok. There is sufficient information to distinguish the two cases,
>> right?
>
> Not really. The current code is quite fragile but errs on the safe
> side. It only clobbers the CSR if it isn't used any more in the MBB.
>
> We could use CalleeSavedInfo, but that is not very accurate when
> shrink wrapping is used.
>
> We can detect 3) PEI has not inserted CSR spills yet. Then
> CalleeSavedInfo will be empty.

Let's ignore shrink wrapping for now. We just need to know whether a  
register has been used (MachineRegisterInfo tracks this) and whether  
RS is being called before CSR spilling.

>
>>> In the first case, the CSR can be used directly without being saved.
>>> In the second case, it must be saved. If the CSR is used later in  
>>> the
>>> MBB, the scavenger assumes the first case. If it isn't used anymore
>>> in
>>> the MBB, we assume the second case and mark the CSR live before
>>> spilling it.
>>
>> My preference is to let the client of the scavenger to be responsible
>> for handling these details.
>
> I am not sure that is a good idea. We are distributing the handling of
> CSRs among three parties: PEI, scavenger, and the target.
>
> Blackfin uses this code whenever it needs a register:
>
>   unsigned Reg = RS->FindUnusedReg(RC, true);
>   if (Reg == 0)
>     Reg = RS->scavengeRegister(RC, II, SPAdj);
>
> It is completely clueless, it just wants a register. Only ARM seems to
> do something different.
>
> I would prefer to let PEI provide accurate live-in lists, let the
> scavenger track available registers accurately based on that info, and
> let the client simply ask for a register without worrying about which
> CSRs are spilled at this exact instance.

You mean RS should treat unused CSRs as if they are defined and livein  
to every MBB. Then RS is responsible for spilling and storing them?  
That's ok. But please don't add them to MBB liveins. The meanings of  
the two are completely different.

But you do see this results in poor code quality, right? We'll end up  
spilling and restoring unused CSRs many times rather than just  
spilling and restoring them in prologue and epilogue.

>
> [...]
>
>>> How do we handle this? Before CSR spill code is inserted, we should
>>> leave CSRs marked as available, and the scavenger should be free to
>>> use them without spilling first. After CSR spill code is inserted,
>>> all
>>
>> Right. PEI should spill them later.
>>
>>> the remaining CSRs should be marked live-in to every MBB in the
>>> function. That's what they are after all. If we do that, the
>>> scavenger
>>
>> We shouldn't be introducing them as livein to MBBs. Do you mean the
>> scavenger should treat them as if they are livein?
>
> Actually, I want to add them to the live-in lists. That way, the
> machine code verifier will also be able to detect clobbered CSRs.

That doesn't make sense to me. Why should unused CSRs be considered  
livein to MBBs by the machine code verifier? Nothing in the code  
should use undefined registers (unless it's explicitly marked undef).

>
> PEI is already doing this with spilled CSRs, is there a downside to
> doing it?


PEI is not doing this right. The spilled register operand should be  
marked "undef" since there wasn't a prior def.

>
>> will be able to use these register without any special handling. It
>>> can see that the free CSR is in use, and it will make sure to spill
>>> it
>>> to the emergency spill slot.
>>
>> That's probably a reasonable step in the right direction. But let's
>> expand on this a bit. I see the following scenarios:
>>
>> 1. Scavenger is used before PEI has saved callee-saved registers. All
>> that scavenger has to do is to mark the scavenged callee-saved
>> register as being used. Later on, PEI will save the CSR and all is
>> well.
>
> Yep.
>
>> 2. If it is used after CSR are spilled, and the chosen CSR has  
>> already
>> been spilled. There is nothing to do.
>
> Yes, but the scavenger needs a safe way of detecting this situation,
> even with shrink wrapping enabled.

Let's ignore shrink wrapping for now and solve the usual cases first.

>
>> 3. If it is used after CSR are spilled, and the chosen CSR has not
>> been spilled. This is the only real messy case.
>>   3a. Treat it as if a scavenged register. That is, spill to the
>> emergency slot and restore it after it has been used. This is
>> basically what you described.
>>   3b. The client should be informed a new CSR is used. It is
>> responsible for modifying the prologue and epilogue. I believe this  
>> is
>> the preferred solution. Of course, a client that's not capable of
>> modifying the prologue should not do this.
>>
>> What do you think?
>
> We could add a TRI->modifyPrologueAndEpilogue() callback, except with
> a better name. If it returns true, we can go ahead and use the CSR. If
> it returns false, we use the emergency slot.
>
> Either the scavenger or the target needs to make sure this callback is
> only called once per CSR per MBB.

Some kind of callback is fine. The target knows how to spill and  
restore registers.

>
>
> Right now, the scavenger is tracking certain CSRs as available, but it
> is not allowed to use them. That looks like a clobbered CSR just
> waiting to happen. These poison CSRs should be marked as used when
> entering the MBB. It does not /have/ to be done through the live-in
> lists - that just seems like a natural choice.

I would just mark the spill source register "undef" and the restore  
def "dead".

Evan

>
> Whatever we do, the scavenger and the machine code verifier should
> agree on the method.
>
> /jakob
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits