[Qemu-devel] [PATCH v2 0/5]

[Qemu-devel] [PATCH v2 0/5]

[Bastian Koppelmann]

Hi,

this series was originally posted by Peer Adelt some time ago[1], but still had
some problems which I tried to fix with this patch set.

The tricky bits here were the FTOUZ and MADD/MSUB.F instructions. The latter 
had the problem of not giving back the correct NAN when the result of the 
add/sub of muladd/sub was invalid. I addressed that by fixing up the value 
later, which feels hacky. I feel the better solution would be extending 
softfloat to recognize ADD_NAN's and emitting the correct NAN there. On the 
other hand it's a change in softfloat for a small edge case. Any comments on
that?

Additionally this patch set adds the UPDFL instructions.

Cheers,
    Bastian

[1] http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg01936.html

v1 -> v2:
    - ftouz: Correctly convert the result from uint32 to f32

Bastian Koppelmann (3):
  target-tricore: Added FTOUZ instruction
  target-tricore: Added MADD.F and MSUB.F instructions
  target-tricore: Add updfl instruction

Peer Adelt (2):
  target-tricore: Added new MOV instruction variant
  target-tricore: Added new JNE instruction variant

 target-tricore/fpu_helper.c      | 148 ++++++++++++++++++++++++++++++++++++++-
 target-tricore/helper.h          |   4 ++
 target-tricore/translate.c       |  47 +++++++++++++
 target-tricore/tricore-opcodes.h |   3 +
 4 files changed, 201 insertions(+), 1 deletion(-)

-- 
2.10.2

[Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Bastian Koppelmann]

Converts a 32-bit floating point number to an unsigned int. The
result is rounded towards zero.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
v1 -> v2:
    - ftouz: Correctly convert the result from uint32 to f32

 target-tricore/fpu_helper.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 target-tricore/helper.h     |  1 +
 target-tricore/translate.c  |  3 +++
 3 files changed, 47 insertions(+)

diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
index 98fe947..f717b53 100644
--- a/target-tricore/fpu_helper.c
+++ b/target-tricore/fpu_helper.c
@@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env, uint32_t arg)
     }
     return (uint32_t)f_result;
 }
+
+uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
+{
+    float32 f_arg = make_float32(arg);
+    uint32_t result;
+    int32_t flags;
+    result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status);
+
+    flags = f_get_excp_flags(env);
+    if (flags) {
+        if (float32_is_any_nan(f_arg)) {
+            flags |= float_flag_invalid;
+            result = 0;
+        /* f_real(D[a]) < 0.0 */
+        } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) {
+            flags |= float_flag_invalid;
+            result = 0;
+        /* f_real(D[a]) > 2^32 -1  */
+        } else if (float32_lt_quiet(0x4f800000, f_arg, &env->fp_status)) {
+            flags |= float_flag_invalid;
+            result = 0xffffffff;
+        } else {
+            flags &= ~float_flag_invalid;
+        }
+        /* once invalid flag has been set, we cannot set inexact anymore
+           since each FPU operation can only assert ONE flag. (see
+           TriCore ISA Manual Vol. 1 (11-9)) */
+        if (!(flags & float_flag_invalid)) {
+            if (!float32_eq(f_arg, uint32_to_float32(result, &env->fp_status),
+                            &env->fp_status)) {
+                flags |= float_flag_inexact;
+            } else {
+                flags &= ~float_flag_inexact;
+            }
+        } else {
+            flags &= ~float_flag_inexact;
+        }
+        f_update_psw_flags(env, flags);
+    } else {
+        env->FPU_FS = 0;
+    }
+    return result;
+}
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 9333e16..467c880 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -112,6 +112,7 @@ DEF_HELPER_3(fdiv, i32, env, i32, i32)
 DEF_HELPER_3(fcmp, i32, env, i32, i32)
 DEF_HELPER_2(ftoi, i32, env, i32)
 DEF_HELPER_2(itof, i32, env, i32)
+DEF_HELPER_2(ftouz, i32, env, i32)
 /* dvinit */
 DEF_HELPER_3(dvinit_b_13, i64, env, i32, i32)
 DEF_HELPER_3(dvinit_b_131, i64, env, i32, i32)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 9a50df9..27c6d31 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6698,6 +6698,9 @@ static void decode_rr_divide(CPUTriCoreState *env, 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_FTOUZ:
+        gen_helper_ftouz(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
+        break;
     default:
         generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
     }
-- 
2.10.2

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Richard Henderson]

On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
> Converts a 32-bit floating point number to an unsigned int. The
> result is rounded towards zero.
>
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
> v1 -> v2:
>     - ftouz: Correctly convert the result from uint32 to f32
>
>  target-tricore/fpu_helper.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  target-tricore/helper.h     |  1 +
>  target-tricore/translate.c  |  3 +++
>  3 files changed, 47 insertions(+)
>
> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
> index 98fe947..f717b53 100644
> --- a/target-tricore/fpu_helper.c
> +++ b/target-tricore/fpu_helper.c
> @@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env, uint32_t arg)
>      }
>      return (uint32_t)f_result;
>  }
> +
> +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
> +{
> +    float32 f_arg = make_float32(arg);
> +    uint32_t result;
> +    int32_t flags;
> +    result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status);
> +
> +    flags = f_get_excp_flags(env);
> +    if (flags) {
> +        if (float32_is_any_nan(f_arg)) {
> +            flags |= float_flag_invalid;
> +            result = 0;
> +        /* f_real(D[a]) < 0.0 */
> +        } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) {
> +            flags |= float_flag_invalid;
> +            result = 0;
> +        /* f_real(D[a]) > 2^32 -1  */
> +        } else if (float32_lt_quiet(0x4f800000, f_arg, &env->fp_status)) {
> +            flags |= float_flag_invalid;
> +            result = 0xffffffff;
> +        } else {
> +            flags &= ~float_flag_invalid;
> +        }

