Re: [PATCH v3 039/114] target/riscv: preserve signed XLEN semantics without target_long

[Philippe Mathieu-Daudé]

(reducing huge Cc list a bit)

On 23/9/26 15:15, Yonggang Luo wrote:

- Mask pointer-masked addresses with extract64/sextract64 at
   runtime XLEN instead of a target_long arithmetic shift.
- Pass vector vx scalars as int64_t after sextract64 from the
   helper tl slot so RV32 keeps bit 31 as the sign bit.
- Leave every target_ulong helper argument unchanged.

I'm not a RISCV expert but have the feeling some target_long_bits()
should be riscv_cpu_mxl_bits() instead (or something else), as it
depends in which state the CPU is running.

Helpers having @env passed as argument could infer locally.

Signed-off-by: Yonggang Luo <luoyongg...@gmail.com>
---
  target/riscv/internals.h            | 12 ++++++++----
  target/riscv/tcg/vector_helper.c    | 21 +++++++++++----------
  target/riscv/tcg/vector_internals.c |  2 +-
  target/riscv/tcg/vector_internals.h |  9 +++++----
  4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 1ee7e99e988..0844867246d 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -214,6 +214,7 @@ static inline target_ulong adjust_addr_body(CPURISCVState 
*env,
  {
      RISCVPmPmm pmm = PMM_FIELD_DISABLED;
      uint32_t pmlen = 0;
+    unsigned kept;
      bool signext = false;

/* do nothing for rv32 mode */

@@ -235,13 +236,16 @@ static inline target_ulong adjust_addr_body(CPURISCVState 
*env,

signext = riscv_cpu_virt_mem_enabled(env, is_virt_addr);

      pmlen = riscv_pm_get_pmlen(pmm);
-    addr = addr << pmlen;

- /* sign/zero extend masked address by N-1 bit */

+    /*
+     * Drop the top pmlen bits of XLEN. Do not rely on target_ulong
+     * shift wrap; common-system target_ulong is always 64 bits.
+     */
+    kept = target_long_bits() - pmlen;
      if (signext) {
-        addr = (target_long)addr >> pmlen;
+        addr = sextract64(addr, 0, kept);
      } else {
-        addr = addr >> pmlen;
+        addr = extract64(addr, 0, kept);
      }

return addr;

diff --git a/target/riscv/tcg/vector_helper.c b/target/riscv/tcg/vector_helper.c
index 0d1e4e09646..0b78a733ce4 100644
--- a/target/riscv/tcg/vector_helper.c
+++ b/target/riscv/tcg/vector_helper.c
@@ -1248,7 +1248,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2,        \
          ETYPE s2 = *((ETYPE *)vs2 + H(i));                               \
          ETYPE carry = vext_elem_mask(v0, i);                             \
                                                                           \
-        *((ETYPE *)vd + H(i)) = DO_OP(s2, (ETYPE)(target_long)s1, carry);\
+        *((ETYPE *)vd + H(i)) = DO_OP(s2, (ETYPE)sextract64(s1, 0, 
target_long_bits()), carry);\
      }                                                                    \
      env->vstart = 0;                                                     \
      /* set tail elements to 1s */                                        \
@@ -1325,7 +1325,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,    
      \
          ETYPE s2 = *((ETYPE *)vs2 + H(i));                      \
          ETYPE carry = !vm && vext_elem_mask(v0, i);             \
          vext_set_elem_mask(vd, i,                               \
-                DO_OP(s2, (ETYPE)(target_long)s1, carry));      \
+                DO_OP(s2, (ETYPE)sextract64(s1, 0, target_long_bits()), 
carry));      \
      }                                                           \
      env->vstart = 0;                                            \
      /*
@@ -1609,7 +1609,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2,   \
              continue;                                               \
          }                                                           \
          vext_set_elem_mask(vd, i,                                   \
-                DO_OP(s2, (ETYPE)(target_long)s1));                 \
+                DO_OP(s2, (ETYPE)sextract64(s1, 0, target_long_bits())));      
           \
      }                                                               \
      env->vstart = 0;                                                \
      /*
@@ -2050,7 +2050,7 @@ GEN_VEXT_VV(vnmsub_vv_w, 4)
  GEN_VEXT_VV(vnmsub_vv_d, 8)

#define OPIVX3(NAME, TD, T1, T2, TX1, TX2, HD, HS2, OP) \

-static void do_##NAME(void *vd, target_long s1, void *vs2, int i)   \
+static void do_##NAME(void *vd, int64_t s1, void *vs2, int i)   \
  {                                                                   \
      TX2 s2 = *((T2 *)vs2 + HS2(i));                                 \
      TD d = *((TD *)vd + HD(i));                                     \
@@ -2246,7 +2246,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,    
           \
      for (i = env->vstart; i < vl; i++) {                             \
          ETYPE s2 = *((ETYPE *)vs2 + H(i));                           \
          ETYPE d = (!vext_elem_mask(v0, i) ? s2 :                     \
-                   (ETYPE)(target_long)s1);                          \
+                   (ETYPE)sextract64(s1, 0, target_long_bits()));              
            \
          *((ETYPE *)vd + H(i)) = d;                                   \
      }                                                                \
      env->vstart = 0;                                                 \
@@ -2397,12 +2397,12 @@ GEN_VEXT_VV_RM(vsaddu_vv_h, 2)
  GEN_VEXT_VV_RM(vsaddu_vv_w, 4)
  GEN_VEXT_VV_RM(vsaddu_vv_d, 8)

-typedef void opivx2_rm_fn(void *vd, target_long s1, void *vs2, int i,

+typedef void opivx2_rm_fn(void *vd, int64_t s1, void *vs2, int i,
                            CPURISCVState *env, uint8_t vxrm);

#define OPIVX2_RM(NAME, TD, T1, T2, TX1, TX2, HD, HS2, OP) \

  static inline void                                                  \
-do_##NAME(void *vd, target_long s1, void *vs2, int i,               \
+do_##NAME(void *vd, int64_t s1, void *vs2, int i,               \
            CPURISCVState *env, uint8_t vxrm)                         \
  {                                                                   \
      TX2 s2 = *((T2 *)vs2 + HS2(i));                                 \
@@ -2410,7 +2410,7 @@ do_##NAME(void *vd, target_long s1, void *vs2, int i,     
          \
  }

static inline void

-vext_vx_rm_1(void *vd, void *v0, target_long s1, void *vs2,
+vext_vx_rm_1(void *vd, void *v0, int64_t s1, void *vs2,
               CPURISCVState *env,
               uint32_t vl, uint32_t vm, uint8_t vxrm,
               opivx2_rm_fn *fn, uint32_t vma, uint32_t esz)
@@ -2427,7 +2427,7 @@ vext_vx_rm_1(void *vd, void *v0, target_long s1, void 
*vs2,
  }

static inline void

-vext_vx_rm_2(void *vd, void *v0, target_long s1, void *vs2,
+vext_vx_rm_2(void *vd, void *v0, int64_t s1, void *vs2,
               CPURISCVState *env,
               uint32_t desc,
               opivx2_rm_fn *fn, uint32_t esz)
@@ -2468,7 +2468,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,    
\
                    void *vs2, CPURISCVState *env,          \
                    uint32_t desc)                          \
  {                                                         \
-    vext_vx_rm_2(vd, v0, s1, vs2, env, desc,              \
+    vext_vx_rm_2(vd, v0, sextract64(s1, 0, target_long_bits()), \
+                 vs2, env, desc,                          \
                   do_##NAME, ESZ);                         \
  }

diff --git a/target/riscv/tcg/vector_internals.c b/target/riscv/tcg/vector_internals.c

index b490b1d3989..95327052666 100644
--- a/target/riscv/tcg/vector_internals.c
+++ b/target/riscv/tcg/vector_internals.c
@@ -81,7 +81,7 @@ void do_vext_vv(void *vd, void *v0, void *vs1, void *vs2,
      vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
  }

-void do_vext_vx(void *vd, void *v0, target_long s1, void *vs2,

+void do_vext_vx(void *vd, void *v0, int64_t s1, void *vs2,
                  CPURISCVState *env, uint32_t desc,
                  opivx2_fn fn, uint32_t esz)
  {
diff --git a/target/riscv/tcg/vector_internals.h 
b/target/riscv/tcg/vector_internals.h
index 5681b818153..da093d561eb 100644
--- a/target/riscv/tcg/vector_internals.h
+++ b/target/riscv/tcg/vector_internals.h
@@ -201,20 +201,20 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,          
\
                 do_##NAME, ESZ);                           \
  }

-typedef void opivx2_fn(void *vd, target_long s1, void *vs2, int i);

+typedef void opivx2_fn(void *vd, int64_t s1, void *vs2, int i);

/*

   * (T1)s1 gives the real operator type.
   * (TX1)(T1)s1 expands the operator type of widen or narrow operations.
   */
  #define OPIVX2(NAME, TD, T1, T2, TX1, TX2, HD, HS2, OP)             \
-static void do_##NAME(void *vd, target_long s1, void *vs2, int i)   \
+static void do_##NAME(void *vd, int64_t s1, void *vs2, int i)       \
  {                                                                   \
      TX2 s2 = *((T2 *)vs2 + HS2(i));                                 \
      *((TD *)vd + HD(i)) = OP(s2, (TX1)(T1)s1);                      \
  }

-void do_vext_vx(void *vd, void *v0, target_long s1, void *vs2,

+void do_vext_vx(void *vd, void *v0, int64_t s1, void *vs2,
                  CPURISCVState *env, uint32_t desc,
                  opivx2_fn fn, uint32_t esz);

@@ -224,7 +224,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, \

                    void *vs2, CPURISCVState *env,          \
                    uint32_t desc)                          \
  {                                                         \
-    do_vext_vx(vd, v0, s1, vs2, env, desc,                \
+    do_vext_vx(vd, v0, sextract64(s1, 0, target_long_bits()), \
+               vs2, env, desc,                            \
                 do_##NAME, ESZ);                           \
  }