[OpenBIOS] [PATCH 2/4] ppc: add new context handler

[Segher Boessenkool]

On Mon, May 02, 2016 at 09:52:01AM +0100, Mark Cave-Ayland wrote:
> >> +struct context *switch_to(struct context *ctx)
> >> +{
> >> +    volatile struct context *save;
> >> +    struct context *ret;
> >> +    unsigned int lr;
> >> +
> >> +    debug("switching to new context:\n");
> >> +    save = __context;
> >> +    __context = ctx;
> >> +
> >> +    asm __volatile__ ("mflr %0\n\t" : "=r" (lr) : : );
> >> +    asm __volatile__ ("bl __switch_context\n");
> >> +    asm __volatile__ ("mtlr %0\n\t" : : "r" (lr) : );
> >> +    
> >> +    ret = __context;
> >> +    __context = (struct context *)save;
> >> +    return ret;
> >> +}
> > 
> > This isn't correct; GCC is free to put LR-modifying code between
> > the asm statements.  You should do it in one asm statement.
> 
> It seems the problem John was seeing is related to the compile flags in
> that the asm above don't work correctly when the binary is compiled with
> -Os. For comparison here is what I see with the different optimisation
> settings:
> 
> -O0 (works):
> 
>    0xfff08350 <+68>:    mflr    r9
>    0xfff08354 <+72>:    stw     r9,8(r31)
>    0xfff08358 <+76>:    bl      0xfff08060 <__switch_context>
>    0xfff0835c <+80>:    lwz     r9,8(r31)
>    0xfff08360 <+84>:    mtlr    r9
> 
> -Os (broken):
> 
>    0xfff082a8 <+32>:    mflr    r9
>    0xfff082ac <+36>:    bl      0xfff08060 <__switch_context>
>    0xfff082b0 <+40>:    mtlr    r9
> 
> So the optimiser has decided to remove the storage to/from the lr stack
> variable which of course means that the lr value isn't being preserved.
> Do I somehow need to mark the "r" (lr) as clobbered or something else?

Oh, yeah, that's another problem: a call can clobber all volatile
(caller-save) registers.  The best solution is to write this whole
thing in actual assembler, instead of fighting the compiler to get
just the assembler you want ;-)



Segher