Re: [RFC PATCH 00/13] target/riscv: Rationalize XLEN and operand length

[Frédéric Pétrot]

Le 07/10/2021 à 19:47, Richard Henderson a écrit :
> This is a partial patch set attempting to set things in the
> right direction for both the UXL and RV128 patch sets.
> 
> One of the things that struck me while reading the RV128 patches
> is the proliferation of riscv_cpu_is_<size>bits functions.  These
> should be all combined into a single function returning an enum.
> 
> Further, that the current set of tests for misa.mxl is frought
> with peril, because the location of the field within misa varies,
> and that the RV128 patch set gets some of it wrong.  It is much
> easier to split out that field for use within QEMU, and only
> reassemble the full MISA csr upon read.

  Couldn't more agree.
  The convoluted process of determining misa length seems to be here to ensure
  that there is no way for a 32-bit arch to "announce" that it is a 64-bit one.

> There are a few changes at the end for pointing the correct
> direction for how it might be best to extend expanders for
> different operand lengths.  I believe the form used in RV128,
> 
> static bool gen_arith_imm_fn(DisasContext *ctx, arg_i *a, DisasExtend ext,
>                              void (*fn32)(TCGv, TCGv, target_long),
>                              void (*fn64)(TCGv, TCGv, target_long),
>                              void (*fn128)(TCGv, TCGv, TCGv, TCGv, 
> target_long))
> 
> is incorrect, because it assumes that is easy to select the
> fn32 and fn64 functions.  But so long as TARGET_RISCV32 is
> still around, and target_long may be 32, that is not the case.
> Instead, pass f_tl and f_32, where f_32 will only ever be used
> when sizeof(target_ulong) > 4.

  I got the point.
  Frédéric
> 
> r~
> 
> 
> Richard Henderson (13):
>   target/riscv: Move cpu_get_tb_cpu_state out of line
>   target/riscv: Create RISCVMXL enumeration
>   target/riscv: Split misa.mxl and misa.ext
>   target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl
>   target/riscv: Add MXL/SXL/UXL to TB_FLAGS
>   target/riscv: Use REQUIRE_64BIT in amo_check64
>   target/riscv: Properly check SEW in amo_op
>   target/riscv: Replace is_32bit with get_xl/get_xlen
>   target/riscv: Replace DisasContext.w with DisasContext.ol
>   target/riscv: Use gen_arith_per_ol for RVM
>   target/riscv: Adjust trans_rev8_32 for riscv64
>   target/riscv: Use gen_unary_per_ol for RVB
>   target/riscv: Use gen_shift*_per_ol for RVB, RVI
> 
>  target/riscv/cpu.h                      |  73 +++-------
>  target/riscv/cpu_bits.h                 |   8 +-
>  hw/riscv/boot.c                         |   2 +-
>  linux-user/elfload.c                    |   2 +-
>  linux-user/riscv/cpu_loop.c             |   2 +-
>  semihosting/arm-compat-semi.c           |   2 +-
>  target/riscv/cpu.c                      |  99 ++++++++------
>  target/riscv/cpu_helper.c               |  91 ++++++++++++-
>  target/riscv/csr.c                      |  68 ++++++----
>  target/riscv/gdbstub.c                  |  10 +-
>  target/riscv/machine.c                  |  10 +-
>  target/riscv/monitor.c                  |   4 +-
>  target/riscv/translate.c                | 170 ++++++++++++++++++------
>  target/riscv/insn_trans/trans_rvb.c.inc | 140 ++++++++++---------
>  target/riscv/insn_trans/trans_rvi.c.inc |  44 +++---
>  target/riscv/insn_trans/trans_rvm.c.inc |  36 +++--
>  target/riscv/insn_trans/trans_rvv.c.inc |  29 ++--
>  17 files changed, 498 insertions(+), 292 deletions(-)
> 

-- 
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA,   Ensimag deputy director |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70      Ad augusta  per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
+---------------------------------------------------------------------------+