[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

[Jakob Stoklund Olesen]

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.

>> 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.

[...]

>> 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.

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

> 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.

> 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.


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.

Whatever we do, the scavenger and the machine code verifier should  
agree on the method.

/jakob