Re: [PATCH v4] MIPS: Support PCREL GOT access

[YunQiang Su <syq@gcc.gnu.org>]

Maciej W. Rozycki <macro@orcam.me.uk> 于2024年2月2日周五 21:30写道：
>
> On Fri, 2 Feb 2024, YunQiang Su wrote:
>
> > Current if we need to access a GOT entry, we use the
> > got_base + offset. Thus we have GOT and XGOT.
> > >From MIPSr6, we have PCREL instructions like ALUIPC,
>
>  I'm assuming ">" above is a typo; overall the change description may need
> a little bit polishing, but let's leave it until all the issues with code
> itself have been sorted.
>
> > so we have no need to use got_base now.
> > For pre-R6, we can use BAL to get the the value of PC.
>
>  I've skimmed over your change and spotted a couple of issues right away,
> as noted below.  I'll go through your change more thoroughly again.
>

Thanks.

>  Have the new relocations been formally documented anywhere?
>

Not yet. I wanted to put the documents in some subdirectory of binutils or gcc,
while it seems that these doc directories were used to hold the documents of
these projects only.

Is it a good idea to host the new document on Github or somewhere else?

For contents, I think that we should have a document for all GNU extensions.
Let's use the new relocs as the starting point.

> > diff --git a/bfd/elf64-mips.c b/bfd/elf64-mips.c
> > index 489a461bb0b..dc4e56d1f55 100644
> > --- a/bfd/elf64-mips.c
> > +++ b/bfd/elf64-mips.c
> > @@ -1670,6 +1782,118 @@ static reloc_howto_type mips_elf64_howto_table_rela[] =
> >        0x0000ffff,            /* dst_mask */
> >        true),                 /* pcrel_offset */
> >
> > +  HOWTO (R_MIPS_GOTPC_HI16,  /* type */
> > +      16,                    /* rightshift */
> > +      4,                     /* size */
> > +      16,                    /* bitsize */
> > +      true,                  /* pc_relative */
> > +      0,                     /* bitpos */
> > +      complain_overflow_signed, /* complain_on_overflow */
> > +      _bfd_mips_elf_generic_reloc,   /* special_function */
> > +      "R_MIPS_GOTPC_HI16",   /* name */
> > +      true,                  /* partial_inplace */
> > +      0x0000ffff,            /* src_mask */
> > +      0x0000ffff,            /* dst_mask */
> > +      true),                 /* pcrel_offset */
>
>  For RELA relocations you need to set `partial_inplace' to `false' and
> `src_mask' to 0, because they do not store the addend in the field
> relocated.  Likewise throughout and with n32.
>

Thanks. Fixed in my local repo. When you have more comments,
I will send V4.

> > diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> > index 69dd71419ff..c8a9ceb7524 100644
> > --- a/bfd/elfxx-mips.c
> > +++ b/bfd/elfxx-mips.c
> > @@ -2267,19 +2267,28 @@ got_page_reloc_p (unsigned int r_type)
> >  static inline bool
> >  got_lo16_reloc_p (unsigned int r_type)
> >  {
> > -  return r_type == R_MIPS_GOT_LO16 || r_type == R_MICROMIPS_GOT_LO16;
> > +  return r_type == R_MIPS_GOT_LO16
> > +      || r_type == R_MIPS_GOTPC_LO16
> > +      || r_type == R_MIPS_GOTPC_ALO16
> > +      || r_type == R_MICROMIPS_GOT_LO16;
> >  }
>
>  Please follow the GNU coding style, i.e.:
>
>   return (r_type == R_MIPS_GOT_LO16
>           || r_type == R_MIPS_GOTPC_LO16
>           || r_type == R_MIPS_GOTPC_ALO16
>           || r_type == R_MICROMIPS_GOT_LO16);
>
> Likewise throughout.
>

Ditto.

>   Maciej