[PATCH] RISC-V: Do fixup for local symbols while with "-mno-relax"

Nelson Chu nelson@rivosinc.com
Tue Dec 12 09:46:10 GMT 2023


The idea looks good to me.  Passed regressions of riscv-gnu-toolchain, so
committed with some minor changes and indent fixes.

There are some TODOs, but no rush to do for now,
1. The implementation is based on the code from bfd/elfnn-riscv.c.  We
probably can move the code to bfd/elfxx-riscv.c, so that can reduce
duplicate code, just like what we did for the architecture parser.  Before
that, I renamed functions and variables from *reloc* to *fixup*, to
distinguish the code from bfd/elfnn-riscv.c, since they are still a little
bit different.
2. Maybe not only pcrel_hi/lo12 relocation with local symbols can be resolved
at assembler time.  Other pc-relative relocation, like branch, may also be
able to perform related optimizations.

Thanks
Nelson

On Wed, Nov 29, 2023 at 5:17 PM <lifang_xia@linux.alibaba.com> wrote:

> From: Lifang Xia <lifang_xia@linux.alibaba.com>
>
> In the scenario of generating .ko files, the kernel does not relax the .ko
> files. However, due to the large amount of relax and local relocation
> information, this increases the size of the .ko files.
>
> In this patch, it will finish the fixup of the local relocations while with
> "-mno-relax" option. This can reduce the size of the relocation table.
>
> gas/
>         * config/tc-riscv.c (struct riscv_pcrel_hi_reloc): New, reference
>           from bfd/elfnn-riscv.c.
>           (riscv_pcrel_hi_reloc_hash): Likewise.
>           (riscv_pcrel_reloc_hash): Likewise.
>           (riscv_pcrel_reloc_eq): Likewise.
>           (riscv_record_pcrel_reolc): Likewise.
>           (md_begin): Init pcrel_hi hash.
>           (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_HI20>: Do fixup and
>           record the pcrel_hi relocs, mark as done while with
>           "-mno-relax".
>           (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_LO12_I>:
>           (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_LO12_S>: Do fixup
>           and mark as done while with "-mno-relax".
>           (riscv_md_end): New, delete riscv_pcrel_hi_reloc_hash.
>         * config/tc-riscv.h (md_end): Define md_end with riscv_md_end.
> gas/
>         * testsuite/gas/riscv/fixup-local*: New tests.
> ---
>  gas/config/tc-riscv.c                         | 111 +++++++++++++++++-
>  gas/config/tc-riscv.h                         |   3 +
>  gas/testsuite/gas/riscv/fixup-local-norelax.d |  23 ++++
>  gas/testsuite/gas/riscv/fixup-local-relax.d   |  41 +++++++
>  gas/testsuite/gas/riscv/fixup-local.s         |  13 ++
>  5 files changed, 190 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/riscv/fixup-local-norelax.d
>  create mode 100644 gas/testsuite/gas/riscv/fixup-local-relax.d
>  create mode 100644 gas/testsuite/gas/riscv/fixup-local.s
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 04738d5e00c..332103574fe 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1569,6 +1569,58 @@ init_opcode_hash (const struct riscv_opcode
> *opcodes,
>    return hash;
>  }
>
> +/* Record all PC-relative high-part relocs we've encountered to help us
> +   later resolve the corresponding low-part relocs.  */
> +struct riscv_pcrel_hi_reloc
> +{
> +  bfd_vma address;
> +  symbolS *symbol;
> +  bfd_vma target;
> +};
> +
> +/* Handle of the pcrel_hi hash table.  */
> +static htab_t riscv_pcrel_hi_reloc_hash;
> +
> +/* Get the key of a entry from the pcrel_hi hash table.  */
> +
> +static hashval_t
> +riscv_pcrel_reloc_hash (const void *entry)
> +{
> +  const struct riscv_pcrel_hi_reloc *e = entry;
> +  return (hashval_t) (e->address);
> +}
> +
> +/* Compare the keys between two entries fo the pcrel_hi hash table.  */
> +
> +static int
> +riscv_pcrel_reloc_eq (const void *entry1, const void *entry2)
> +{
> +  const struct riscv_pcrel_hi_reloc *e1 = entry1, *e2 = entry2;
> +  return e1->address == e2->address;
> +}
> +
> +/* Record the pcrel_hi relocs.  */
> +
> +static bool
> +riscv_record_pcrel_reolc (htab_t p, bfd_vma address, symbolS *symbol,
> +                          bfd_vma target)
> +{
> +  struct riscv_pcrel_hi_reloc entry = { address, symbol, target };
> +  struct riscv_pcrel_hi_reloc **slot
> +      = (struct riscv_pcrel_hi_reloc **)htab_find_slot (p, &entry,
> INSERT);
> +  if (slot == NULL)
> +    return false;
> +
> +  *slot = (struct riscv_pcrel_hi_reloc *)xmalloc (
> +      sizeof (struct riscv_pcrel_hi_reloc));
> +  if (*slot != NULL)
> +    {
> +      **slot = entry;
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* This function is called once, at assembler startup time.  It should
> set up
>     all the tables, etc. that the MD part of the assembler will need.  */
>
> @@ -1605,6 +1657,11 @@ md_begin (void)
>    opcode_names_hash = str_htab_create ();
>    init_opcode_names_hash ();
>
> +  /* Create pcrel_hi hash table to resolve the relocation while with
> +     -mno-relax.  */
> +  riscv_pcrel_hi_reloc_hash
> +      = htab_create (1024, riscv_pcrel_reloc_hash, riscv_pcrel_reloc_eq,
> free);
> +
>    /* Set the default alignment for the text section.  */
>    record_alignment (text_section, riscv_opts.rvc ? 1 : 2);
>  }
> @@ -4213,9 +4270,52 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
> ATTRIBUTE_UNUSED)
>        break;
>
>      case BFD_RELOC_RISCV_PCREL_HI20:
> +      /* record the pcrel_hi relocs of the local symbols. And evaluate
> the hi20
> +         of the lcoal symbols. Fill in a tentative value to improve
> objdump
> +         readability for -mrelax, and set fx_done for -mno-relax.  */
> +      if (S_IS_LOCAL (fixP->fx_addsy) && fixP->fx_addsy
> +          && S_GET_SEGMENT (fixP->fx_addsy) == seg)
> +        {
> +          bfd_vma target = S_GET_VALUE (fixP->fx_addsy) + *valP;
> +          bfd_vma value = target - md_pcrel_from (fixP);
> +
> +          /* Record PCREL_HI20.  */
> +          if (!riscv_record_pcrel_reolc (riscv_pcrel_hi_reloc_hash,
> +                                         md_pcrel_from (fixP),
> fixP->fx_addsy,
> +                                         target))
> +            {
> +              as_warn ("too many pcrel_hi");
> +            }
> +
> +          bfd_putl32 (bfd_getl32 (buf)
> +                          | ENCODE_UTYPE_IMM (RISCV_CONST_HIGH_PART
> (value)),
> +                      buf);
> +          if (!riscv_opts.relax)
> +            fixP->fx_done = 1;
> +        }
> +      relaxable = riscv_opts.relax;
> +      break;
>      case BFD_RELOC_RISCV_PCREL_LO12_S:
>      case BFD_RELOC_RISCV_PCREL_LO12_I:
> -      relaxable = riscv_opts.relax;
> +      {
> +        /* Resolve the low12 of the local symboles with the pcrel_hi
> relocs.
> +           Fill in a tentative value to improve objdump readability for
> +           -mrelax, and set fx_done for -mno-relax.  */
> +        bfd_vma location_pcrel_hi = S_GET_VALUE (fixP->fx_addsy) + *valP;
> +        struct riscv_pcrel_hi_reloc search = { location_pcrel_hi, 0, 0 };
> +        struct riscv_pcrel_hi_reloc *entry
> +            = htab_find (riscv_pcrel_hi_reloc_hash, &search);
> +        if (entry && entry->symbol && S_IS_LOCAL (entry->symbol)
> +            && S_GET_SEGMENT (entry->symbol) == seg)
> +          {
> +            bfd_vma target = entry->target;
> +            bfd_vma value = target - entry->address;
> +            bfd_putl32 (bfd_getl32 (buf) | ENCODE_ITYPE_IMM (value), buf);
> +            if (!riscv_opts.relax)
> +              fixP->fx_done = 1;
> +          }
> +        relaxable = riscv_opts.relax;
> +      }
>        break;
>
>      case BFD_RELOC_RISCV_ALIGN:
> @@ -4980,6 +5080,15 @@ riscv_md_finish (void)
>    riscv_set_public_attributes ();
>    if (riscv_opts.relax)
>      bfd_map_over_sections (stdoutput, riscv_insert_uleb128_fixes, NULL);
> +
> +}
> +
> +/* Called just before the assembler exits.  */
> +
> +void
> +riscv_md_end (void)
> +{
> +  htab_delete (riscv_pcrel_hi_reloc_hash);
>  }
>
>  /* Adjust the symbol table.  */
> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> index 0c70c7d4739..9d2f05a4241 100644
> --- a/gas/config/tc-riscv.h
> +++ b/gas/config/tc-riscv.h
> @@ -80,6 +80,9 @@ extern int riscv_parse_long_option (const char *);
>  extern void riscv_pre_output_hook (void);
>  #define GAS_SORT_RELOCS 1
>
> +#define md_end riscv_md_end
> +extern void riscv_md_end (void);
> +
>  /* Let the linker resolve all the relocs due to relaxation.  */
>  #define tc_fix_adjustable(fixp) 0
>  #define md_allow_local_subtract(l,r,s) 0
> diff --git a/gas/testsuite/gas/riscv/fixup-local-norelax.d
> b/gas/testsuite/gas/riscv/fixup-local-norelax.d
> new file mode 100644
> index 00000000000..22132a32f91
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/fixup-local-norelax.d
> @@ -0,0 +1,23 @@
> +#as: -march=rv64i -mno-relax
> +#source: fixup-local.s
> +#objdump: -dr
> +
> +tmpdir/fixup-local.o:     file format elf64-littleriscv
> +
> +
> +Disassembly of section .text:
> +
> +0+0000 <foo>:
> +[      ]+0:[   ]+00000517[     ]+auipc a0,0x0
> +[      ]+4:[   ]+00850513[     ]+addi  a0,a0,8 # 8 <foo\+0x8>
> +[      ]+8:[   ]+00000517[     ]+auipc a0,0x0
> +[              ]+8: R_RISCV_PCREL_HI20 bar
> +[      ]+c:[   ]+00050513[     ]+mv    a0,a0
> +[              ]+c: R_RISCV_PCREL_LO12_I       .L0
> +[      ]+10:[  ]+00000517[     ]+auipc a0,0x0
> +[              ]+10: R_RISCV_PCREL_HI20        foo
> +[      ]+14:[  ]+00050513[     ]+mv    a0,a0
> +[              ]+14: R_RISCV_PCREL_LO12_I      .L0
> +[      ]+18:[  ]+00000517[     ]+auipc a0,0x0
> +[      ]+1c:[  ]+00852503[     ]+lw    a0,8\(a0\) # 20 <foo\+0x20>
> +[      ]+20:[  ]+00008067[     ]+ret
> diff --git a/gas/testsuite/gas/riscv/fixup-local-relax.d
> b/gas/testsuite/gas/riscv/fixup-local-relax.d
> new file mode 100644
> index 00000000000..49c475c5dcb
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/fixup-local-relax.d
> @@ -0,0 +1,41 @@
> +#as: -march=rv64i
> +#source: fixup-local.s
> +#objdump: -dr
> +
> +tmpdir/fixup-local.o:     file format elf64-littleriscv
> +
> +
> +Disassembly of section .text:
> +
> +0+0000 <foo>:
> +[      ]+0:[   ]+00000517[     ]+auipc a0,0x0
> +[              ]+0: R_RISCV_PCREL_HI20 .LL0
> +[              ]+0: R_RISCV_RELAX      \*ABS\*
> +[      ]+4:[   ]+00850513[     ]+addi  a0,a0,8 # 8 <.LL0>
> +[              ]+4: R_RISCV_PCREL_LO12_I       .L0
> +[              ]+4: R_RISCV_RELAX      \*ABS\*
> +
> +0000000000000008 <.LL0>:
> +[      ]+8:[   ]+00000517[     ]+auipc a0,0x0
> +[              ]+8: R_RISCV_PCREL_HI20 bar
> +[              ]+8: R_RISCV_RELAX      \*ABS\*
> +[      ]+c:[   ]+00050513[     ]+mv    a0,a0
> +[              ]+c: R_RISCV_PCREL_LO12_I       .L0
> +[              ]+c: R_RISCV_RELAX      \*ABS\*
> +[      ]+10:[  ]+00000517[     ]+auipc a0,0x0
> +[              ]+10: R_RISCV_PCREL_HI20        foo
> +[              ]+10: R_RISCV_RELAX     \*ABS\*
> +[      ]+14:[  ]+00050513[     ]+mv    a0,a0
> +[              ]+14: R_RISCV_PCREL_LO12_I      .L0
> +[              ]+14: R_RISCV_RELAX     \*ABS\*
> +
> +0000000000000018 <.LL1>:
> +[      ]+18:[  ]+00000517[     ]+auipc a0,0x0
> +[              ]+18: R_RISCV_PCREL_HI20        .LL2
> +[              ]+18: R_RISCV_RELAX     \*ABS\*
> +[      ]+1c:[  ]+00852503[     ]+lw    a0,8\(a0\) # 20 <.LL2>
> +[              ]+1c: R_RISCV_PCREL_LO12_I      .LL1
> +[              ]+1c: R_RISCV_RELAX     \*ABS\*
> +
> +0000000000000020 <.LL2>:
> +[      ]+20:[  ]+00008067[     ]+ret
> diff --git a/gas/testsuite/gas/riscv/fixup-local.s
> b/gas/testsuite/gas/riscv/fixup-local.s
> new file mode 100644
> index 00000000000..44b47311235
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/fixup-local.s
> @@ -0,0 +1,13 @@
> +.global foo
> +.global bar
> +foo:
> +       la a0, .LL0
> +.LL0:
> +       la a0, bar
> +       la a0, foo
> +.LL1:
> +       auipc a0, %pcrel_hi(.LL2)
> +       lw    a0, %pcrel_lo(.LL1)(a0)
> +
> +.LL2:
> +       ret
> --
> 2.39.2 (Apple Git-143)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20231212/fb2eaca1/attachment-0001.htm>


More information about the Binutils mailing list