[PATCH 1/2] RISC-V: Stop generating mapping symbol $x, replace with $x<isa>.
Andrew Oates
andrew@andrewoates.com
Fri Feb 21 21:11:51 GMT 2025
On Fri, Feb 21, 2025, 02:33 Nelson Chu <nelson@rivosinc.com> wrote:
> 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.
>
I agree this is a lot of work for the linker, but it has to do it anyway,
right? Otherwise it won't be able to correctly link psABI-compliant objects
generated by other tool chains (like llvm). Or is that a non-goal?
Alternatively: given the merging rules for the architecture strings when
linking objects files together, can we just say that having the linker
leave $x alone is always correct as it will refer to a superset
architecture strings of whatever the original is? (assuming there are no
incompatibilities, but that should be easily detected anyway)
Obligatory-I'm-not-an-expert :)
---
> gas/config/tc-riscv.c | 41 ++++++++++++++---------
> gas/testsuite/gas/riscv/mapping-symbols.d | 7 ++--
> 2 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 493c393f5b2..ee3980f43f1 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -539,7 +539,12 @@ make_mapping_symbol (enum riscv_seg_mstate state,
> name = buff;
> }
> else
> - name = "$x";
> + {
> + /* The $x without isa shouldn't be generated since it will put
> more
> + burden on the linker when linking objects with different
> isa. */
> + name = "$x";
> + as_warn (_("The mapping symbol $x shouldn't be generated"));
> + }
> break;
> default:
> abort ();
> @@ -585,11 +590,20 @@ 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 frag. */
> + const char *str = NULL;
> + symbolS *p = symbol_previous (symbol);
> + for (; p != NULL; p = symbol_previous (p))
> + {
> + if (symbol_get_frag (p) == frag
> + && 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 +621,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 +635,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 first instruction 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/20250221/0b1d23e2/attachment-0001.htm>
More information about the Binutils
mailing list