llvm dev - Mar 2010 - [LLVMdev] RegisterScavenging on targets without subregisters

Kalle:

Your patch is similar to what I'd coded (and am testing, which means a
couple of hours before I consider committing). Other than cosmetic changes
and changing 'NULL' to '0' (it's an integer list, after
all). This patch now
causes new problems in the CellSPU backend (more stqd's and lqd's), so I
have to investigate those before committing the patch.

'make test' is our friend. 'make test' is our friend. 'make
test' is our
friend. :-)


-scooter

On Thu, Feb 25, 2010 at 11:25 PM, <Kalle.Raiskila at nokia.com> wrote:
>
> Scott Michel skrev:
> > No, I wasn't having a management lobotomy moment. If the
target's
> registers have
> > no subregisters, SubUsed is false and the assert gets tripped.
> >
> > Ok, back to the original question: What was the original intent in
this
> code
> > (lines 186-193 in lib/CodeGen/RegisterScavenging.cpp)?
>
> You beat me to it :). A simple bypass (patch attached) does at least not
> break 'make check' with x86 and SPU backends. (And fixes the
problem
> with call.ll-test breaking when the SPU bigframes patch is applied)
>
> kalle
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100226/f9dee0a0/attachment.html>

On Feb 26, 2010, at 10:09 AM, Scott Michel wrote:
> Kalle:
> 
> Your patch is similar to what I'd coded (and am testing, which means a
couple of hours before I consider committing). Other than cosmetic changes and
changing 'NULL' to '0' (it's an integer list, after all).
This patch now causes new problems in the CellSPU backend (more stqd's and
lqd's), so I have to investigate those before committing the patch.
I am afraid the assert is correct, you are ignoring the outer "if
(!isUsed(Reg))..."

Without sub-registers, the assert would simplify to:

    if (MO.isUse()) {
      assert(isUsed(Reg) && "Using an undefined register!")
    } else {
      assert(MO.isDef());

llc -verify-machineinstrs will probably tell you the same thing.

My guess is that CellSPU is not properly managing kill flags on machine
operands. That is a requirement before you can use the register scavenger. It
took a while to get ARM to that state.

/jakob

On Feb 28, 2010, at 11:49 PM, <Kalle.Raiskila at nokia.com>
<Kalle.Raiskila at nokia.com> wrote:
> Jakob Stoklund Olesen skrev:
>> On Feb 26, 2010, at 10:09 AM, Scott Michel wrote:
>>> This patch now causes new problems in the CellSPU
>>> backend (more stqd's and lqd's), so I have to investigate
those
>>> before committing the patch.
> 
> Yes, it is because the register scavenger unconditionally allocates a 
> spill slot from the stack, thus eliminating the possibility of a small 
> prologue and epilogue.
If you look at the ARM target, you will see multiple solutions.

- Thumb1 uses a fixed, reserved scratch register (r12) instead of the scavenger.

- If possible, an extra callee-saved register is spilled instead of allocating
an emergency spill slot.

- Finally, an emergency spill slot is only allocated if it is needed.
>> I am afraid the assert is correct, you are ignoring the outer "if
>> (!isUsed(Reg))..."
> 
> Hmm.. didn't miss it. Just didn't understand it :)
> So the problem is that a use-operation does not have its register marked 
> as used? I.e. something is badly broken?
Exactly. If a register is not marked as used, the scavenger thinks it is free to
be used as a scratch register. That is very bad if it was really needed.

It means that the backend is using a register after it had been <kill>ed.
(Or maybe before it was <def>ined).
>> My guess is that CellSPU is not properly managing kill flags on
>> machine operands. That is a requirement before you can use the
>> register scavenger. It took a while to get ARM to that state.
> 
> I am beginning to wonder if it was not a bad idea to try to enable it in 
> the first place... I'll look into the ARM backend and see if I get any 
> wiser.

On Mar 1, 2010, at 7:33 AM, Jakob Stoklund Olesen wrote:
> 
> On Feb 28, 2010, at 11:49 PM, <Kalle.Raiskila at nokia.com>
<Kalle.Raiskila at nokia.com> wrote:
> 
>> Jakob Stoklund Olesen skrev:
>>> On Feb 26, 2010, at 10:09 AM, Scott Michel wrote:
>>>> This patch now causes new problems in the CellSPU
>>>> backend (more stqd's and lqd's), so I have to
investigate those
>>>> before committing the patch.
>> 
>> Yes, it is because the register scavenger unconditionally allocates a 
>> spill slot from the stack, thus eliminating the possibility of a small 
>> prologue and epilogue.
> 
> If you look at the ARM target, you will see multiple solutions.
> 
> - Thumb1 uses a fixed, reserved scratch register (r12) instead of the
scavenger.
> 
<nitpick>

ARM mode used to reserve R12 for this. Thumb used to reserve R3. Both ARM and
Thumb now use the scavenger.

</nitpick>
> - If possible, an extra callee-saved register is spilled instead of
allocating an emergency spill slot.
> 
> - Finally, an emergency spill slot is only allocated if it is needed.
>>> I am afraid the assert is correct, you are ignoring the outer
"if
>>> (!isUsed(Reg))..."
>> 
>> Hmm.. didn't miss it. Just didn't understand it :)
>> So the problem is that a use-operation does not have its register
marked
>> as used? I.e. something is badly broken?
> 
> Exactly. If a register is not marked as used, the scavenger thinks it is
free to be used as a scratch register. That is very bad if it was really needed.
> 
> It means that the backend is using a register after it had been
<kill>ed. (Or maybe before it was <def>ined).
> 
>>> My guess is that CellSPU is not properly managing kill flags on
>>> machine operands. That is a requirement before you can use the
>>> register scavenger. It took a while to get ARM to that state.
>> 
>> I am beginning to wonder if it was not a bad idea to try to enable it
in
>> the first place... I'll look into the ARM backend and see if I get
any
>> wiser.
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev