[Qemu-devel] [PATCH 0/4] Added 5 instructions to the tricore target

[Qemu-devel] [PATCH 0/4] Added 5 instructions to the tricore target

[peer.adelt]

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

This patch set contains 5 new instructions:
- FTOUZ (converts float to unsigned int, rounds towards zero)
- MADD.F / MSUB.F (multiplies two floats and adds/subtracts result
                   to/from the third operand)
- MOV (new variant in RR format - see ISA v1.6 for details)
- JNE (new variant in SBC format - see ISA v1.6 for details) 

Peer Adelt (4):
  target-tricore: Added FTOUZ instruction
  target-tricore: Added MADD.F and MSUB.F instructions
  target-tricore: Added new MOV instruction variant
  target-tricore: Added new JNE instruction variant

 target-tricore/fpu_helper.c      | 74 ++++++++++++++++++++++++++++++++++++++++
 target-tricore/helper.h          |  3 ++
 target-tricore/translate.c       | 16 +++++++++
 target-tricore/tricore-opcodes.h |  2 ++
 4 files changed, 95 insertions(+)

-- 
2.7.4 (Apple Git-66)

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

[peer.adelt]

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

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

Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
---
 target-tricore/fpu_helper.c | 20 ++++++++++++++++++++
 target-tricore/helper.h     |  1 +
 target-tricore/translate.c  |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
index 98fe947..ccaa6b0 100644
--- a/target-tricore/fpu_helper.c
+++ b/target-tricore/fpu_helper.c
@@ -215,3 +215,23 @@ 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)) {
+            result = 0;
+        }
+        f_update_psw_flags(env, flags);
+    } else {
+        env->FPU_FS = 0;
+    }
+    return (uint32_t)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 83fa4fc..a109c15 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.7.4 (Apple Git-66)

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

[Bastian Koppelmann]

On 05/30/2016 12:59 AM, peer.adelt@c-lab.de wrote:
> From: Peer Adelt <peer.adelt@c-lab.de>
> 
> Converts a 32-bit floating point number to an unsigned int. The
> result is rounded towards zero.
> 
> Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
> ---
>  target-tricore/fpu_helper.c | 20 ++++++++++++++++++++
>  target-tricore/helper.h     |  1 +
>  target-tricore/translate.c  |  3 +++
>  3 files changed, 24 insertions(+)
> 

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
    Bastian

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

[Richard Henderson]

On 05/29/2016 03:59 PM, peer.adelt@c-lab.de wrote:
> +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg)
...
> +    uint32_t result;
...
> +    return (uint32_t)result;

Don't add pointless casts.


r~

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

[peer.adelt]

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

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: Peer Adelt <peer.adelt@c-lab.de>
---
 target-tricore/fpu_helper.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
 target-tricore/helper.h     |  2 ++
 target-tricore/translate.c  |  8 +++++++
 3 files changed, 64 insertions(+)

diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c
index ccaa6b0..3207818 100644
--- a/target-tricore/fpu_helper.c
+++ b/target-tricore/fpu_helper.c
@@ -159,6 +159,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;
+
+    flags = f_get_excp_flags(env);
+    f_result = float32_muladd(arg1, arg2, arg3, flags, &env->fp_status);
+
+    if (flags) {
+        /* If the output is a NaN, but the inputs aren't,
+           we return a unique value.  */
+        if ((flags & float_flag_invalid)
+            && !float32_is_any_nan(arg1)
+            && !float32_is_any_nan(arg2)) {
+                f_result = MUL_NAN;
+        }
+        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;
+
+    flags = f_get_excp_flags(env);
+    f_result = float32_muladd(-arg1, arg2, arg3, flags, &env->fp_status);
+
+    if (flags) {
+        /* If the output is a NaN, but the inputs aren't,
+           we return a unique value.  */
+        if ((flags & float_flag_invalid)
+            && !float32_is_any_nan(arg1)
+            && !float32_is_any_nan(arg2)) {
+                f_result = MUL_NAN;
+        }
+        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 a109c15..e66b433 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.7.4 (Apple Git-66)

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

[Bastian Koppelmann]

On 05/30/2016 12:59 AM, peer.adelt@c-lab.de wrote:
> +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;
> +
> +    flags = f_get_excp_flags(env);
> +    f_result = float32_muladd(arg1, arg2, arg3, flags, &env->fp_status);
The flags argument allows the caller to select negation of the
addend, the intermediate product, or the final result. These are not
excp_flags. Additionally, you need to collect the excp_flags after doing
the muladd.
> +
> +    if (flags) {
> +        /* If the output is a NaN, but the inputs aren't,
> +           we return a unique value.  */
> +        if ((flags & float_flag_invalid)
> +            && !float32_is_any_nan(arg1)
> +            && !float32_is_any_nan(arg2)) {
You are not doing what the comment says. You have three inputs here but
you only check two.

> +    float32 arg2 = make_float32(r2);
> +    float32 arg3 = make_float32(r3);
> +    float32 f_result;
> +
> +    flags = f_get_excp_flags(env);
> +    f_result = float32_muladd(-arg1, arg2, arg3, flags, &env->fp_status);
Likewise.

> +    if (flags) {
> +        /* If the output is a NaN, but the inputs aren't,
> +           we return a unique value.  */
> +        if ((flags & float_flag_invalid)
> +            && !float32_is_any_nan(arg1)
> +            && !float32_is_any_nan(arg2)) {
Likewise.

Cheers,
    Bastian

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

[Richard Henderson]

On 05/29/2016 03:59 PM, peer.adelt@c-lab.de wrote:
> +    flags = f_get_excp_flags(env);
> +    f_result = float32_muladd(-arg1, arg2, arg3, flags, &env->fp_status);

Bastian already pointed out that flags here is being used wrong,
but I thought I'd reinforce that "-arg1" is not how floating-point
negation works.  You need to use float_muladd_negate_product as
that 4th argument to float32_muladd.


r~

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

[peer.adelt]

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].

Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
---
 target-tricore/translate.c       | 4 ++++
 target-tricore/tricore-opcodes.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index e66b433..2145f64 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6224,6 +6224,10 @@ 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_EXT:
+        tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
+        tcg_gen_mov_tl(cpu_gpr_d[(r3 + 1)], cpu_gpr_d[r2]);
+        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]);
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index df666b0..2f25613 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_EXT                           = 0x81,
     OPC2_32_RR_NE                                = 0x11,
     OPC2_32_RR_OR_EQ                             = 0x27,
     OPC2_32_RR_OR_GE                             = 0x2b,
-- 
2.7.4 (Apple Git-66)

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

[Bastian Koppelmann]

On 05/30/2016 12:59 AM, peer.adelt@c-lab.de 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].
> 
> Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
> ---
>  target-tricore/translate.c       | 4 ++++
>  target-tricore/tricore-opcodes.h | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/target-tricore/translate.c b/target-tricore/translate.c
> index e66b433..2145f64 100644
> --- a/target-tricore/translate.c
> +++ b/target-tricore/translate.c
> @@ -6224,6 +6224,10 @@ 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_EXT:
> +        tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
> +        tcg_gen_mov_tl(cpu_gpr_d[(r3 + 1)], cpu_gpr_d[r2]);
> +        break;

Since this is a 1.6+ instruction, please check the hflags for the 1.6
ISA and raise an exception if 1.6+ ISA is not met. See OPC1_16_SRC_MOV_E
as an example.

Cheers,
    Bastian

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

[Richard Henderson]

On 05/29/2016 03:59 PM, peer.adelt@c-lab.de wrote:
> +        tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
> +        tcg_gen_mov_tl(cpu_gpr_d[(r3 + 1)], cpu_gpr_d[r2]);

Don't add pointless parenthesis.

But more importantly, you need to worry about overlap between r3 and r2.
In the general case you'll need a temporary to perform this move.


r~

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

[peer.adelt]

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)).

Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
---
 target-tricore/translate.c       | 1 +
 target-tricore/tricore-opcodes.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 2145f64..9ad9fcc 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3363,6 +3363,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, int r1,
         gen_branch_condi(ctx, TCG_COND_EQ, cpu_gpr_d[15], constant, offset);
         break;
     case OPC1_16_SBC_JNE:
+    case OPC1_16_SBC_JNE16:
         gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[15], constant, offset);
         break;
 /* SBRN-format jumps */
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index 2f25613..7925354 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -318,6 +318,7 @@ enum {
     OPC1_16_SBR_JLEZ                                 = 0x8e,
     OPC1_16_SBR_JLTZ                                 = 0x0e,
     OPC1_16_SBC_JNE                                  = 0x5e,
+    OPC1_16_SBC_JNE16                                = 0xde,
     OPC1_16_SBR_JNE                                  = 0x7e,
     OPC1_16_SB_JNZ                                   = 0xee,
     OPC1_16_SBR_JNZ                                  = 0xf6,
-- 
2.7.4 (Apple Git-66)

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

[Bastian Koppelmann]

On 05/30/2016 12:59 AM, peer.adelt@c-lab.de wrote:
> 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)).
> 
> Signed-off-by: Peer Adelt <peer.adelt@c-lab.de>
> ---
>  target-tricore/translate.c       | 1 +
>  target-tricore/tricore-opcodes.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/target-tricore/translate.c b/target-tricore/translate.c
> index 2145f64..9ad9fcc 100644
> --- a/target-tricore/translate.c
> +++ b/target-tricore/translate.c
> @@ -3363,6 +3363,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, int r1,
>          gen_branch_condi(ctx, TCG_COND_EQ, cpu_gpr_d[15], constant, offset);
>          break;
>      case OPC1_16_SBC_JNE:
> +    case OPC1_16_SBC_JNE16:
>          gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[15], constant, offset);
>          break;

You forgot to call gen_compute_branch() from decode_16Bit_opc() for this
instruction, which should do the addition of 16 to disp4. Also please
add a check for 1.6+ ISA as suggested for the patch before.

Cheers,
    Bastian