| Message ID | 2cffe9cb8055c9451872b3a08192e19fec12fb1a.1607967113.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Start to remove xlen preprocess | expand |
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
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~
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~
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); }
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(-)