This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add support for R_MIPS_JALR
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 16 Sep 2009 11:10:44 -0700
- Subject: Re: [PATCH] Add support for R_MIPS_JALR
- References: <19118.39785.790622.796631@ropi.home> <871vm89fyc.fsf@firetop.home>
Richard Sandiford writes:
> Excellent patch, thanks.
Thanks.
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > +mrelax-pic-calls
> > +Target Report Mask(RELAX_PIC_CALLS)
> > +Try to turn PIC (indirect) calls into direct calls
>
> "Try to allow the linker to turn PIC calls into branches"? Bit clumsy,
> I know, but the current version seems to suggest that GCC can do the
> change itself, rather than simply emitting a hint for the linker.
>
> Better suggestions welcome!
I think yours is better. If we extend binutils to also transform jalr->jal
and jr->j in -mno-plt executables we technically wouldn't be creating
"branches" anymore but I can definitely live with that little ambiguity for
simplicity's sake.
> > + symbol = mips_pic_call_symbol_from_set (defs->ref, reg);
> > + if (!symbol)
> > + return NULL_RTX;
> > +
> > + /* If we have more than one definition, they need to be identical. */
> > + for (defs = defs->next; defs; defs = defs->next)
> > + {
> > + rtx other;
> > +
> > + other = mips_pic_call_symbol_from_set (defs->ref, reg);
> > + if (!rtx_equal_p (symbol, other))
> > + return NULL_RTX;
> > + }
>
> I'd be tempted to split this out into a new function and use it for
> the "reg-to-reg" copy case in mips_pic_call_symbol_from_set too.
> Things like scheduling could in principle introduce deliberate
> redundancies here, and since it's such an easy change to make...
I was trying to limit the complexity of the algorithm by only following one
set of multi-definitions but, sure, I can do that.
There is no need for a new function though as this is exactly what
mips_find_pic_call_symbol does. I.e.:
static rtx
mips_pic_call_symbol_from_set (df_ref def, rtx reg)
{
[...]
/* Follow simple register copies. */
if (REG_P (src))
mips_find_pic_call_symbol (def_insn, src);
> Why not imply -mabicalls too, rather than list it in every test?
Sure, I can do that. I was actually debating whether to imply -mabicalls
from -mshared but that wouldn't help with the -mno-shared -mno-plt tests.
I'll add -mrelax-pic-calls -> -mabicalls.
> As far as the generic testsuite changes go, I've a slight preference for
> adding -mno-relax-pic-calls instead skipping tests, but I don't feel too
> strongly about it.
That should work and that wouldn't change other targets. I'll try that.
Adam