Re: [PATCH] Support N32(32-bit ABI on 64-bit ISA) in riscv

[Liao Shihua <shihua@iscas.ac.cn>]

[-- Attachment #1: Type: text/plain, Size: 18147 bytes --]

Hello, Nelson

Thanks for your advice, I will try to do this.

Liao Shihua

在 2024/11/18 10:40, Nelson Chu 写道:
> There are some comments as follows.  Not quite sure what to do with 
> experimental features in the toolchain, we used to have assembler/link 
> options or configuration options to make it different from regular 
> users.  Besides, it is probably easier to remove the features if they 
> are experimental and controlled by experimental options when they 
> cause problems or are not useful as expected.  So if N32 is, then 
> ideally we should have some experimental options.  For example, if 
> people still want the link errors when linking rv32 and rv64, then 
> they just don't enable the experimental options.
>
> Thanks
> Nelson
>
> On Sat, Nov 16, 2024 at 11:16 AM Liao Shihua <shihua@iscas.ac.cn> wrote:
>
>     RISC-V N32 ABI means using 32-bit ABI on 64-bit ISA, the discussion in
>     https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/381 .
>     At this moment, N32 is supported batemental toolchain.
>     Three OpenSource  RTOS using this feature and have been merged in
>     upstream.
>     You can see them in
>     EasyXem (AUTOSAR CP R19-11):
>     https://atomgit.com/easyxmen/XMen/tree/rv64ilp32-dev
>     Nuttx: https://github.com/apache/nuttx
>     RT-Thread:https://github.com/RT-Thread/rt-thread/pull/9194
>
>     This patch add a new bfd_mach bfd_mach_riscv64n32 and a new
>     e_flags N32.
>     bfd_mach_riscv64n32 has the same bits in a word/address and
>     ARCH_SIZE with rv32,
>     but use rv64's PRSTATUS. N32 use the 6th bit of e_flags layout.
>     In addition, this patch replace xlen with abi_xlen in
>     riscv_target_format().
>
>     bfd/ChangeLog:
>
>             * archures.c: Add bfd_mach_riscv64n32.
>             * bfd-in2.h (bfd_mach_riscv64n32): Ditto.
>             * cpu-riscv.c: Ditto.
>             * elfnn-riscv.c (ABI_N32_P):Add ABI_N32_P
>             (perform_relocation):Ditto.
>             (riscv_merge_arch_attr_info):Remove elf check when using N32.
>             (_bfd_riscv_elf_merge_private_bfd_data):Ditto.
>             (_bfd_riscv_relax_section):Ditto.
>             (riscv_elf_object_p):Using bfd_mach_riscv64n32 with N32.
>
>     binutils/ChangeLog:
>
>             * readelf.c (decode_RISCV_machine_flags):Add N32 Flag.
>
>     gas/ChangeLog:
>
>             * config/tc-riscv.c (riscv_set_n32):Set N32 flag.
>             (riscv_set_abi_by_arch):Removed check.
>             (riscv_target_format):Ditto
>             (md_begin):Ditto
>             (s_riscv_attribute):Ditto
>             * testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.d: Removed.
>             * testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.l: Removed.
>
>     include/ChangeLog:
>
>             * elf/riscv.h (EF_RISCV_N32):Add N32 Flag.
>
>     opcodes/ChangeLog:
>
>             * riscv-dis.c (print_insn_args):
>             (riscv_disassemble_insn):
>
>     ---
>      bfd/archures.c                                |  1 +
>      bfd/bfd-in2.h                                 |  1 +
>      bfd/cpu-riscv.c                               |  2 ++
>      bfd/elfnn-riscv.c                             | 32
>     +++++++++++++------
>      binutils/readelf.c                            |  3 ++
>      gas/config/tc-riscv.c                         | 23 ++++++++++---
>      .../gas/riscv/mabi-fail-rv64iq-ilp32.d        |  3 --
>      .../gas/riscv/mabi-fail-rv64iq-ilp32.l        |  2 --
>      include/elf/riscv.h                           |  3 ++
>      opcodes/riscv-dis.c                           |  6 ++--
>      10 files changed, 53 insertions(+), 23 deletions(-)
>      delete mode 100644 gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.d
>      delete mode 100644 gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.l
>
>     diff --git a/bfd/archures.c b/bfd/archures.c
>     index c4decc59e4a..063659631fa 100644
>     --- a/bfd/archures.c
>     +++ b/bfd/archures.c
>     @@ -448,6 +448,7 @@ DESCRIPTION
>      .  bfd_arch_riscv,
>      .#define bfd_mach_riscv32      132
>      .#define bfd_mach_riscv64      164
>     +.#define bfd_mach_riscv64n32   16432
>      .  bfd_arch_rl78,
>      .#define bfd_mach_rl78         0x75
>      .  bfd_arch_rx,               {* Renesas RX.  *}
>     diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
>     index 3b047d922f3..0ee6bbc2a73 100644
>     --- a/bfd/bfd-in2.h
>     +++ b/bfd/bfd-in2.h
>     @@ -1714,6 +1714,7 @@ enum bfd_architecture
>        bfd_arch_riscv,
>      #define bfd_mach_riscv32       132
>      #define bfd_mach_riscv64       164
>     +#define bfd_mach_riscv64n32    16432
>        bfd_arch_rl78,
>      #define bfd_mach_rl78          0x75
>        bfd_arch_rx,        /* Renesas RX.  */
>     diff --git a/bfd/cpu-riscv.c b/bfd/cpu-riscv.c
>     index 58cbdd846be..1df99dc6e2d 100644
>     --- a/bfd/cpu-riscv.c
>     +++ b/bfd/cpu-riscv.c
>     @@ -86,6 +86,7 @@ riscv_scan (const struct bfd_arch_info *info,
>     const char *string)
>      enum
>      {
>        I_riscv64,
>     +  I_riscv64n32,
>        I_riscv32
>      };
>
>     @@ -96,6 +97,7 @@ enum
>      static const bfd_arch_info_type arch_info_struct[] =
>      {
>        N (64, bfd_mach_riscv64, "riscv:rv64", false, NN (I_riscv64)),
>     +  N (32, bfd_mach_riscv64n32, "riscv:rv64", false, NN
>     (I_riscv64n32)),
>        N (32, bfd_mach_riscv32, "riscv:rv32", false, NULL)
>      };
>
>     diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
>     index c8bf45f4293..b05f4fde433 100644
>     --- a/bfd/elfnn-riscv.c
>     +++ b/bfd/elfnn-riscv.c
>     @@ -138,6 +138,11 @@
>
>      #define RISCV_ELF_WORD_BYTES (1 << RISCV_ELF_LOG_WORD_BYTES)
>
>     +#define ABI_N32_P(abfd) \
>     +  ((elf_elfheader (abfd)->e_flags & EF_RISCV_N32) != 0)
>     +
>     +static bool ABI_N32 = false;
>     +
>      /* The name of the dynamic interpreter.  This is put in the .interp
>         section.  */
>
>     @@ -1823,7 +1828,8 @@ perform_relocation (const reloc_howto_type
>     *howto,
>
>          case R_RISCV_CALL:
>          case R_RISCV_CALL_PLT:
>     -      if (ARCH_SIZE > 32 && !VALID_UTYPE_IMM
>     (RISCV_CONST_HIGH_PART (value)))
>     +      if ((ARCH_SIZE > 32 || ABI_N32_P (input_bfd))
>     +         && !VALID_UTYPE_IMM (RISCV_CONST_HIGH_PART (value)))
>             return bfd_reloc_overflow;
>            value = ENCODE_UTYPE_IMM (RISCV_CONST_HIGH_PART (value))
>                   | (ENCODE_ITYPE_IMM (value) << 32);
>     @@ -3927,7 +3933,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>     *in_arch, char *out_arch)
>          return NULL;
>
>        /* Checking XLEN.  */
>     -  if (xlen_out != xlen_in)
>     +  if (xlen_out != xlen_in && !ABI_N32_P (ibfd))
>          {
>            _bfd_error_handler
>             (_("error: %pB: ISA string of input (%s) doesn't match "
>     @@ -3947,7 +3953,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>     *in_arch, char *out_arch)
>        if (!riscv_merge_multi_letter_ext (&in, &out))
>          return NULL;
>
>     -  if (xlen_in != xlen_out)
>     +  if (xlen_in != xlen_out && !ABI_N32_P (ibfd))
>          {
>            _bfd_error_handler
>             (_("error: %pB: XLEN of input (%u) doesn't match "
>     @@ -3955,7 +3961,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>     *in_arch, char *out_arch)
>            return NULL;
>          }
>
>     -  if (xlen_in != ARCH_SIZE)
>     +  if (xlen_in != ARCH_SIZE && !ABI_N32_P (ibfd))
>          {
>            _bfd_error_handler
>             (_("error: %pB: unsupported XLEN (%u), you might be "
>     @@ -3963,7 +3969,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>     *in_arch, char *out_arch)
>            return NULL;
>          }
>
>     -  merged_arch_str = riscv_arch_str (ARCH_SIZE, &merged_subsets);
>     +  merged_arch_str = riscv_arch_str (xlen_in, &merged_subsets);
>
>
> The xlen of n32 seems to be rv64.  If we link n32 with normal rv64 
> objects, it looks fine since xlen will always be rv64.  But how about 
> linking n32 with normal rv32 objects? The merged xlen will be changed 
> to the latest input xlen, so it might be rv32 or rv64 when we link n32 
> with other rv32 objects, it looks weird but is it expected?  This 
> might also have a merged output with rv32 + q, which looks like a 
> conflict but there won't be any alerts since we used not to call 
> riscv_parse_check_conflicts by linker.
>
>        /* Release the subset lists.  */
>        riscv_release_subset_list (&in_subsets);
>     @@ -4233,6 +4239,9 @@ _bfd_riscv_elf_merge_private_bfd_data (bfd
>     *ibfd, struct bfd_link_info *info)
>
>        /* Allow linking TSO and non-TSO, and keep the TSO flag. */
>        elf_elfheader (obfd)->e_flags |= new_flags & EF_RISCV_TSO;
>     +
>     +  /* Allow linking N32 and non-N32, and keep the N32 flag. */
>     +  elf_elfheader (obfd)->e_flags |= new_flags & EF_RISCV_N32;
>
>        return true;
>
>     @@ -5400,7 +5409,7 @@ _bfd_riscv_relax_section (bfd *abfd,
>     asection *sec,
>        return ret;
>      }
>
>     -#if ARCH_SIZE == 32
>     +#if ARCH_SIZE == 32 && !ABI_N32
>      # define PRSTATUS_SIZE                 204
>      # define PRSTATUS_OFFSET_PR_CURSIG     12
>      # define PRSTATUS_OFFSET_PR_PID                24
>     @@ -5570,10 +5579,13 @@ riscv_elf_grok_psinfo (bfd *abfd,
>     Elf_Internal_Note *note)
>      static bool
>      riscv_elf_object_p (bfd *abfd)
>      {
>     -  /* There are only two mach types in RISCV currently.  */
>     -  if (strcmp (abfd->xvec->name, "elf32-littleriscv") == 0
>     -      || strcmp (abfd->xvec->name, "elf32-bigriscv") == 0)
>     -    bfd_default_set_arch_mach (abfd, bfd_arch_riscv,
>     bfd_mach_riscv32);
>     +  ABI_N32 = ABI_N32_P (abfd);
>     +  /* There are only three mach types in RISCV currently. */
>     +  if (ABI_N32)
>     +    bfd_default_set_arch_mach (abfd, bfd_arch_riscv,
>     bfd_mach_riscv64n32);
>     +  else if (strcmp (abfd->xvec->name, "elf32-littleriscv") == 0
>     +    || strcmp (abfd->xvec->name, "elf32-bigriscv") == 0)
>     +       bfd_default_set_arch_mach (abfd, bfd_arch_riscv,
>     bfd_mach_riscv32);
>        else
>          bfd_default_set_arch_mach (abfd, bfd_arch_riscv,
>     bfd_mach_riscv64);
>
>     diff --git a/binutils/readelf.c b/binutils/readelf.c
>     index 73163e0ee21..58248d84135 100644
>     --- a/binutils/readelf.c
>     +++ b/binutils/readelf.c
>     @@ -4532,6 +4532,9 @@ decode_RISCV_machine_flags (char *out,
>     unsigned e_flags)
>        if (e_flags & EF_RISCV_TSO)
>          out = stpcpy (out, ", TSO");
>
>     +  if (e_flags & EF_RISCV_N32)
>     +    out = stpcpy (out, ", N32");
>     +
>        switch (e_flags & EF_RISCV_FLOAT_ABI)
>          {
>          case EF_RISCV_FLOAT_ABI_SOFT:
>     diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>     index e19142dca19..dde9c52ccc1 100644
>     --- a/gas/config/tc-riscv.c
>     +++ b/gas/config/tc-riscv.c
>     @@ -306,6 +306,13 @@ riscv_set_tso (void)
>        elf_flags |= EF_RISCV_TSO;
>      }
>
>     +/* Turn on the n32 flag for elf_flags once we have enabled n32
>     model.  */
>     +static void
>     +riscv_set_n32 (void)
>     +{
>     +  elf_flags |= EF_RISCV_N32;
>     +}
>     +
>      /* The linked list hanging off of .subsets_list records all
>     enabled extensions,
>         which are parsed from the architecture string.  The
>     architecture string can
>         be set by the -march option, the elf architecture attributes,
>     and the
>     @@ -409,7 +416,7 @@ riscv_set_abi_by_arch (void)
>            gas_assert (abi_xlen != 0 && xlen != 0 && float_abi !=
>     FLOAT_ABI_DEFAULT);
>            if (abi_xlen > xlen)
>             as_bad ("can't have %d-bit ABI on %d-bit ISA", abi_xlen,
>     xlen);
>     -      else if (abi_xlen < xlen)
>     +      else if (abi_xlen < xlen && (abi_xlen != 32 && xlen != 64))
>             as_bad ("%d-bit ABI not yet supported on %d-bit ISA",
>     abi_xlen, xlen);
>
>            if (riscv_subset_supports (&riscv_rps_as, "e") && !rve_abi)
>     @@ -435,6 +442,10 @@ riscv_set_abi_by_arch (void)
>
>        if (rve_abi)
>          elf_flags |= EF_RISCV_RVE;
>     +
>     +  if (abi_xlen == 32 && xlen == 64)
>     +    riscv_set_n32 ();
>     +
>      }
>
>      /* Handle of the OPCODE hash table.  */
>     @@ -734,9 +745,9 @@ const char *
>      riscv_target_format (void)
>      {
>        if (target_big_endian)
>     -    return xlen == 64 ? "elf64-bigriscv" : "elf32-bigriscv";
>     +    return abi_xlen == 64 ? "elf64-bigriscv" : "elf32-bigriscv";
>        else
>     -    return xlen == 64 ? "elf64-littleriscv" : "elf32-littleriscv";
>     +    return abi_xlen == 64 ? "elf64-littleriscv" :
>     "elf32-littleriscv";
>      }
>
>      /* Return the length of instruction INSN.  */
>     @@ -1876,7 +1887,8 @@ riscv_record_pcrel_fixup (htab_t p, const
>     asection *sec, bfd_vma address,
>      void
>      md_begin (void)
>      {
>     -  unsigned long mach = xlen == 64 ? bfd_mach_riscv64 :
>     bfd_mach_riscv32;
>     +  unsigned long mach = xlen == 64 ?
>     +      (abi_xlen == 32 ? bfd_mach_riscv64n32 : bfd_mach_riscv64) :
>     bfd_mach_riscv32;
>
>
> Indent, exceed 80 char for one line.
>
>
>        if (! bfd_set_arch_mach (stdoutput, bfd_arch_riscv, mach))
>          as_warn (_("could not set architecture and machine"));
>     @@ -5674,7 +5686,8 @@ s_riscv_attribute (int ignored ATTRIBUTE_UNUSED)
>            if (old_xlen != xlen)
>             {
>               /* We must re-init bfd again if xlen is changed. */
>     -         unsigned long mach = xlen == 64 ? bfd_mach_riscv64 :
>     bfd_mach_riscv32;
>     +         unsigned long mach = xlen == 64 ?
>     +      (abi_xlen == 32 ? bfd_mach_riscv64n32 : bfd_mach_riscv64) :
>     bfd_mach_riscv32;
>
>
> Indent wrong.
>
>               bfd_find_target (riscv_target_format (), stdoutput);
>
>               if (! bfd_set_arch_mach (stdoutput, bfd_arch_riscv, mach))
>     diff --git a/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.d
>     b/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.d
>     deleted file mode 100644
>     index e3155f48956..00000000000
>     --- a/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.d
>     +++ /dev/null
>     @@ -1,3 +0,0 @@
>     -#as: -march-attr -mabi=ilp32
>     -#source: mabi-attr-rv64iq.s
>     -#error_output: mabi-fail-rv64iq-ilp32.l
>     diff --git a/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.l
>     b/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.l
>     deleted file mode 100644
>     index 8d45a07fd36..00000000000
>     --- a/gas/testsuite/gas/riscv/mabi-fail-rv64iq-ilp32.l
>     +++ /dev/null
>     @@ -1,2 +0,0 @@
>     -.*Assembler messages:
>     -.*Error: 32-bit ABI not yet supported on 64-bit ISA
>     diff --git a/include/elf/riscv.h b/include/elf/riscv.h
>     index 24903c04d91..f88a1c0701a 100644
>     --- a/include/elf/riscv.h
>     +++ b/include/elf/riscv.h
>     @@ -140,6 +140,9 @@ END_RELOC_NUMBERS (R_RISCV_max)
>      /* File uses the TSO model. */
>      #define EF_RISCV_TSO 0x0010
>
>     +/* File uses the N32 model.  */
>     +#define EF_RISCV_N32 0x0020
>     +
>      /* Additional section types.  */
>      #define SHT_RISCV_ATTRIBUTES (SHT_LOPROC + 3) /* Section holds
>     attributes.  */
>
>     diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>     index 80018db837a..09fac0c3cbe 100644
>     --- a/opcodes/riscv-dis.c
>     +++ b/opcodes/riscv-dis.c
>     @@ -349,7 +349,7 @@ print_insn_args (const char *oparg, insn_t l,
>     bfd_vma pc, disassemble_info *info
>                 case 'j':
>                   if (((l & MASK_C_ADDI) == MATCH_C_ADDI) && rd != 0)
>                     maybe_print_address (pd, rd, EXTRACT_CITYPE_IMM
>     (l), 0);
>     -             if (info->mach == bfd_mach_riscv64
>     +             if (info->mach == bfd_mach_riscv64 || info->mach ==
>     bfd_mach_riscv64n32
>
>
> Indent, exceed 80 char for one line., and also looks like missing 
> brackets.
>
>                       && ((l & MASK_C_ADDIW) == MATCH_C_ADDIW) && rd != 0)
>                     maybe_print_address (pd, rd, EXTRACT_CITYPE_IMM
>     (l), 1);
>                   print (info->stream, dis_style_immediate, "%d",
>     @@ -555,7 +555,7 @@ print_insn_args (const char *oparg, insn_t l,
>     bfd_vma pc, disassemble_info *info
>               if (((l & MASK_ADDI) == MATCH_ADDI && rs1 != 0)
>                   || (l & MASK_JALR) == MATCH_JALR)
>                 maybe_print_address (pd, rs1, EXTRACT_ITYPE_IMM (l), 0);
>     -         if (info->mach == bfd_mach_riscv64
>     +         if (info->mach == bfd_mach_riscv64|| info->mach ==
>     bfd_mach_riscv64n32
>
>
> Indent, exceed 80 char for one line, a space before ||, and also looks 
> like missing brackets.
>
>                   && ((l & MASK_ADDIW) == MATCH_ADDIW) && rs1 != 0)
>                 maybe_print_address (pd, rs1, EXTRACT_ITYPE_IMM (l), 1);
>               print (info->stream, dis_style_immediate, "%d",
>     @@ -941,7 +941,7 @@ riscv_disassemble_insn (bfd_vma memaddr,
>        if (op != NULL)
>          {
>            /* If XLEN is not known, get its value from the ELF class.  */
>     -      if (info->mach == bfd_mach_riscv64)
>     +      if (info->mach == bfd_mach_riscv64 || info->mach ==
>     bfd_mach_riscv64n32)
>             xlen = 64;
>            else if (info->mach == bfd_mach_riscv32)
>             xlen = 32;
>     -- 
>     2.34.1
>
>