Re: [mips] fix $gp restore bug

[Richard Sandiford <rdsandiford at googlemail dot com>]

Adam Nemet <anemet@caviumnetworks.com> writes:
>>  I don't think (D)ADDIUPC would actually be of any help here -- the 16-bit 
>> immediate would not cover a range wider than the branch displacement 
>> already does.  I could imagine a "fix" where the addend would be shifted 
>> left by a few bits (the result could be masked to maintain some 
>> reasonable alignment), but that would only push the limit a little bit 
>> further, and then at a cost.
>
> No, I actually meant DADDUPC:
>
>         lui       $1,%hi(target-1f)
> 1:
>         (d)addiu  $1,$1,%lo(target-1b)
> 	(d)addupc $1,$1
>         jr        $1

Thanks for arguing the case here ;)  As well as getting us out of this hole,
it would remove the need for the "all calls must be through $25" rule.
That was the main reason why the new non-PIC executable support wasn't
entirely transparent to GCC.

Plus it would allow a nice PIC model for non-GOT-based bare-metal ABIs.
I know at least one customer in the past who needed this.

Oh well...

> Maciej W. Rozycki writes:
>>  I found the limitation of GCC that jumps cannot use a scratch register 
>> painful in the past already; my understanding is jumps are processed too 
>> late in the game for new register allocations to be possible (and the 
>> process would have to be iterative), correct?
>
> Yes if the scratch is only needed conditionally depending on the "final" code
> layout you need to preallocate the scratch register even though you might not
> use it at the end.

FWIW, the idea I'm toying with at the moment is:

  - Make every branch use pic_offset_table_rtx.

  - Try to detect cases where a function only needs a global pointer
    because of branches.  In this case, emit easily-identifiable "ghost"
    instructions (in the .md "type" attribute sense) to set, save and
    restore the gp.

    These ghost instructions would replace the sequences we'd normally use.
    They would have 0 length at this stage, and in theory shouldn't interfere
    with things like scheduling.

  - At the end of md_reorg, run shorten_branches to see whether
    all branches are in range.  This should be safe because we run
    dbr_schedule within mips_reorg.  Nothing after mips_reorg should
    change the insn stream besides shorten_branches itself.

  - If some branches are not in range, go through the insn stream and
    replace all the ghost instructions with real implementations.
    (This will of course invalidate the results of the shorten_branches
    call, but the "real" shorten_branches pass will then calculate new
    lengths.)

For added points, we could add some way of telling shorten_branches
that md_reorg has already calculated correct values (in the case
where all branches were in range).  This would avoid another,
redundant, pass.

Besides the complication, the main drawback I can see is that we'd be
unable to fill the delay slots of branches with insns that restore $gp,
even in the usual case where no long branches are needed.

That's certainly a problem for -Os.  I'm not sure it's much of a concern
for other options.  dbr_schedule doesn't take the pipeline into account
at all when filling delay slots, and postponing a restoration of $gp
isn't always a good idea performance-wise.

Richard