[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

[John Mosby]

On Wed, Aug 12, 2009 at 1:04 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk>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.


Seems like it has to be pushed down to the MBB...

We can detect 3) PEI has not inserted CSR spills yet. Then
> CalleeSavedInfo will be empty.


CalleeSavedInfo just says which CSRs are used in at least one
MBB in a function. It assumes all used CSRs will be spilled in
entry and restored in return blocks.


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


Actually ARM does that also.


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


I agree


> [...]
>
> >> 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?


I agree with Jakob on this. Is it that there needs to be a more general

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


With shrink wrapping, you need to know where a given CSR is spilled, i.e.
the region in which it is spilled/restored, which is no longer the entire
function.

What I plan to do, not yet implementing it, is allocate all regs in one pass
of
PEI, possibly introducing new CSR uses requiring spills, recompute the
frame,
then place the spills and restores (shrink wrap). This way there is no need
to
keep track of whether or not a CSR has been spilled yet.

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


That sounds fine to me.


> 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 committed an RS patch to this effect, but Evan backed it out. I don't see
CSR clobbering possible, what am I missing here?
Do you mean using FindUnusedReg() with ExCalleeSaved=false? (sorry, it's
late)


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


Certainly.

John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090812/1ba15729/attachment.html>