I don't understand this bit.  Surely float_flag_invalid is already set by 
softfloat.  And the bounding is done as well.  What's up?

> +        /* once invalid flag has been set, we cannot set inexact anymore
> +           since each FPU operation can only assert ONE flag. (see
> +           TriCore ISA Manual Vol. 1 (11-9)) */
> +        if (!(flags & float_flag_invalid)) {
> +            if (!float32_eq(f_arg, uint32_to_float32(result, &env->fp_status),
> +                            &env->fp_status)) {
> +                flags |= float_flag_inexact;
> +            } else {
> +                flags &= ~float_flag_inexact;
> +            }
> +        } else {
> +            flags &= ~float_flag_inexact;
> +        }

And inexact is already set as well.

The best I can think is meant is

   /* Only one flag can be asserted.  */
   if (flags & float_flag_invalid) {
     flags &= ~float_flag_inexact;
   }

> +        f_update_psw_flags(env, flags);
> +    } else {
> +        env->FPU_FS = 0;
> +    }
> +    return result;
> +}


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Bastian Koppelmann]

On 11/08/2016 12:36 PM, Richard Henderson wrote:
> On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
>> Converts a 32-bit floating point number to an unsigned int. The
>> result is rounded towards zero.
>>
>> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> ---
>> v1 -> v2:
>>     - ftouz: Correctly convert the result from uint32 to f32
>>
>>  target-tricore/fpu_helper.c | 43
>> +++++++++++++++++++++++++++++++++++++++++++
>>  target-tricore/helper.h     |  1 +
>>  target-tricore/translate.c  |  3 +++
>>  3 files changed, 47 insertions(+)
>>
>> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
>> index 98fe947..f717b53 100644
>> --- a/target-tricore/fpu_helper.c
>> +++ b/target-tricore/fpu_helper.c
>> @@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env,
>> uint32_t arg)
>>      }
>>      return (uint32_t)f_result;
>>  }
>> +
>> +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
>> +{
>> +    float32 f_arg = make_float32(arg);
>> +    uint32_t result;
>> +    int32_t flags;
>> +    result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status);
>> +
>> +    flags = f_get_excp_flags(env);
>> +    if (flags) {
>> +        if (float32_is_any_nan(f_arg)) {
>> +            flags |= float_flag_invalid;
>> +            result = 0;
>> +        /* f_real(D[a]) < 0.0 */
>> +        } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) {
>> +            flags |= float_flag_invalid;
>> +            result = 0;
>> +        /* f_real(D[a]) > 2^32 -1  */
>> +        } else if (float32_lt_quiet(0x4f800000, f_arg,
>> &env->fp_status)) {
>> +            flags |= float_flag_invalid;
>> +            result = 0xffffffff;
>> +        } else {
>> +            flags &= ~float_flag_invalid;
>> +        }
> 
> I don't understand this bit.  Surely float_flag_invalid is already set
> by softfloat.  And the bounding is done as well.  What's up?
> 

Consider 0x836d4e86 as an input which is clearly negative, however
float_flag_invalid is not set. The hardware on the other hand does set it.

Cheers,
    Bastian

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Richard Henderson]

On 11/08/2016 02:37 PM, Bastian Koppelmann wrote:
> Consider 0x836d4e86 as an input which is clearly negative, however
> float_flag_invalid is not set. The hardware on the other hand does set it.

Hmm.  This is -0x1.da9d0cp-121.  Softfloat claims that we should round to zero 
first, and only if the result is still < 0, raise invalid.  Which does sound 
plausible as a common behaviour.

Does your hardware raise invalid for true -0.0, i.e. 0x80000000?


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Bastian Koppelmann]

On 11/08/2016 04:06 PM, Richard Henderson wrote:
> On 11/08/2016 02:37 PM, Bastian Koppelmann wrote:
>> Consider 0x836d4e86 as an input which is clearly negative, however
>> float_flag_invalid is not set. The hardware on the other hand does set
>> it.
> 
> Hmm.  This is -0x1.da9d0cp-121.  Softfloat claims that we should round
> to zero first, and only if the result is still < 0, raise invalid. 
> Which does sound plausible as a common behaviour.

TriCore does it the other way round.

> 
> Does your hardware raise invalid for true -0.0, i.e. 0x80000000?

No, -0.0 does not raise invalid.

Cheers,
    Bastian

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Richard Henderson]

On 11/08/2016 04:12 PM, Bastian Koppelmann wrote:
> On 11/08/2016 04:06 PM, Richard Henderson wrote:
>> On 11/08/2016 02:37 PM, Bastian Koppelmann wrote:
>>> Consider 0x836d4e86 as an input which is clearly negative, however
>>> float_flag_invalid is not set. The hardware on the other hand does set
>>> it.
>>
>> Hmm.  This is -0x1.da9d0cp-121.  Softfloat claims that we should round
>> to zero first, and only if the result is still < 0, raise invalid.
>> Which does sound plausible as a common behaviour.
>
> TriCore does it the other way round.
>
>>
>> Does your hardware raise invalid for true -0.0, i.e. 0x80000000?
>
> No, -0.0 does not raise invalid.

Then I suppose the check should look like

   flags = f_get_excp_flags(env);
   if (flags & float_flag_invalid) {
     flags &= ~float_flag_inexact;
   } else if (float32_lt_quiet(f_arg, 0, &env->status)) {
     flags = float_flag_invalid;
   }
   if (flags) {
     f_update_psw_flags(env, flags);
   }

Note that the 0.0 that you use is truncated to 0 by C for the uint32_t argument.


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Bastian Koppelmann]

On 11/08/2016 04:25 PM, Richard Henderson wrote:
> On 11/08/2016 04:12 PM, Bastian Koppelmann wrote:
>> On 11/08/2016 04:06 PM, Richard Henderson wrote:
>>> On 11/08/2016 02:37 PM, Bastian Koppelmann wrote:
>>>> Consider 0x836d4e86 as an input which is clearly negative, however
>>>> float_flag_invalid is not set. The hardware on the other hand does set
>>>> it.
>>>
>>> Hmm.  This is -0x1.da9d0cp-121.  Softfloat claims that we should round
>>> to zero first, and only if the result is still < 0, raise invalid.
>>> Which does sound plausible as a common behaviour.
>>
>> TriCore does it the other way round.
>>
>>>
>>> Does your hardware raise invalid for true -0.0, i.e. 0x80000000?
>>
>> No, -0.0 does not raise invalid.
> 
> Then I suppose the check should look like
> 
>   flags = f_get_excp_flags(env);
>   if (flags & float_flag_invalid) {
>     flags &= ~float_flag_inexact;
>   } else if (float32_lt_quiet(f_arg, 0, &env->status)) {
>     flags = float_flag_invalid;
>   }
>   if (flags) {
>     f_update_psw_flags(env, flags);
>   }
> 
> Note that the 0.0 that you use is truncated to 0 by C for the uint32_t
> argument.

Not quite -- it does not catch that an input NaN results in 0 as opposed
to -1 returned by softfloat.

But otherwise thanks for the review. This resulted in much nicer code.

Cheers,
    Bastian

[Qemu-devel] [PATCH v2 2/5] target-tricore: Added MADD.F and MSUB.F instructions

[Bastian Koppelmann]

Multiplies D[a] and D[b] and adds/subtracts the result to/from D[d].
The result is put in D[c]. All operands are floating-point numbers.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target-tricore/fpu_helper.c | 93 ++++++++++++++++++++++++++++++++++++++++++++-
 target-tricore/helper.h     |  2 +
 target-tricore/translate.c  |  8 ++++
 3 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
index f717b53..d530a0b 100644
--- a/target-tricore/fpu_helper.c
+++ b/target-tricore/fpu_helper.c
@@ -21,7 +21,8 @@
 #include "cpu.h"
 #include "exec/helper-proto.h"
 
-#define ADD_NAN   0x7cf00001
+#define QUIET_NAN 0x7fc00000
+#define ADD_NAN   0x7fc00001
 #define DIV_NAN   0x7fc00008
 #define MUL_NAN   0x7fc00002
 #define FPU_FS PSW_USB_C
@@ -47,6 +48,42 @@ static inline bool f_is_denormal(float32 arg)
     return float32_is_zero_or_denormal(arg) && !float32_is_zero(arg);
 }
 
