This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add support for R_MIPS_JALR


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]