[PATCH v2 1/2] RISC-V: Stop generating mapping symbol $x, replace with $x<isa>.

Nelson Chu nelson@rivosinc.com
Mon Mar 3 03:41:05 GMT 2025


Committed, thanks.  Since we always generate $x+isa for now, the
dis-assembler may also need something like cache to speedup the display
later.

Nelson

On Mon, Feb 24, 2025 at 3:36 PM Nelson Chu <nelson@rivosinc.com> wrote:

> v2 change,
> 1. We added $x+isa at start of all sections, so must find at least that one
> after searching all symbol table if S_GET_SEGMENT(s) == now_seg.
> 2. We shouldn't generate $x for now, that's internal error, so using
> gas_assert instead as_warn.
>
> The psABI defined $x to the architecture which is same as the file elf
> attribute.  But GNU defined it to that is same as the previous $x<isa>,
> and always generated $x<isa> at the begining of each section.  That is
> because considering two objects have different architecture in their elf
> attributes, then $x will always be wrong after linking since the merged
> arch string will be changed.  For example, object A with rv32ic and object
> B with rv32ia, $x from A is rv32ic and $x from B is rv32ia, but the final
> output is rv32ica, so $x from A and B need to be updated to rv32ic and
> rv32ia by linker respectively.  I think let linker to do this is not good,
> so in order to follow the psABI, we will stop generating the $x for now.
> Instead, all $x will be replaced with the corresponding $x<isa>.  The
> dis-assembler will also treat $x like what psABI defined.
> ---
>  gas/config/tc-riscv.c                     | 51 ++++++++++++-----------
>  gas/testsuite/gas/riscv/mapping-symbols.d |  7 ++--
>  2 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 493c393f5b2..ca269bd9a46 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -531,15 +531,12 @@ make_mapping_symbol (enum riscv_seg_mstate state,
>        name = "$d";
>        break;
>      case MAP_INSN:
> -      if (arch_str != NULL)
> -       {
> -         size_t size = strlen (arch_str) + 3; /* "$x" + '\0'  */
> -         buff = xmalloc (size);
> -         snprintf (buff, size, "$x%s", arch_str);
> -         name = buff;
> -       }
> -      else
> -       name = "$x";
> +      gas_assert (arch_str);
> +
> +      size_t size = strlen (arch_str) + 3; /* "$x" + '\0'  */
> +      buff = xmalloc (size);
> +      snprintf (buff, size, "$x%s", arch_str);
> +      name = buff;
>        break;
>      default:
>        abort ();
> @@ -585,11 +582,22 @@ make_mapping_symbol (enum riscv_seg_mstate state,
>
>    if (odd_data_padding)
>      {
> -      /* If the removed mapping symbol is $x+arch, then add it back to
> -        the next $x.  */
> -      const char *str = removed != NULL
> -                       && strncmp (S_GET_NAME (removed), "$xrv", 4) == 0
> -                       ? S_GET_NAME (removed) + 2 : NULL;
> +      /* Search and find the previous $x+isa which in the same section.
> +        We added $x+isa at the start of all sections, so should find at
> +        least that one.  */
> +      const char *str = NULL;
> +      symbolS *p = symbol_previous (symbol);
> +      for (; p != NULL; p = symbol_previous (p))
> +       {
> +         if (now_seg == S_GET_SEGMENT (p)
> +             && S_GET_NAME (p)
> +             && strncmp (S_GET_NAME (p), "$xrv", 4) == 0
> +             && S_GET_VALUE (p) <= S_GET_VALUE (symbol))
> +           {
> +             str = S_GET_NAME (p) + 2;
> +             break;
> +           }
> +       }
>        make_mapping_symbol (MAP_INSN, frag->fr_fix + 1, frag, str,
>                            false/* odd_data_padding */);
>      }
> @@ -607,7 +615,6 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
>  {
>    enum riscv_seg_mstate from_state =
>         seg_info (now_seg)->tc_segment_info_data.map_state;
> -  bool reset_seg_arch_str = false;
>
>    if (!SEG_NORMAL (now_seg)
>        /* For now we only add the mapping symbols to text sections.
> @@ -622,26 +629,22 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
>    symbolS *seg_arch_symbol =
>         seg_info (now_seg)->tc_segment_info_data.arch_map_symbol;
>    if (to_state == MAP_INSN && seg_arch_symbol == 0)
> -    {
> -      /* Always add $x+arch at the first instruction of section.  */
> -      reset_seg_arch_str = true;
> -    }
> +    /* The start of section.  */
> +    ;
>    else if (seg_arch_symbol != 0
>            && to_state == MAP_INSN
>            && !fr_align_code
>            && strcmp (riscv_rps_as.subset_list->arch_str,
>                       S_GET_NAME (seg_arch_symbol) + 2) != 0)
> -    {
> -      reset_seg_arch_str = true;
> -    }
> +    /* Different architecture string.  */
> +    ;
>    else if (from_state == to_state)
>      return;
>
>    valueT value = (valueT) (frag_now_fix () - max_chars);
>    seg_info (now_seg)->tc_segment_info_data.map_state = to_state;
>    seg_info (now_seg)->tc_segment_info_data.last_insn16 = false;
> -  const char *arch_str = reset_seg_arch_str
> -                        ? riscv_rps_as.subset_list->arch_str : NULL;
> +  const char *arch_str = riscv_rps_as.subset_list->arch_str;
>    make_mapping_symbol (to_state, value, frag_now, arch_str,
>                        false/* odd_data_padding */);
>  }
> diff --git a/gas/testsuite/gas/riscv/mapping-symbols.d
> b/gas/testsuite/gas/riscv/mapping-symbols.d
> index 057454bf991..ab1d4700e5f 100644
> --- a/gas/testsuite/gas/riscv/mapping-symbols.d
> +++ b/gas/testsuite/gas/riscv/mapping-symbols.d
> @@ -20,14 +20,13 @@ SYMBOL TABLE:
>  0+00 l    d  .text.odd.align.start.insn        0+00
> .text.odd.align.start.insn
>  0+00 l       .text.odd.align.start.insn        0+00 \$xrv32i2p1_c2p0
>  0+02 l       .text.odd.align.start.insn        0+00 \$d
> -0+08 l       .text.odd.align.start.insn        0+00 \$xrv32i2p1
>  0+00 l    d  .text.odd.align.start.data        0+00
> .text.odd.align.start.data
>  0+00 l       .text.odd.align.start.data        0+00 \$d
>  0+00 l    d  .text.zero.fill.first     0+00 .text.zero.fill.first
>  0+00 l       .text.zero.fill.first     0+00 \$xrv32i2p1_c2p0
>  0+00 l    d  .text.zero.fill.last      0+00 .text.zero.fill.last
>  0+00 l       .text.zero.fill.last      0+00 \$xrv32i2p1_c2p0
> -0+02 l       .text.zero.fill.last      0+00 \$x
> +0+02 l       .text.zero.fill.last      0+00 \$xrv32i2p1_c2p0
>  0+00 l    d  .text.zero.fill.align.A   0+00 .text.zero.fill.align.A
>  0+00 l       .text.zero.fill.align.A   0+00 \$xrv32i2p1_c2p0
>  0+00 l    d  .text.zero.fill.align.B   0+00 .text.zero.fill.align.B
> @@ -42,9 +41,9 @@ SYMBOL TABLE:
>  0+00 l    d  .text.relax.align 0+00 .text.relax.align
>  0+00 l       .text.relax.align 0+00 \$xrv32i2p1_c2p0
>  0+08 l       .text.relax.align 0+00 \$xrv32i2p1
> -0+0a l       .text.section.padding     0+00 \$x
> +0+0a l       .text.section.padding     0+00 \$xrv32i2p1_c2p0
>  0+03 l       .text.odd.align.start.insn        0+00 \$d
> -0+04 l       .text.odd.align.start.insn        0+00 \$x
> +0+04 l       .text.odd.align.start.insn        0+00 \$xrv32i2p1
>  0+01 l       .text.odd.align.start.data        0+00 \$d
>  0+02 l       .text.odd.align.start.data        0+00 \$xrv32i2p1_c2p0
>  0+00 l    d  .riscv.attributes 0+00 .riscv.attributes
> --
> 2.39.3 (Apple Git-146)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250303/29e00236/attachment-0001.htm>


More information about the Binutils mailing list