+static inline float32 f_maddsub_nan_result(float32 arg1, float32 arg2,
+                                           float32 arg3, float32 result,
+                                           uint32_t flags)
+{
+    uint32_t aSign, bSign, cSign;
+    uint32_t aExp, bExp, cExp;
+
+    if (float32_is_any_nan(arg1) || float32_is_any_nan(arg2) ||
+        float32_is_any_nan(arg3)) {
+        return QUIET_NAN;
+    } else if (float32_is_infinity(arg1) && float32_is_zero(arg2)) {
+        return MUL_NAN;
+    } else if (float32_is_zero(arg1) && float32_is_infinity(arg2)) {
+        return MUL_NAN;
+    } else {
+        aSign = arg1 >> 31;
+        bSign = arg2 >> 31;
+        cSign = arg3 >> 31;
+
+        aExp = (arg1 >> 23) & 0xff;
+        bExp = (arg2 >> 23) & 0xff;
+        cExp = (arg3 >> 23) & 0xff;
+
+        if (flags & float_muladd_negate_c) {
+            cSign ^= 1;
+        }
+        if (((aExp == 0xff) || (bExp == 0xff)) && (cExp == 0xff)) {
+            if (aSign ^ bSign ^ cSign) {
+                return ADD_NAN;
+            }
+        }
+    }
+
+    return result;
+}
+
 static void f_update_psw_flags(CPUTriCoreState *env, uint8_t flags)
 {
     uint8_t some_excp = 0;
@@ -159,6 +196,60 @@ uint32_t helper_fdiv(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
     return (uint32_t)f_result;
 }
 
+uint32_t helper_fmadd(CPUTriCoreState *env, uint32_t r1,
+                      uint32_t r2, uint32_t r3)
+{
+    uint32_t flags;
+    float32 arg1 = make_float32(r1);
+    float32 arg2 = make_float32(r2);
+    float32 arg3 = make_float32(r3);
+    float32 f_result;
+
+    f_result = float32_muladd(arg1, arg2, arg3, 0, &env->fp_status);
+
+    flags = f_get_excp_flags(env);
+    if (flags) {
+        if (flags & float_flag_invalid) {
+            arg1 = float32_squash_input_denormal(arg1, &env->fp_status);
+            arg2 = float32_squash_input_denormal(arg2, &env->fp_status);
+            arg3 = float32_squash_input_denormal(arg3, &env->fp_status);
+            f_result = f_maddsub_nan_result(arg1, arg2, arg3, f_result, flags);
+        }
+        f_update_psw_flags(env, flags);
+    } else {
+        env->FPU_FS = 0;
+    }
+    return (uint32_t)f_result;
+}
+
+uint32_t helper_fmsub(CPUTriCoreState *env, uint32_t r1,
+                      uint32_t r2, uint32_t r3)
+{
+    uint32_t flags;
+    float32 arg1 = make_float32(r1);
+    float32 arg2 = make_float32(r2);
+    float32 arg3 = make_float32(r3);
+    float32 f_result;
+
+    f_result = float32_muladd(arg1, arg2, arg3, float_muladd_negate_product,
+                              &env->fp_status);
+
+    flags = f_get_excp_flags(env);
+    if (flags) {
+        if (flags & float_flag_invalid) {
+            arg1 = float32_squash_input_denormal(arg1, &env->fp_status);
+            arg2 = float32_squash_input_denormal(arg2, &env->fp_status);
+            arg3 = float32_squash_input_denormal(arg3, &env->fp_status);
+
+            f_result = f_maddsub_nan_result(arg1, arg2, arg3, f_result, flags);
+        }
+        f_update_psw_flags(env, flags);
+    } else {
+        env->FPU_FS = 0;
+    }
+    return (uint32_t)f_result;
+}
+
 uint32_t helper_fcmp(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
 {
     uint32_t result, flags;
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 467c880..c897a44 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -109,6 +109,8 @@ DEF_HELPER_3(fadd, i32, env, i32, i32)
 DEF_HELPER_3(fsub, i32, env, i32, i32)
 DEF_HELPER_3(fmul, i32, env, i32, i32)
 DEF_HELPER_3(fdiv, i32, env, i32, i32)
+DEF_HELPER_4(fmadd, i32, env, i32, i32, i32)
+DEF_HELPER_4(fmsub, i32, env, i32, i32, i32)
 DEF_HELPER_3(fcmp, i32, env, i32, i32)
 DEF_HELPER_2(ftoi, i32, env, i32)
 DEF_HELPER_2(itof, i32, env, i32)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 27c6d31..3fec353 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -7096,6 +7096,14 @@ static void decode_rrr_divide(CPUTriCoreState *env, DisasContext *ctx)
     case OPC2_32_RRR_SUB_F:
         gen_helper_fsub(cpu_gpr_d[r4], cpu_env, cpu_gpr_d[r1], cpu_gpr_d[r3]);
         break;
+    case OPC2_32_RRR_MADD_F:
+        gen_helper_fmadd(cpu_gpr_d[r4], cpu_env, cpu_gpr_d[r1],
+                         cpu_gpr_d[r2], cpu_gpr_d[r3]);
+        break;
+    case OPC2_32_RRR_MSUB_F:
+        gen_helper_fmsub(cpu_gpr_d[r4], cpu_env, cpu_gpr_d[r1],
+                         cpu_gpr_d[r2], cpu_gpr_d[r3]);
+        break;
     default:
         generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
     }
-- 
2.10.2

Re: [Qemu-devel] [PATCH v2 2/5] target-tricore: Added MADD.F and MSUB.F instructions

[Richard Henderson]

On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
> Multiplies D[a] and D[b] and adds/subtracts the result to/from D[d].
> The result is put in D[c]. All operands are floating-point numbers.
>
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
>  target-tricore/fpu_helper.c | 93 ++++++++++++++++++++++++++++++++++++++++++++-
>  target-tricore/helper.h     |  2 +
>  target-tricore/translate.c  |  8 ++++
>  3 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
> index f717b53..d530a0b 100644
> --- a/target-tricore/fpu_helper.c
> +++ b/target-tricore/fpu_helper.c
> @@ -21,7 +21,8 @@
>  #include "cpu.h"
>  #include "exec/helper-proto.h"
>
> -#define ADD_NAN   0x7cf00001
> +#define QUIET_NAN 0x7fc00000
> +#define ADD_NAN   0x7fc00001
>  #define DIV_NAN   0x7fc00008
>  #define MUL_NAN   0x7fc00002
>  #define FPU_FS PSW_USB_C
> @@ -47,6 +48,42 @@ static inline bool f_is_denormal(float32 arg)
>      return float32_is_zero_or_denormal(arg) && !float32_is_zero(arg);
>  }
>
> +static inline float32 f_maddsub_nan_result(float32 arg1, float32 arg2,
> +                                           float32 arg3, float32 result,
> +                                           uint32_t flags)
> +{
> +    uint32_t aSign, bSign, cSign;
> +    uint32_t aExp, bExp, cExp;
> +
> +    if (float32_is_any_nan(arg1) || float32_is_any_nan(arg2) ||
> +        float32_is_any_nan(arg3)) {
> +        return QUIET_NAN;
> +    } else if (float32_is_infinity(arg1) && float32_is_zero(arg2)) {
> +        return MUL_NAN;
> +    } else if (float32_is_zero(arg1) && float32_is_infinity(arg2)) {
> +        return MUL_NAN;
> +    } else {
> +        aSign = arg1 >> 31;
> +        bSign = arg2 >> 31;
> +        cSign = arg3 >> 31;
> +
> +        aExp = (arg1 >> 23) & 0xff;
> +        bExp = (arg2 >> 23) & 0xff;
> +        cExp = (arg3 >> 23) & 0xff;
> +
> +        if (flags & float_muladd_negate_c) {

flags here is muladd argument,

> +    flags = f_get_excp_flags(env);
> +    if (flags) {
> +        if (flags & float_flag_invalid) {
> +            arg1 = float32_squash_input_denormal(arg1, &env->fp_status);
> +            arg2 = float32_squash_input_denormal(arg2, &env->fp_status);
> +            arg3 = float32_squash_input_denormal(arg3, &env->fp_status);
> +            f_result = f_maddsub_nan_result(arg1, arg2, arg3, f_result, flags);

but passed here as exception bits.

> +        }
> +        f_update_psw_flags(env, flags);
> +    } else {
> +        env->FPU_FS = 0;
> +    }
> +    return (uint32_t)f_result;
> +}
> +
> +uint32_t helper_fmsub(CPUTriCoreState *env, uint32_t r1,
> +                      uint32_t r2, uint32_t r3)
> +{
> +    uint32_t flags;
> +    float32 arg1 = make_float32(r1);
> +    float32 arg2 = make_float32(r2);
> +    float32 arg3 = make_float32(r3);
> +    float32 f_result;
> +
> +    f_result = float32_muladd(arg1, arg2, arg3, float_muladd_negate_product,
> +                              &env->fp_status);
> +
> +    flags = f_get_excp_flags(env);
> +    if (flags) {
> +        if (flags & float_flag_invalid) {
> +            arg1 = float32_squash_input_denormal(arg1, &env->fp_status);
> +            arg2 = float32_squash_input_denormal(arg2, &env->fp_status);
> +            arg3 = float32_squash_input_denormal(arg3, &env->fp_status);
> +
> +            f_result = f_maddsub_nan_result(arg1, arg2, arg3, f_result, flags);

Same.  Also, nan_result needs to check float_muladd_negate_product as used 
here, not float_muladd_negate_c as used above.


r~

Re: [Qemu-devel] [PATCH v2 2/5] target-tricore: Added MADD.F and MSUB.F instructions

[Bastian Koppelmann]

On 11/08/2016 12:42 PM, Richard Henderson wrote:
> On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
>> Multiplies D[a] and D[b] and adds/subtracts the result to/from D[d].
>> The result is put in D[c]. All operands are floating-point numbers.
>>
>> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> ---
>>  target-tricore/fpu_helper.c | 93
>> ++++++++++++++++++++++++++++++++++++++++++++-
>>  target-tricore/helper.h     |  2 +
>>  target-tricore/translate.c  |  8 ++++
>>  3 files changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
>> index f717b53..d530a0b 100644
>> --- a/target-tricore/fpu_helper.c
>> +++ b/target-tricore/fpu_helper.c
>> @@ -21,7 +21,8 @@
>>  #include "cpu.h"
>>  #include "exec/helper-proto.h"
>>
>> -#define ADD_NAN   0x7cf00001
>> +#define QUIET_NAN 0x7fc00000
>> +#define ADD_NAN   0x7fc00001
>>  #define DIV_NAN   0x7fc00008
>>  #define MUL_NAN   0x7fc00002
>>  #define FPU_FS PSW_USB_C
>> @@ -47,6 +48,42 @@ static inline bool f_is_denormal(float32 arg)
>>      return float32_is_zero_or_denormal(arg) && !float32_is_zero(arg);
>>  }
>>
>> +static inline float32 f_maddsub_nan_result(float32 arg1, float32 arg2,
>> +                                           float32 arg3, float32 result,
>> +                                           uint32_t flags)
>> +{
>> +    uint32_t aSign, bSign, cSign;
>> +    uint32_t aExp, bExp, cExp;
>> +
>> +    if (float32_is_any_nan(arg1) || float32_is_any_nan(arg2) ||
>> +        float32_is_any_nan(arg3)) {
>> +        return QUIET_NAN;
>> +    } else if (float32_is_infinity(arg1) && float32_is_zero(arg2)) {
>> +        return MUL_NAN;
>> +    } else if (float32_is_zero(arg1) && float32_is_infinity(arg2)) {
>> +        return MUL_NAN;
>> +    } else {
>> +        aSign = arg1 >> 31;
>> +        bSign = arg2 >> 31;
>> +        cSign = arg3 >> 31;
>> +
>> +        aExp = (arg1 >> 23) & 0xff;
>> +        bExp = (arg2 >> 23) & 0xff;
>> +        cExp = (arg3 >> 23) & 0xff;
>> +
>> +        if (flags & float_muladd_negate_c) {
> 
> flags here is muladd argument,
> 

I don't know what you mean by that.

However, I took this from softfloat's float32_muladd() to catch the case
of an invalid addition to return the correct NAN.

I thought of extending softfloat's muladd() to return a specializable
ADD_NAN to catch this case in the first place, but TriCore would be the
only target using it and I'm not sure if it's worth while.

Any comments on this idea?

Cheers,
    Bastian

Re: [Qemu-devel] [PATCH v2 2/5] target-tricore: Added MADD.F and MSUB.F instructions

[Richard Henderson]

On 11/08/2016 04:08 PM, Bastian Koppelmann wrote:
> On 11/08/2016 12:42 PM, Richard Henderson wrote:
>> On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
>>> Multiplies D[a] and D[b] and adds/subtracts the result to/from D[d].
>>> The result is put in D[c]. All operands are floating-point numbers.
>>>
>>> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>>> ---
>>>  target-tricore/fpu_helper.c | 93
>>> ++++++++++++++++++++++++++++++++++++++++++++-
>>>  target-tricore/helper.h     |  2 +
>>>  target-tricore/translate.c  |  8 ++++
>>>  3 files changed, 102 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
>>> index f717b53..d530a0b 100644
>>> --- a/target-tricore/fpu_helper.c
>>> +++ b/target-tricore/fpu_helper.c
>>> @@ -21,7 +21,8 @@
>>>  #include "cpu.h"
>>>  #include "exec/helper-proto.h"
>>>
>>> -#define ADD_NAN   0x7cf00001
>>> +#define QUIET_NAN 0x7fc00000
>>> +#define ADD_NAN   0x7fc00001
>>>  #define DIV_NAN   0x7fc00008
>>>  #define MUL_NAN   0x7fc00002
>>>  #define FPU_FS PSW_USB_C
>>> @@ -47,6 +48,42 @@ static inline bool f_is_denormal(float32 arg)
>>>      return float32_is_zero_or_denormal(arg) && !float32_is_zero(arg);
>>>  }
>>>
>>> +static inline float32 f_maddsub_nan_result(float32 arg1, float32 arg2,
>>> +                                           float32 arg3, float32 result,
>>> +                                           uint32_t flags)
>>> +{
>>> +    uint32_t aSign, bSign, cSign;
>>> +    uint32_t aExp, bExp, cExp;
>>> +
>>> +    if (float32_is_any_nan(arg1) || float32_is_any_nan(arg2) ||
>>> +        float32_is_any_nan(arg3)) {
>>> +        return QUIET_NAN;
>>> +    } else if (float32_is_infinity(arg1) && float32_is_zero(arg2)) {
>>> +        return MUL_NAN;
>>> +    } else if (float32_is_zero(arg1) && float32_is_infinity(arg2)) {
>>> +        return MUL_NAN;
>>> +    } else {
>>> +        aSign = arg1 >> 31;
>>> +        bSign = arg2 >> 31;
>>> +        cSign = arg3 >> 31;
>>> +
>>> +        aExp = (arg1 >> 23) & 0xff;
>>> +        bExp = (arg2 >> 23) & 0xff;
>>> +        cExp = (arg3 >> 23) & 0xff;
>>> +
>>> +        if (flags & float_muladd_negate_c) {
>>
>> flags here is muladd argument,
>>
>
> I don't know what you mean by that.

I mean that the type you pass is not the type you check here.
They're different sets of bitmasks.

> I thought of extending softfloat's muladd() to return a specializable
> ADD_NAN to catch this case in the first place, but TriCore would be the
> only target using it and I'm not sure if it's worth while.
>
> Any comments on this idea?

I think just fixing up tricore is the way to go.


r~

[Qemu-devel] [PATCH v2 3/5] target-tricore: Added new MOV instruction variant

[Bastian Koppelmann]

From: Peer Adelt <peer.adelt@c-lab.de>

Puts the content of data register D[a] into E[c][63:32] and the
content of data register D[b] into E[c][31:0].

[BK: fix style error]
Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
Message-Id: <1465314555-11501-4-git-send-email-peer.adelt@c-lab.de>
---
 target-tricore/translate.c       | 15 +++++++++++++++
 target-tricore/tricore-opcodes.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 3fec353..4fe8a5f 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6034,11 +6034,15 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
     uint32_t op2;
     int r3, r2, r1;
 
+    TCGv temp;
+
     r3 = MASK_OP_RR_D(ctx->opcode);
     r2 = MASK_OP_RR_S2(ctx->opcode);
     r1 = MASK_OP_RR_S1(ctx->opcode);
     op2 = MASK_OP_RR_OP2(ctx->opcode);
 
+    temp = tcg_temp_new();
+
     switch (op2) {
     case OPC2_32_RR_ABS:
         gen_abs(cpu_gpr_d[r3], cpu_gpr_d[r2]);
@@ -6224,6 +6228,16 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
     case OPC2_32_RR_MOV:
         tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r2]);
         break;
+    case OPC2_32_RR_MOV_64:
+        if (tricore_feature(env, TRICORE_FEATURE_16)) {
+            CHECK_REG_PAIR(r3);
+            tcg_gen_mov_tl(temp, cpu_gpr_d[r1]);
+            tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r2]);
+            tcg_gen_mov_tl(cpu_gpr_d[r3 + 1], temp);
+        } else {
+            generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
+        }
+        break;
     case OPC2_32_RR_NE:
         tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_d[r1],
                            cpu_gpr_d[r2]);
@@ -6344,6 +6358,7 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
     default:
         generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
     }
+    tcg_temp_free(temp);
 }
 
 static void decode_rr_logical_shift(CPUTriCoreState *env, DisasContext *ctx)
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index df666b0..78ba338 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -1062,6 +1062,7 @@ enum {
     OPC2_32_RR_MIN_H                             = 0x78,
     OPC2_32_RR_MIN_HU                            = 0x79,
     OPC2_32_RR_MOV                               = 0x1f,
+    OPC2_32_RR_MOV_64                            = 0x81,
     OPC2_32_RR_NE                                = 0x11,
     OPC2_32_RR_OR_EQ                             = 0x27,
     OPC2_32_RR_OR_GE                             = 0x2b,
-- 
2.10.2

Re: [Qemu-devel] [PATCH v2 3/5] target-tricore: Added new MOV instruction variant

[Richard Henderson]

On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
> From: Peer Adelt <peer.adelt@c-lab.de>
>
> Puts the content of data register D[a] into E[c][63:32] and the
> content of data register D[b] into E[c][31:0].
>
> [BK: fix style error]
> Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
> Message-Id: <1465314555-11501-4-git-send-email-peer.adelt@c-lab.de>
> ---
>  target-tricore/translate.c       | 15 +++++++++++++++
>  target-tricore/tricore-opcodes.h |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/target-tricore/translate.c b/target-tricore/translate.c
> index 3fec353..4fe8a5f 100644
> --- a/target-tricore/translate.c
> +++ b/target-tricore/translate.c
> @@ -6034,11 +6034,15 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
>      uint32_t op2;
>      int r3, r2, r1;
>
> +    TCGv temp;
> +
>      r3 = MASK_OP_RR_D(ctx->opcode);
>      r2 = MASK_OP_RR_S2(ctx->opcode);
>      r1 = MASK_OP_RR_S1(ctx->opcode);
>      op2 = MASK_OP_RR_OP2(ctx->opcode);
>
> +    temp = tcg_temp_new();

There's no reason to allocate temp here, for all of the insns that don't 
require it, as opposed to ...
> +
>      switch (op2) {
>      case OPC2_32_RR_ABS:
>          gen_abs(cpu_gpr_d[r3], cpu_gpr_d[r2]);
> @@ -6224,6 +6228,16 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
>      case OPC2_32_RR_MOV:
>          tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r2]);
>          break;
> +    case OPC2_32_RR_MOV_64:
> +        if (tricore_feature(env, TRICORE_FEATURE_16)) {
> +            CHECK_REG_PAIR(r3);

... here within the IF, right before it's used.

> +            tcg_gen_mov_tl(temp, cpu_gpr_d[r1]);
> +            tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r2]);
> +            tcg_gen_mov_tl(cpu_gpr_d[r3 + 1], temp);

And freed here...

> +        } else {
> +            generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
> +        }
> +        break;
>      case OPC2_32_RR_NE:
>          tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_d[r1],
>                             cpu_gpr_d[r2]);
> @@ -6344,6 +6358,7 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
>      default:
>          generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
>      }
> +    tcg_temp_free(temp);

