[RFC PATCH 0/5] RISC-V: Support mapping symbols with ISA string

[Tsukasa OI <research_trasio@irq.a4lg.com>]

Hello,

After a detailed analysis of Nelson's patch, I made my own version based
on it.  It clearly lacks proper attribution (Signed-Off-By lines) and
should be considered as RFC.  Nelson, I would like to hear your thoughts.

Nelson's original patch:
<https://sourceware.org/pipermail/binutils/2022-August/122220.html>
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_mapping_sym_with_arch>

    Also see:

    [1] Proposal: Extend .option directive for control enabled extensions
    on specific code region,
    <https://github.com/riscv-non-isa/riscv-asm-manual/pull/67>

    [2] Proposal: Add mapping symbol,
    <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196>



I found two issues on his patch:

(1) "$x" is the start of instructions with the default architecture.
Nelson's patch effectively handled it as a change to code but with no ISA
changes and this is clearly against Kito's proposal:

<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196/files#diff-361ae9d87b1e297413d1551ec0743e2e893217198eac78e95568a3718be26fb1R805>

> an instruction mapping symobl
> without ISA string means using ISA configuration from ELF attribute.

In my version, "$x" is emitted if no architecture string is changed by
.option directives and such.  Even if the arch is changed to revert the
original, it's not the same:

    # Example
    # (we have default architecture here such as RV32I)
    .option arch, +c
    .option arch, -c
    # (not default, even if the resulting ISA string is the same)

The only exception is when we use ".option push" before the architecture is
changed and that state is restored by ".option pop".

    # Example
    # (we have default architecture here)
    .option push
    # (now we can make changes to the architecture)
    .option arch, +c
    # (... then restore it)
    .option pop
    # (now we have the default architecture, again)

This also applies to the disassembler.  If we meet the "$x" mapping symbol,
we need to revert to the default architecture.  Since we need to save the
default architecture, the disassembler part is a bit more complex than
Nelson's implementation.


(2) Also, the mapping symbols are handled per section but "current"
architecture as managed by the assembler is not.  It did work previously
because we didn't have to keep track of the architecture (ISA string) but
we have to explicitly handle that now.

So, the first instruction after the section change requires emitting the
mapping symbol if necessary.  Still, to do that, looking up the last
mapping symbol is sufficient (saving RISC-V preset list is not required).


My changes made MAP_INSN_ARCH not necessary so I removed it.


I also added four test cases (mapping-{05..08}) whether we are handling
complex situations correctly.  Note that mapping-{05,06,08} will fail on
Nelson's implementation.



p.s.

Nelson, are you going to upstream this feature now or wait until the
proposal by Kito is approved?  If your answer is the latter, I will submit
my disassembler changes first (to make the disassembler support of this
feature cleaner).



Thanks,
Tsukasa




Tsukasa OI (5):
  RISC-V: Use bool on riscv_set_options members
  gas: Copyediting on tc-riscv.c
  RISC-V: Mapping symbols with ISA string on assembler
  RISC-V: Mapping symbols with ISA string on disassembler
  RISC-V: New testcases for mapping symbols w/ ISA

 gas/config/tc-riscv.c                         | 67 +++++++++++++++----
 gas/config/tc-riscv.h                         |  5 ++
 gas/testsuite/gas/riscv/mapping-01a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-02a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-03a.d         | 10 +--
 gas/testsuite/gas/riscv/mapping-04a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-05.s          | 11 +++
 gas/testsuite/gas/riscv/mapping-05a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-05b.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-06.s          | 11 +++
 gas/testsuite/gas/riscv/mapping-06a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-06b.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-07.s          | 12 ++++
 gas/testsuite/gas/riscv/mapping-07a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-07b.d         | 21 ++++++
 gas/testsuite/gas/riscv/mapping-08.s          | 34 ++++++++++
 gas/testsuite/gas/riscv/mapping-08a.d         | 18 +++++
 gas/testsuite/gas/riscv/mapping-08b.d         | 23 +++++++
 gas/testsuite/gas/riscv/mapping-norelax-03a.d | 10 +--
 gas/testsuite/gas/riscv/mapping-norelax-04a.d |  4 +-
 gas/testsuite/gas/riscv/option-arch-01a.d     |  2 +-
 opcodes/riscv-dis.c                           | 44 ++++++++++--
 22 files changed, 317 insertions(+), 37 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/mapping-05.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-05a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-05b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-06a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-07a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-08a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08b.d


base-commit: 453595283c323e106a60b229999756b45ae6b2d8
-- 
2.34.1