1. Patchew
  2. QEMU
  3. [PATCH v3 00/15] RISC-V: Start to remove xlen preprocess
  4. View patch

[PATCH v3 09/15] target/riscv: fpu_helper: Match function defs in HELPER macros

Alistair Francis posted 15 patches 5 years ago
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Palmer Dabbelt <palmer@dabbelt.com>
There is a newer version of this series
[PATCH v3 09/15] target/riscv: fpu_helper: Match function defs in HELPER macros
Posted by Alistair Francis 5 years ago 
The helper functions defined in helper.h specify that the argument is of
type target_long. Let's change the implementation to match the header
definition.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/fpu_helper.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index bb346a8249..507d7fe7fa 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -224,13 +224,13 @@ target_ulong helper_fcvt_wu_s(CPURISCVState *env, uint64_t rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
+target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
 {
     float32 frs1 = check_nanbox_s(rs1);
     return float32_to_int64(frs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
+target_ulong helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
 {
     float32 frs1 = check_nanbox_s(rs1);
     return float32_to_uint64(frs1, &env->fp_status);
@@ -248,12 +248,12 @@ uint64_t helper_fcvt_s_wu(CPURISCVState *env, target_ulong rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_s_l(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_s_l(CPURISCVState *env, target_ulong rs1)
 {
     return nanbox_s(int64_to_float32(rs1, &env->fp_status));
 }
 
-uint64_t helper_fcvt_s_lu(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_s_lu(CPURISCVState *env, target_ulong rs1)
 {
     return nanbox_s(uint64_to_float32(rs1, &env->fp_status));
 }
@@ -337,12 +337,12 @@ target_ulong helper_fcvt_wu_d(CPURISCVState *env, uint64_t frs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
+target_ulong helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
 {
     return float64_to_int64(frs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
+target_ulong helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
 {
     return float64_to_uint64(frs1, &env->fp_status);
 }
@@ -359,12 +359,12 @@ uint64_t helper_fcvt_d_wu(CPURISCVState *env, target_ulong rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_d_l(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_d_l(CPURISCVState *env, target_ulong rs1)
 {
     return int64_to_float64(rs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_d_lu(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_d_lu(CPURISCVState *env, target_ulong rs1)
 {
     return uint64_to_float64(rs1, &env->fp_status);
 }
-- 
2.29.2
Re: [PATCH v3 09/15] target/riscv: fpu_helper: Match function defs in HELPER macros
Posted by Bin Meng 5 years ago 
Hi Alistair,

On Tue, Dec 15, 2020 at 4:34 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> The helper functions defined in helper.h specify that the argument is of

I can't find the helper.h that declare these functions?

> type target_long. Let's change the implementation to match the header
> definition.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/fpu_helper.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index bb346a8249..507d7fe7fa 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -224,13 +224,13 @@ target_ulong helper_fcvt_wu_s(CPURISCVState *env, uint64_t rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> +target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      return float32_to_int64(frs1, &env->fp_status);

float32_to_int64() returns int64_t, so there is a truncation if
changing it to target_ulong for 32-bit.

>  }
>
> -uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
> +target_ulong helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      return float32_to_uint64(frs1, &env->fp_status);
> @@ -248,12 +248,12 @@ uint64_t helper_fcvt_s_wu(CPURISCVState *env, target_ulong rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_s_l(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_s_l(CPURISCVState *env, target_ulong rs1)
>  {
>      return nanbox_s(int64_to_float32(rs1, &env->fp_status));
>  }
>
> -uint64_t helper_fcvt_s_lu(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_s_lu(CPURISCVState *env, target_ulong rs1)
>  {
>      return nanbox_s(uint64_to_float32(rs1, &env->fp_status));
>  }
> @@ -337,12 +337,12 @@ target_ulong helper_fcvt_wu_d(CPURISCVState *env, uint64_t frs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
> +target_ulong helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
>  {
>      return float64_to_int64(frs1, &env->fp_status);
>  }
>
> -uint64_t helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
> +target_ulong helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
>  {
>      return float64_to_uint64(frs1, &env->fp_status);
>  }
> @@ -359,12 +359,12 @@ uint64_t helper_fcvt_d_wu(CPURISCVState *env, target_ulong rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_d_l(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_d_l(CPURISCVState *env, target_ulong rs1)
>  {
>      return int64_to_float64(rs1, &env->fp_status);
>  }
>
> -uint64_t helper_fcvt_d_lu(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_d_lu(CPURISCVState *env, target_ulong rs1)
>  {
>      return uint64_to_float64(rs1, &env->fp_status);
>  }

Regards,
Bin
Re: [PATCH v3 09/15] target/riscv: fpu_helper: Match function defs in HELPER macros
Posted by Richard Henderson 5 years ago 
On 12/15/20 3:38 AM, Bin Meng wrote:
>>  #if defined(TARGET_RISCV64)
>> -uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
>> +target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
>>  {
>>      float32 frs1 = check_nanbox_s(rs1);
>>      return float32_to_int64(frs1, &env->fp_status);
> 
> float32_to_int64() returns int64_t, so there is a truncation if
> changing it to target_ulong for 32-bit.

There's not, because this function isn't defined for 32-bit (see first quoted
line).  But this point of confusion is exactly what I pointed out vs the
previous revision.


r~
Re: [PATCH v3 09/15] target/riscv: fpu_helper: Match function defs in HELPER macros
Posted by Alistair Francis 5 years ago 
On Tue, Dec 15, 2020 at 7:13 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 12/15/20 3:38 AM, Bin Meng wrote:
> >>  #if defined(TARGET_RISCV64)
> >> -uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> >> +target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> >>  {
> >>      float32 frs1 = check_nanbox_s(rs1);
> >>      return float32_to_int64(frs1, &env->fp_status);
> >
> > float32_to_int64() returns int64_t, so there is a truncation if
> > changing it to target_ulong for 32-bit.
>
> There's not, because this function isn't defined for 32-bit (see first quoted
> line).  But this point of confusion is exactly what I pointed out vs the
> previous revision.

Ok, I have swapped this to changing helper.h now.

Alistair

>
>
> r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.