... not here.


r~

[Qemu-devel] [PATCH v2 4/5] target-tricore: Added new JNE instruction variant

[Bastian Koppelmann]

From: Peer Adelt <peer.adelt@c-lab.de>

If D[15] is != sign_ext(const4) then PC will be set to (PC +
zero_ext(disp4 + 16)).

[BK: fixed style errors]
Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
Message-Id: <1465314555-11501-5-git-send-email-peer.adelt@c-lab.de>
---
 target-tricore/translate.c       | 18 ++++++++++++++++++
 target-tricore/tricore-opcodes.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 4fe8a5f..be9b6b4 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3362,9 +3362,17 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, int r1,
     case OPC1_16_SBC_JEQ:
         gen_branch_condi(ctx, TCG_COND_EQ, cpu_gpr_d[15], constant, offset);
         break;
+    case OPC1_16_SBC_JEQ2:
+        gen_branch_condi(ctx, TCG_COND_EQ, cpu_gpr_d[15], constant,
+                         offset + 16);
+        break;
     case OPC1_16_SBC_JNE:
         gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[15], constant, offset);
         break;
+    case OPC1_16_SBC_JNE2:
+        gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[15],
+                         constant, offset + 16);
+        break;
 /* SBRN-format jumps */
     case OPC1_16_SBRN_JZ_T:
         temp = tcg_temp_new();
