diff mbox series

[04/10] target/tricore: Implement FTOU insn

Message ID 20230826160242.312052-5-kbastian@mail.uni-paderborn.de
State New
Headers show
Series TriCore 1.6.2 insn and bugfixes | expand

Commit Message

Bastian Koppelmann Aug. 26, 2023, 4:02 p.m. UTC 
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/tricore/fpu_helper.c               | 25 +++++++++++++++++++++++
 target/tricore/helper.h                   |  1 +
 target/tricore/translate.c                |  3 +++
 tests/tcg/tricore/Makefile.softmmu-target |  1 +
 tests/tcg/tricore/asm/test_ftou.S         | 12 +++++++++++
 5 files changed, 42 insertions(+)
 create mode 100644 tests/tcg/tricore/asm/test_ftou.S

Comments

Richard Henderson Aug. 27, 2023, 4:50 a.m. UTC | #1 
On 8/26/23 09:02, Bastian Koppelmann wrote:
> +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
> +{
> +    float32 f_arg = make_float32(arg);
> +    uint32_t result;
> +    int32_t flags = 0;
> +
> +    if (float32_is_any_nan(f_arg)) {
> +        result = 0;
> +        flags |= float_flag_invalid;
> +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
> +        result = 0;
> +        flags = float_flag_invalid;
> +    } else {
> +        result = float32_to_uint32(f_arg, &env->fp_status);
> +        flags = f_get_excp_flags(env);
> +    }

You should allow softfloat to diagnose the special cases, and negative -> 0 is standard 
behaviour.  Therefore:

     result = float32_to_uint32(f_arg, status);
     flags = f_get_excp_flags();

     if (flags) {
         if ((flags & float_flag_invalid)
             && !(get_float_exception_flags() & float_flag_invalid_cvti)) {
             /* invalid without cvti is nan input */
             result = 0;
         }
         f_update_psw_flags(...);
     } else {
         fs = 0;
     }


r~
Bastian Koppelmann Aug. 27, 2023, 11:07 a.m. UTC | #2 
On Sat, Aug 26, 2023 at 09:50:51PM -0700, Richard Henderson wrote:
> On 8/26/23 09:02, Bastian Koppelmann wrote:
> > +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
> > +{
> > +    float32 f_arg = make_float32(arg);
> > +    uint32_t result;
> > +    int32_t flags = 0;
> > +
> > +    if (float32_is_any_nan(f_arg)) {
> > +        result = 0;
> > +        flags |= float_flag_invalid;
> > +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
> > +        result = 0;
> > +        flags = float_flag_invalid;
> > +    } else {
> > +        result = float32_to_uint32(f_arg, &env->fp_status);
> > +        flags = f_get_excp_flags(env);
> > +    }
> 
> You should allow softfloat to diagnose the special cases, and negative -> 0
> is standard behaviour.  Therefore:

You're right. However, there is one special case, negative -> 0 ought to raise
float_flags_invalid. All that has already been done for ftouz, so I will match
that implementation.

Cheers,
Bastian
Richard Henderson Aug. 27, 2023, 2:49 p.m. UTC | #3 
On 8/27/23 04:07, Bastian Koppelmann wrote:
> On Sat, Aug 26, 2023 at 09:50:51PM -0700, Richard Henderson wrote:
>> On 8/26/23 09:02, Bastian Koppelmann wrote:
>>> +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
>>> +{
>>> +    float32 f_arg = make_float32(arg);
>>> +    uint32_t result;
>>> +    int32_t flags = 0;
>>> +
>>> +    if (float32_is_any_nan(f_arg)) {
>>> +        result = 0;
>>> +        flags |= float_flag_invalid;
>>> +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
>>> +        result = 0;
>>> +        flags = float_flag_invalid;
>>> +    } else {
>>> +        result = float32_to_uint32(f_arg, &env->fp_status);
>>> +        flags = f_get_excp_flags(env);
>>> +    }
>>
>> You should allow softfloat to diagnose the special cases, and negative -> 0
>> is standard behaviour.  Therefore:
> 
> You're right. However, there is one special case, negative -> 0 ought to raise
> float_flags_invalid.

https://gitlab.com/qemu-project/qemu/-/blob/master/fpu/softfloat-parts.c.inc?ref_type=heads#L1162

Already done.  As I say, this is all standard IEEE behaviour.


r~
Bastian Koppelmann Aug. 27, 2023, 4:36 p.m. UTC | #4 
On Sun, Aug 27, 2023 at 07:49:52AM -0700, Richard Henderson wrote:
> On 8/27/23 04:07, Bastian Koppelmann wrote:
> > On Sat, Aug 26, 2023 at 09:50:51PM -0700, Richard Henderson wrote:
> > > On 8/26/23 09:02, Bastian Koppelmann wrote:
> > > > +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
> > > > +{
> > > > +    float32 f_arg = make_float32(arg);
> > > > +    uint32_t result;
> > > > +    int32_t flags = 0;
> > > > +
> > > > +    if (float32_is_any_nan(f_arg)) {
> > > > +        result = 0;
> > > > +        flags |= float_flag_invalid;
> > > > +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
> > > > +        result = 0;
> > > > +        flags = float_flag_invalid;
> > > > +    } else {
> > > > +        result = float32_to_uint32(f_arg, &env->fp_status);
> > > > +        flags = f_get_excp_flags(env);
> > > > +    }
> > > 
> > > You should allow softfloat to diagnose the special cases, and negative -> 0
> > > is standard behaviour.  Therefore:
> > 
> > You're right. However, there is one special case, negative -> 0 ought to raise
> > float_flags_invalid.
> 
> https://gitlab.com/qemu-project/qemu/-/blob/master/fpu/softfloat-parts.c.inc?ref_type=heads#L1162

Lets say the exponent is negative and the sign is negative, then we raise
float_flag_inexact and we never reach the code you mentioned here. However,
TriCore HW raises float_flag_invalid as well in that case. This is what I'm
catching with float32_lt_quiet() in the same manner as ftouz.

Cheers,
Bastian
Richard Henderson Aug. 27, 2023, 6:32 p.m. UTC | #5 
On 8/27/23 09:36, Bastian Koppelmann wrote:
> On Sun, Aug 27, 2023 at 07:49:52AM -0700, Richard Henderson wrote:
>> On 8/27/23 04:07, Bastian Koppelmann wrote:
>>> On Sat, Aug 26, 2023 at 09:50:51PM -0700, Richard Henderson wrote:
>>>> On 8/26/23 09:02, Bastian Koppelmann wrote:
>>>>> +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
>>>>> +{
>>>>> +    float32 f_arg = make_float32(arg);
>>>>> +    uint32_t result;
>>>>> +    int32_t flags = 0;
>>>>> +
>>>>> +    if (float32_is_any_nan(f_arg)) {
>>>>> +        result = 0;
>>>>> +        flags |= float_flag_invalid;
>>>>> +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
>>>>> +        result = 0;
>>>>> +        flags = float_flag_invalid;
>>>>> +    } else {
>>>>> +        result = float32_to_uint32(f_arg, &env->fp_status);
>>>>> +        flags = f_get_excp_flags(env);
>>>>> +    }
>>>>
>>>> You should allow softfloat to diagnose the special cases, and negative -> 0
>>>> is standard behaviour.  Therefore:
>>>
>>> You're right. However, there is one special case, negative -> 0 ought to raise
>>> float_flags_invalid.
>>
>> https://gitlab.com/qemu-project/qemu/-/blob/master/fpu/softfloat-parts.c.inc?ref_type=heads#L1162
> 
> Lets say the exponent is negative and the sign is negative, then we raise
> float_flag_inexact and we never reach the code you mentioned here. However,
> TriCore HW raises float_flag_invalid as well in that case. This is what I'm
> catching with float32_lt_quiet() in the same manner as ftouz.

Hmph.  Buggy hardware.  You'd better document this special case,
that less-than-zero is detected before rounding.

I presume -0.0 does not raise invalid, that the bug does not extend that far?


r~
Bastian Koppelmann Aug. 27, 2023, 6:59 p.m. UTC | #6 
On Sun, Aug 27, 2023 at 11:32:03AM -0700, Richard Henderson wrote:
> On 8/27/23 09:36, Bastian Koppelmann wrote:
> > On Sun, Aug 27, 2023 at 07:49:52AM -0700, Richard Henderson wrote:
> > > On 8/27/23 04:07, Bastian Koppelmann wrote:
> > > > On Sat, Aug 26, 2023 at 09:50:51PM -0700, Richard Henderson wrote:
> > > > > On 8/26/23 09:02, Bastian Koppelmann wrote:
> > > > > > +uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
> > > > > > +{
> > > > > > +    float32 f_arg = make_float32(arg);
> > > > > > +    uint32_t result;
> > > > > > +    int32_t flags = 0;
> > > > > > +
> > > > > > +    if (float32_is_any_nan(f_arg)) {
> > > > > > +        result = 0;
> > > > > > +        flags |= float_flag_invalid;
> > > > > > +    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
> > > > > > +        result = 0;
> > > > > > +        flags = float_flag_invalid;
> > > > > > +    } else {
> > > > > > +        result = float32_to_uint32(f_arg, &env->fp_status);
> > > > > > +        flags = f_get_excp_flags(env);
> > > > > > +    }
> > > > > 
> > > > > You should allow softfloat to diagnose the special cases, and negative -> 0
> > > > > is standard behaviour.  Therefore:
> > > > 
> > > > You're right. However, there is one special case, negative -> 0 ought to raise
> > > > float_flags_invalid.
> > > 
> > > https://gitlab.com/qemu-project/qemu/-/blob/master/fpu/softfloat-parts.c.inc?ref_type=heads#L1162
> > 
> > Lets say the exponent is negative and the sign is negative, then we raise
> > float_flag_inexact and we never reach the code you mentioned here. However,
> > TriCore HW raises float_flag_invalid as well in that case. This is what I'm
> > catching with float32_lt_quiet() in the same manner as ftouz.
> 
> Hmph.  Buggy hardware.  You'd better document this special case,
> that less-than-zero is detected before rounding.

Will do. I'll do the same for ftouz.

> 
> I presume -0.0 does not raise invalid, that the bug does not extend that far?

Yes, -0.0 does not raise invalid.

Cheers,
Bastian
diff mbox series

Patch

diff --git a/target/tricore/fpu_helper.c b/target/tricore/fpu_helper.c
index cb7ee7dd35..ceacb6657e 100644
--- a/target/tricore/fpu_helper.c
+++ b/target/tricore/fpu_helper.c
@@ -429,6 +429,31 @@  uint32_t helper_ftoiz(CPUTriCoreState *env, uint32_t arg)
     return result;
 }
 
+uint32_t helper_ftou(CPUTriCoreState *env, uint32_t arg)
+{
+    float32 f_arg = make_float32(arg);
+    uint32_t result;
+    int32_t flags = 0;
+
+    if (float32_is_any_nan(f_arg)) {
+        result = 0;
+        flags |= float_flag_invalid;
+    } else if (float32_lt_quiet(f_arg, 0, &env->fp_status)) {
+        result = 0;
+        flags = float_flag_invalid;
+    } else {
+        result = float32_to_uint32(f_arg, &env->fp_status);
+        flags = f_get_excp_flags(env);
+    }
+
+    if (flags) {
+        f_update_psw_flags(env, flags);
+    } else {
+        env->FPU_FS = 0;
+    }
+    return result;
+}
+
 uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
 {
     float32 f_arg = make_float32(arg);
diff --git a/target/tricore/helper.h b/target/tricore/helper.h
index 190645413a..827fbaa692 100644
--- a/target/tricore/helper.h
+++ b/target/tricore/helper.h
@@ -114,6 +114,7 @@  DEF_HELPER_2(ftoi, i32, env, i32)
 DEF_HELPER_2(itof, i32, env, i32)
 DEF_HELPER_2(utof, i32, env, i32)
 DEF_HELPER_2(ftoiz, i32, env, i32)
+DEF_HELPER_2(ftou, i32, env, i32)
 DEF_HELPER_2(ftouz, i32, env, i32)
 DEF_HELPER_2(updfl, void, env, i32)
 /* dvinit */
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index bb7dad75d6..fb9a7113a8 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -6273,6 +6273,9 @@  static void decode_rr_divide(DisasContext *ctx)
     case OPC2_32_RR_ITOF:
         gen_helper_itof(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
         break;
+    case OPC2_32_RR_FTOU:
+        gen_helper_ftou(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
+        break;
     case OPC2_32_RR_FTOUZ:
         gen_helper_ftouz(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
         break;
diff --git a/tests/tcg/tricore/Makefile.softmmu-target b/tests/tcg/tricore/Makefile.softmmu-target
index 7a7d73a60c..e6ed5c56f2 100644
--- a/tests/tcg/tricore/Makefile.softmmu-target
+++ b/tests/tcg/tricore/Makefile.softmmu-target
@@ -15,6 +15,7 @@  TESTS += test_dvstep.asm.tst
 TESTS += test_fadd.asm.tst
 TESTS += test_fmul.asm.tst
 TESTS += test_ftoi.asm.tst
+TESTS += test_ftou.asm.tst
 TESTS += test_imask.asm.tst
 TESTS += test_insert.asm.tst
 TESTS += test_ld_bu.asm.tst
diff --git a/tests/tcg/tricore/asm/test_ftou.S b/tests/tcg/tricore/asm/test_ftou.S
new file mode 100644
index 0000000000..10f106ad62
--- /dev/null
+++ b/tests/tcg/tricore/asm/test_ftou.S
@@ -0,0 +1,12 @@ 
+#include "macros.h"
+.text
+.global _start
+_start:
+    TEST_D_D(ftou, 1, 0x00000000, 0x1733f6c2)
+    TEST_D_D(ftou, 2, 0x00000000, 0x2c9d9cdc)
+    TEST_D_D(ftou, 3, 0xffffffff, 0x56eb7395)
+    TEST_D_D(ftou, 4, 0x79900800, 0x4ef32010)
+    TEST_D_D(ftou, 5, 0x0353f510, 0x4c54fd44)
+
+    TEST_PASSFAIL
+