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

[Jan Beulich]

On 21.02.2025 10:07, Nelson Chu wrote:
> On Fri, Feb 21, 2025 at 4:27 PM Jan Beulich <jbeulich@suse.com> wrote:
> 
>> On 21.02.2025 07:33, Nelson Chu wrote:
>>> --- 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"));
>>> +     }
>>
>> What's the user to do about this, if it appears? Isn't this an internal
> 
> error?
> 
> 
>> Also, nit: diagnostics normally don't start with a capital letter.
>>
> 
> Git it, thanks.
> 
> 
>>
>>> @@ -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 */);
>>
>> Are there any guarantees that str will not end up as NULL here? I.e. that
>> such a "previous $x+isa" will be found?
> 
> 
> In theory, there must be since we always added $x+isa at the beginning of
> each section.

But you're looking for something in the same frag, i.e. generally unrelated
to what's at the start of a section.

>  The odd paddings are handled in the riscv_handle_align at
> the end, which all other normal mapping symbols should already be added,
> including all $x+isa at the beginning of each section.  But that's in
> theory, even though I never got the above as_warn messages for now after
> building the multi-lib toolchains with newlib/glibc, and also passing the
> gcc/binutils regressions.  So I use as_warn rather as_bad, since it should
> just be a display issue for now, maybe just a warning without stopping
> building or compiling it.  But yeah it's eventually an internal error, so
> as_bad may be more suitable.

Hmm, my take is that internal errors are normally flagged using gas_assert()
or abort(). Like as_warn(), as_bad() normally indicates to the user that they
need to change their code, for not fulfilling a certain requirement.

Jan