@@ -4097,6 +4105,16 @@ static void decode_16Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
         const16 = MASK_OP_SBC_CONST4_SEXT(ctx->opcode);
         gen_compute_branch(ctx, op1, 0, 0, const16, address);
         break;
+    case OPC1_16_SBC_JEQ2:
+    case OPC1_16_SBC_JNE2:
+        if (tricore_feature(env, TRICORE_FEATURE_16)) {
+            address = MASK_OP_SBC_DISP4(ctx->opcode);
+            const16 = MASK_OP_SBC_CONST4_SEXT(ctx->opcode);
+            gen_compute_branch(ctx, op1, 0, 0, const16, address);
+        } else {
+            generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
+        }
+        break;
 /* SBRN-format */
     case OPC1_16_SBRN_JNZ_T:
     case OPC1_16_SBRN_JZ_T:
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index 78ba338..08394b8 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -311,6 +311,7 @@ enum {
     OPC1_16_SRR_EQ                                   = 0x3a,
     OPC1_16_SB_J                                     = 0x3c,
     OPC1_16_SBC_JEQ                                  = 0x1e,
+    OPC1_16_SBC_JEQ2                                 = 0x9e,
     OPC1_16_SBR_JEQ                                  = 0x3e,
     OPC1_16_SBR_JGEZ                                 = 0xce,
     OPC1_16_SBR_JGTZ                                 = 0x4e,
@@ -318,6 +319,7 @@ enum {
     OPC1_16_SBR_JLEZ                                 = 0x8e,
     OPC1_16_SBR_JLTZ                                 = 0x0e,
     OPC1_16_SBC_JNE                                  = 0x5e,
+    OPC1_16_SBC_JNE2                                 = 0xde,
     OPC1_16_SBR_JNE                                  = 0x7e,
     OPC1_16_SB_JNZ                                   = 0xee,
     OPC1_16_SBR_JNZ                                  = 0xf6,
-- 
2.10.2

[Qemu-devel] [PATCH v2 5/5] target-tricore: Add updfl instruction

[Bastian Koppelmann]

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target-tricore/fpu_helper.c | 12 ++++++++++++
 target-tricore/helper.h     |  1 +
 target-tricore/translate.c  |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
index d530a0b..89de0ea 100644
--- a/target-tricore/fpu_helper.c
+++ b/target-tricore/fpu_helper.c
@@ -349,3 +349,15 @@ uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
     }
     return result;
 }
+
+void helper_updfl(CPUTriCoreState *env, uint32_t arg)
+{
+    env->FPU_FS =  extract32(arg, 7, 1) & extract32(arg, 15, 1);
+    env->FPU_FI = (extract32(arg, 6, 1) & extract32(arg, 14, 1)) << 31;
+    env->FPU_FV = (extract32(arg, 5, 1) & extract32(arg, 13, 1)) << 31;
+    env->FPU_FZ = (extract32(arg, 4, 1) & extract32(arg, 12, 1)) << 31;
+    env->FPU_FU = (extract32(arg, 3, 1) & extract32(arg, 11, 1)) << 31;
+    /* clear FX and RM */
+    env->PSW &= ~(extract32(arg, 10, 1) << 26);
+    env->PSW |= (extract32(arg, 2, 1) & extract32(arg, 10, 1)) << 26;
+}
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index c897a44..012ac96 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -115,6 +115,7 @@ DEF_HELPER_3(fcmp, i32, env, i32, i32)
 DEF_HELPER_2(ftoi, i32, env, i32)
 DEF_HELPER_2(itof, i32, env, i32)
 DEF_HELPER_2(ftouz, i32, env, i32)
+DEF_HELPER_2(updfl, void, env, i32)
 /* dvinit */
 DEF_HELPER_3(dvinit_b_13, i64, env, i32, i32)
 DEF_HELPER_3(dvinit_b_131, i64, env, i32, i32)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index be9b6b4..a3f980a 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6848,6 +6848,9 @@ static void decode_rr1_mul(CPUTriCoreState *env, DisasContext *ctx)
         GEN_HELPER_UU(mulr_h, cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2], n);
         gen_calc_usb_mulr_h(cpu_gpr_d[r3]);
         break;
+    case OPC2_32_RR_UPDFL:
+        gen_helper_updfl(cpu_env, cpu_gpr_d[r1]);
+        break;
     default:
         generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
     }
-- 
2.10.2

Re: [Qemu-devel] [PATCH v2 5/5] target-tricore: Add updfl instruction

[Richard Henderson]

On 11/07/2016 03:44 PM, Bastian Koppelmann wrote:
> +void helper_updfl(CPUTriCoreState *env, uint32_t arg)
> +{
> +    env->FPU_FS =  extract32(arg, 7, 1) & extract32(arg, 15, 1);
> +    env->FPU_FI = (extract32(arg, 6, 1) & extract32(arg, 14, 1)) << 31;
> +    env->FPU_FV = (extract32(arg, 5, 1) & extract32(arg, 13, 1)) << 31;
> +    env->FPU_FZ = (extract32(arg, 4, 1) & extract32(arg, 12, 1)) << 31;
> +    env->FPU_FU = (extract32(arg, 3, 1) & extract32(arg, 11, 1)) << 31;
> +    /* clear FX and RM */
> +    env->PSW &= ~(extract32(arg, 10, 1) << 26);
> +    env->PSW |= (extract32(arg, 2, 1) & extract32(arg, 10, 1)) << 26;
> +}

Need a call to fpu_set_state here, to update the rounding mode in env->fp_status.


r~