[Qemu-devel] [PATCH 1/5] ppc: Don't generate dead code on unconditional branches

[Qemu-devel] [PATCH 1/5] ppc: Don't generate dead code on unconditional branches

[Benjamin Herrenschmidt]

We are always generating the "else" case of the condition even when
generating an unconditional branch that will never hit it.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/translate.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d1837f8..47eb9ed 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3479,8 +3479,10 @@ static inline void gen_bcond(DisasContext *ctx, int type)
         } else {
             gen_goto_tb(ctx, 0, li);
         }
-        gen_set_label(l1);
-        gen_goto_tb(ctx, 1, ctx->nip);
+        if ((bo & 0x14) != 0x14) {
+            gen_set_label(l1);
+            gen_goto_tb(ctx, 1, ctx->nip);
+        }
     } else {
         if (NARROW_MODE(ctx)) {
             tcg_gen_andi_tl(cpu_nip, target, (uint32_t)~3);
@@ -3488,9 +3490,11 @@ static inline void gen_bcond(DisasContext *ctx, int type)
             tcg_gen_andi_tl(cpu_nip, target, ~3);
         }
         tcg_gen_exit_tb(0);
-        gen_set_label(l1);
-        gen_update_nip(ctx, ctx->nip);
-        tcg_gen_exit_tb(0);
+        if ((bo & 0x14) != 0x14) {
+            gen_set_label(l1);
+            gen_update_nip(ctx, ctx->nip);
+            tcg_gen_exit_tb(0);
+        }
     }
     if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
         tcg_temp_free(target);
-- 
2.7.4

[Qemu-devel] [PATCH 2/5] ppc: Improve flags for helpers loading/writing the time facilities

[Benjamin Herrenschmidt]

Those helpers never load from or store to the TCG tracked environment,
not do they generate synchronous exceptions (they might generate an
asynchronous interrupt but that's not an issue here).

So we can make them all use TCG_CALL_NO_RWG

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/helper.h | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 8eada2f..b0c1db9 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -595,35 +595,35 @@ DEF_HELPER_2(load_dump_spr, void, env, i32)
 DEF_HELPER_2(store_dump_spr, void, env, i32)
 DEF_HELPER_4(fscr_facility_check, void, env, i32, i32, i32)
 DEF_HELPER_4(msr_facility_check, void, env, i32, i32, i32)
-DEF_HELPER_1(load_tbl, tl, env)
-DEF_HELPER_1(load_tbu, tl, env)
-DEF_HELPER_1(load_atbl, tl, env)
-DEF_HELPER_1(load_atbu, tl, env)
-DEF_HELPER_1(load_601_rtcl, tl, env)
-DEF_HELPER_1(load_601_rtcu, tl, env)
+DEF_HELPER_FLAGS_1(load_tbl, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_1(load_tbu, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_1(load_atbl, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_1(load_atbu, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_1(load_601_rtcl, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_1(load_601_rtcu, TCG_CALL_NO_RWG, tl, env)
 #if !defined(CONFIG_USER_ONLY)
 #if defined(TARGET_PPC64)
-DEF_HELPER_1(load_purr, tl, env)
+DEF_HELPER_FLAGS_1(load_purr, TCG_CALL_NO_RWG, tl, env)
 #endif
 DEF_HELPER_2(store_sdr1, void, env, tl)
-DEF_HELPER_2(store_tbl, void, env, tl)
-DEF_HELPER_2(store_tbu, void, env, tl)
-DEF_HELPER_2(store_atbl, void, env, tl)
-DEF_HELPER_2(store_atbu, void, env, tl)
-DEF_HELPER_2(store_601_rtcl, void, env, tl)
-DEF_HELPER_2(store_601_rtcu, void, env, tl)
-DEF_HELPER_1(load_decr, tl, env)
-DEF_HELPER_2(store_decr, void, env, tl)
-DEF_HELPER_1(load_hdecr, tl, env)
-DEF_HELPER_2(store_hdecr, void, env, tl)
+DEF_HELPER_FLAGS_2(store_tbl, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_tbu, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_atbl, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_atbu, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_601_rtcl, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_601_rtcu, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_1(load_decr, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_2(store_decr, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_1(load_hdecr, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_2(store_hdecr, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_2(store_hid0_601, void, env, tl)
 DEF_HELPER_3(store_403_pbr, void, env, i32, tl)
-DEF_HELPER_1(load_40x_pit, tl, env)
-DEF_HELPER_2(store_40x_pit, void, env, tl)
+DEF_HELPER_FLAGS_1(load_40x_pit, TCG_CALL_NO_RWG, tl, env)
+DEF_HELPER_FLAGS_2(store_40x_pit, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
-DEF_HELPER_2(store_booke_tcr, void, env, tl)
-DEF_HELPER_2(store_booke_tsr, void, env, tl)
+DEF_HELPER_FLAGS_2(store_booke_tcr, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_booke_tsr, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/5] ppc: Improve flags for helpers loading/writing the time facilities

[Richard Henderson]

On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> Those helpers never load from or store to the TCG tracked environment,
> not do they generate synchronous exceptions (they might generate an

s/not/nor/

> asynchronous interrupt but that's not an issue here).
>
> So we can make them all use TCG_CALL_NO_RWG
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/helper.h | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 3/5] ppc: Improve the exception helpers flags

[Benjamin Herrenschmidt]

They generate exceptions, but they don't update the environment

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/helper.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index b0c1db9..d83086e 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -1,5 +1,5 @@
-DEF_HELPER_3(raise_exception_err, void, env, i32, i32)
-DEF_HELPER_2(raise_exception, void, env, i32)
+DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, void, env, i32, i32)
+DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, void, env, i32)
 DEF_HELPER_4(tw, void, env, tl, tl, i32)
 #if defined(TARGET_PPC64)
 DEF_HELPER_4(td, void, env, tl, tl, i32)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/5] ppc: Improve the exception helpers flags

[Richard Henderson]

On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> They generate exceptions, but they don't update the environment
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/helper.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 4/5] ppc: Improve a few more helper flags

[Benjamin Herrenschmidt]

Mostly turn "store" type of helpers into TCG_CALL_NO_WG because
they can take exceptions. Also fixup_thrm doesn't read nor write
the tracked environment.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/helper.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d83086e..dcf3f95 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -1,8 +1,8 @@
 DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, void, env, i32, i32)
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_4(tw, void, env, tl, tl, i32)
+DEF_HELPER_FLAGS_4(tw, TCG_CALL_NO_WG, void, env, tl, tl, i32)
 #if defined(TARGET_PPC64)
-DEF_HELPER_4(td, void, env, tl, tl, i32)
+DEF_HELPER_FLAGS_4(td, TCG_CALL_NO_WG, void, env, tl, tl, i32)
 #endif
 #if !defined(CONFIG_USER_ONLY)
 DEF_HELPER_2(store_msr, void, env, tl)
@@ -22,12 +22,12 @@ DEF_HELPER_1(check_tlb_flush, void, env)
 #endif
 
 DEF_HELPER_3(lmw, void, env, tl, i32)
-DEF_HELPER_3(stmw, void, env, tl, i32)
+DEF_HELPER_FLAGS_3(stmw, TCG_CALL_NO_WG, void, env, tl, i32)
 DEF_HELPER_4(lsw, void, env, tl, i32, i32)
 DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32)
-DEF_HELPER_4(stsw, void, env, tl, i32, i32)
-DEF_HELPER_3(dcbz, void, env, tl, i32)
-DEF_HELPER_2(icbi, void, env, tl)
+DEF_HELPER_FLAGS_4(stsw, TCG_CALL_NO_WG, void, env, tl, i32, i32)
+DEF_HELPER_FLAGS_3(dcbz, TCG_CALL_NO_WG, void, env, tl, i32)
+DEF_HELPER_FLAGS_2(icbi, TCG_CALL_NO_WG, void, env, tl)
 DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)
 
 #if defined(TARGET_PPC64)
@@ -690,4 +690,4 @@ DEF_HELPER_4(dscli, void, env, fprp, fprp, i32)
 DEF_HELPER_4(dscliq, void, env, fprp, fprp, i32)
 
 DEF_HELPER_1(tbegin, void, env)
-DEF_HELPER_1(fixup_thrm, void, env)
+DEF_HELPER_FLAGS_1(fixup_thrm, TCG_CALL_NO_RWG, void, env)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 4/5] ppc: Improve a few more helper flags

[Richard Henderson]

On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> Mostly turn "store" type of helpers into TCG_CALL_NO_WG because
> they can take exceptions. Also fixup_thrm doesn't read nor write
> the tracked environment.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/helper.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[Benjamin Herrenschmidt]

Translate most conditions to TCG conditions and avoid the helper
for most of the common cases.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/translate.c | 168 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 132 insertions(+), 36 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 47eb9ed..561976f 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
 
 /***                                Trap                                   ***/
 
-/* Check for unconditional traps (always or never) */
-static bool check_unconditional_trap(DisasContext *ctx)
+static int TO2tcg[32] = {
+        TCG_COND_NEVER, /* no condition */
+        TCG_COND_GTU,   /* 0x01 u> */
+        TCG_COND_LTU,   /* 0x02 u< */
+        TCG_COND_NE,    /* 0x03 u< or u> -> NE */
+        TCG_COND_EQ,    /* 0x04 = */
+        TCG_COND_GEU,   /* 0x05 u> or = */
+        TCG_COND_LEU,   /* 0x06 u< or = */
+        TCG_COND_ALWAYS,/* 0x07 u< or u> or = -> ALWAYS */
+        TCG_COND_GT,    /* 0x08 > */
+        -1,             /* 0x09 > or u> -> weird */
+        -1,             /* 0x0a > or u< -> weird */
+        -1,             /* 0x0b > or u< or u> -> weird */
+        TCG_COND_GE,    /* 0x0c > or = */
+        -1,             /* 0x0d > or = or u> */
+        -1,             /* 0x0e > or = or u< */
+        -1,             /* 0x0f > or = or u> or u< */
+        TCG_COND_LT,    /* 0x10 < */
+        -1,             /* 0x11 < or u> -> weird */
+        -1,             /* 0x12 < or u< -> weird */
+        -1,             /* 0x13 < or u< or u> -> weird */
+        TCG_COND_LE,    /* 0x14 < or = */
+        -1,             /* 0x15 < or = or u> -> weird */
+        -1,             /* 0x16 < or = or u< -> weird */
+        TCG_COND_ALWAYS,/* 0x17 < or = or u< or u> -> ALWAYS */
+        TCG_COND_NE,    /* 0x18 < or > -> NE */
+        -1,             /* 0x19 < or > or u> -> weird */
+        -1,             /* 0x1a < or > or u< -> weird */
+        -1,             /* 0x1b < or > or u> or u< -> weird */
+        TCG_COND_ALWAYS,/* 0x1c < or > or = -> ALWAYS */
+        TCG_COND_ALWAYS,/* 0x1d < or > or = or u> -> ALWAYS */
+        TCG_COND_ALWAYS,/* 0x1e < or > or = or u< -> ALWAYS */
+        TCG_COND_ALWAYS,/* 0x1f < or > or = or u< -> ALWAYS */
+};
+
+#define TRAP_UNCOND     (-1)
+#define TRAP_HELPER     (-2)
+
+static int precheck_trap(DisasContext *ctx)
 {
-    /* Trap never */
-    if (TO(ctx->opcode) == 0) {
-        return true;
+    int cond = TO2tcg[TO(ctx->opcode)];
+
+    /* Weird traps go to helper */
+    if (cond < 0) {
+        return TRAP_HELPER;
     }
-    /* Trap always */
-    if (TO(ctx->opcode) == 31) {
+    /* Unconditionals */
+    if (cond == TCG_COND_ALWAYS) {
         gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
-        return true;
+        return TRAP_UNCOND;
     }
-    return false;
+    if (cond == TCG_COND_NEVER) {
+        return TRAP_UNCOND;
+    }
+    /* Invert the condition as we branch over the exception when the
+     * condition is *not* met
+     */
+    return tcg_invert_cond(cond);
+}
+
+static void gen_trap(DisasContext *ctx)
+{
+    TCGv_i32 t0, t1;
+
+    t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
+    t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
+    gen_update_nip(ctx, ctx->nip - 4);
+    gen_helper_raise_exception_err(cpu_env, t0, t1);
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i32(t1);
 }
 
 /* tw */
 static void gen_tw(DisasContext *ctx)
 {
-    TCGv_i32 t0;
+    int cond = precheck_trap(ctx);
+    TCGLabel *l1;
+    TCGv t0;
+    TCGv t1;
 
-    if (check_unconditional_trap(ctx)) {
+    if (cond == TRAP_UNCOND) {
+        return;
+    } else if (cond == TRAP_HELPER) {
+        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
+        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
+                      cpu_gpr[rB(ctx->opcode)], trapop);
+        tcg_temp_free_i32(trapop);
         return;
     }
-    t0 = tcg_const_i32(TO(ctx->opcode));
-    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
-                  t0);
-    tcg_temp_free_i32(t0);
+    l1 = gen_new_label();
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
+    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
+    tcg_gen_brcond_tl(cond, t0, t1, l1);
+    gen_trap(ctx);
+    gen_set_label(l1);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
 }
 
 /* twi */
 static void gen_twi(DisasContext *ctx)
 {
+    int cond = precheck_trap(ctx);
+    TCGLabel *l1;
     TCGv t0;
-    TCGv_i32 t1;
 
-    if (check_unconditional_trap(ctx)) {
+    if (cond == TRAP_UNCOND) {
+        return;
+    } else if (cond == TRAP_HELPER) {
+        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
+        t0 = tcg_const_tl(SIMM(ctx->opcode));
+        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
+        tcg_temp_free_i32(trapop);
+        tcg_temp_free(t0);
         return;
     }
-    t0 = tcg_const_tl(SIMM(ctx->opcode));
-    t1 = tcg_const_i32(TO(ctx->opcode));
-    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
+    l1 = gen_new_label();
+    t0 = tcg_temp_new();
+    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
+    tcg_gen_brcondi_tl(cond, t0, SIMM(ctx->opcode), l1);
+    gen_trap(ctx);
+    gen_set_label(l1);
     tcg_temp_free(t0);
-    tcg_temp_free_i32(t1);
 }
 
 #if defined(TARGET_PPC64)
 /* td */
 static void gen_td(DisasContext *ctx)
 {
-    TCGv_i32 t0;
+    int cond = precheck_trap(ctx);
+    TCGLabel *l1;
 
-    if (check_unconditional_trap(ctx)) {
+    if (cond == TRAP_UNCOND) {
+        return;
+    } else if (cond == TRAP_HELPER) {
+        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
+        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)],
+                      cpu_gpr[rB(ctx->opcode)], trapop);
+        tcg_temp_free_i32(trapop);
         return;
     }
-    t0 = tcg_const_i32(TO(ctx->opcode));
-    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
-                  t0);
-    tcg_temp_free_i32(t0);
+    l1 = gen_new_label();
+    tcg_gen_brcond_tl(cond, cpu_gpr[rA(ctx->opcode)],
+                      cpu_gpr[rB(ctx->opcode)], l1);
+    gen_trap(ctx);
+    gen_set_label(l1);
 }
 
 /* tdi */
 static void gen_tdi(DisasContext *ctx)
 {
-    TCGv t0;
-    TCGv_i32 t1;
+    int cond = precheck_trap(ctx);
+    TCGLabel *l1;
 
-    if (check_unconditional_trap(ctx)) {
+    if (cond == TRAP_UNCOND) {
+        return;
+    } else if (cond == TRAP_HELPER) {
+        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
+        TCGv t0 = tcg_const_tl(SIMM(ctx->opcode));
+        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
+        tcg_temp_free_i32(trapop);
+        tcg_temp_free(t0);
         return;
     }
-    t0 = tcg_const_tl(SIMM(ctx->opcode));
-    t1 = tcg_const_i32(TO(ctx->opcode));
-    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
-    tcg_temp_free(t0);
-    tcg_temp_free_i32(t1);
+    l1 = gen_new_label();
+    tcg_gen_brcondi_tl(cond, cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), l1);
+    gen_trap(ctx);
+    gen_set_label(l1);
 }
-#endif
+#endif /* defined(TARGET_PPC64) */
 
 /***                          Processor control                            ***/
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8592 bytes --]

On Sun, Jul 31, 2016 at 03:13:13PM +1000, Benjamin Herrenschmidt wrote:
> Translate most conditions to TCG conditions and avoid the helper
> for most of the common cases.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

The other patches in the series look good AFAICT, but I'm not really
confident enough in TCG to have a good feeling.  I'm hoping to get an
R-b from rth before merging.

> ---
>  target-ppc/translate.c | 168 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 132 insertions(+), 36 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 47eb9ed..561976f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
>  
>  /***                                Trap                                   ***/
>  
> -/* Check for unconditional traps (always or never) */
> -static bool check_unconditional_trap(DisasContext *ctx)
> +static int TO2tcg[32] = {
> +        TCG_COND_NEVER, /* no condition */
> +        TCG_COND_GTU,   /* 0x01 u> */
> +        TCG_COND_LTU,   /* 0x02 u< */
> +        TCG_COND_NE,    /* 0x03 u< or u> -> NE */
> +        TCG_COND_EQ,    /* 0x04 = */
> +        TCG_COND_GEU,   /* 0x05 u> or = */
> +        TCG_COND_LEU,   /* 0x06 u< or = */
> +        TCG_COND_ALWAYS,/* 0x07 u< or u> or = -> ALWAYS */
> +        TCG_COND_GT,    /* 0x08 > */
> +        -1,             /* 0x09 > or u> -> weird */
> +        -1,             /* 0x0a > or u< -> weird */
> +        -1,             /* 0x0b > or u< or u> -> weird */
> +        TCG_COND_GE,    /* 0x0c > or = */
> +        -1,             /* 0x0d > or = or u> */
> +        -1,             /* 0x0e > or = or u< */
> +        -1,             /* 0x0f > or = or u> or u< */
> +        TCG_COND_LT,    /* 0x10 < */
> +        -1,             /* 0x11 < or u> -> weird */
> +        -1,             /* 0x12 < or u< -> weird */
> +        -1,             /* 0x13 < or u< or u> -> weird */
> +        TCG_COND_LE,    /* 0x14 < or = */
> +        -1,             /* 0x15 < or = or u> -> weird */
> +        -1,             /* 0x16 < or = or u< -> weird */
> +        TCG_COND_ALWAYS,/* 0x17 < or = or u< or u> -> ALWAYS */
> +        TCG_COND_NE,    /* 0x18 < or > -> NE */
> +        -1,             /* 0x19 < or > or u> -> weird */
> +        -1,             /* 0x1a < or > or u< -> weird */
> +        -1,             /* 0x1b < or > or u> or u< -> weird */
> +        TCG_COND_ALWAYS,/* 0x1c < or > or = -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1d < or > or = or u> -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1e < or > or = or u< -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1f < or > or = or u< -> ALWAYS */
> +};
> +
> +#define TRAP_UNCOND     (-1)
> +#define TRAP_HELPER     (-2)
> +
> +static int precheck_trap(DisasContext *ctx)
>  {
> -    /* Trap never */
> -    if (TO(ctx->opcode) == 0) {
> -        return true;
> +    int cond = TO2tcg[TO(ctx->opcode)];
> +
> +    /* Weird traps go to helper */
> +    if (cond < 0) {
> +        return TRAP_HELPER;
>      }
> -    /* Trap always */
> -    if (TO(ctx->opcode) == 31) {
> +    /* Unconditionals */
> +    if (cond == TCG_COND_ALWAYS) {
>          gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
> -        return true;
> +        return TRAP_UNCOND;
>      }
> -    return false;
> +    if (cond == TCG_COND_NEVER) {
> +        return TRAP_UNCOND;
> +    }
> +    /* Invert the condition as we branch over the exception when the
> +     * condition is *not* met
> +     */
> +    return tcg_invert_cond(cond);
> +}
> +
> +static void gen_trap(DisasContext *ctx)
> +{
> +    TCGv_i32 t0, t1;
> +
> +    t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
> +    t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
> +    gen_update_nip(ctx, ctx->nip - 4);
> +    gen_helper_raise_exception_err(cpu_env, t0, t1);
> +    tcg_temp_free_i32(t0);
> +    tcg_temp_free_i32(t1);
>  }
>  
>  /* tw */
>  static void gen_tw(DisasContext *ctx)
>  {
> -    TCGv_i32 t0;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
> +    TCGv t0;
> +    TCGv t1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], trapop);
> +        tcg_temp_free_i32(trapop);
>          return;
>      }
> -    t0 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                  t0);
> -    tcg_temp_free_i32(t0);
> +    l1 = gen_new_label();
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> +    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
> +    tcg_gen_brcond_tl(cond, t0, t1, l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
> +    tcg_temp_free(t0);
> +    tcg_temp_free(t1);
>  }
>  
>  /* twi */
>  static void gen_twi(DisasContext *ctx)
>  {
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>      TCGv t0;
> -    TCGv_i32 t1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        t0 = tcg_const_tl(SIMM(ctx->opcode));
> +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
> +        tcg_temp_free_i32(trapop);
> +        tcg_temp_free(t0);
>          return;
>      }
> -    t0 = tcg_const_tl(SIMM(ctx->opcode));
> -    t1 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
> +    l1 = gen_new_label();
> +    t0 = tcg_temp_new();
> +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> +    tcg_gen_brcondi_tl(cond, t0, SIMM(ctx->opcode), l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>      tcg_temp_free(t0);
> -    tcg_temp_free_i32(t1);
>  }
>  
>  #if defined(TARGET_PPC64)
>  /* td */
>  static void gen_td(DisasContext *ctx)
>  {
> -    TCGv_i32 t0;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], trapop);
> +        tcg_temp_free_i32(trapop);
>          return;
>      }
> -    t0 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                  t0);
> -    tcg_temp_free_i32(t0);
> +    l1 = gen_new_label();
> +    tcg_gen_brcond_tl(cond, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>  }
>  
>  /* tdi */
>  static void gen_tdi(DisasContext *ctx)
>  {
> -    TCGv t0;
> -    TCGv_i32 t1;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        TCGv t0 = tcg_const_tl(SIMM(ctx->opcode));
> +        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
> +        tcg_temp_free_i32(trapop);
> +        tcg_temp_free(t0);
>          return;
>      }
> -    t0 = tcg_const_tl(SIMM(ctx->opcode));
> -    t1 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
> -    tcg_temp_free(t0);
> -    tcg_temp_free_i32(t1);
> +    l1 = gen_new_label();
> +    tcg_gen_brcondi_tl(cond, cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>  }
> -#endif
> +#endif /* defined(TARGET_PPC64) */
>  
>  /***                          Processor control                            ***/
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[Richard Henderson]

On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> Translate most conditions to TCG conditions and avoid the helper
> for most of the common cases.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/translate.c | 168 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 132 insertions(+), 36 deletions(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 47eb9ed..561976f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
>
>  /***                                Trap                                   ***/
>
> -/* Check for unconditional traps (always or never) */
> -static bool check_unconditional_trap(DisasContext *ctx)
> +static int TO2tcg[32] = {

Should be const.

> +        -1,             /* 0x0f > or = or u> or u< */
> +        -1,             /* 0x13 < or u< or u> -> weird */
> +        -1,             /* 0x19 < or > or u> -> weird */
> +        -1,             /* 0x1a < or > or u< -> weird */
> +        -1,             /* 0x1b < or > or u> or u< -> weird */

Not that it matters much, but when you have both < and >, or both u< and u>, 
you can collapse to NE, no matter what else is there.

> +#define TRAP_UNCOND     (-1)
> +#define TRAP_HELPER     (-2)
> +
> +static int precheck_trap(DisasContext *ctx)
>  {
> -    /* Trap never */
> -    if (TO(ctx->opcode) == 0) {
> -        return true;
> +    int cond = TO2tcg[TO(ctx->opcode)];
> +
> +    /* Weird traps go to helper */
> +    if (cond < 0) {
> +        return TRAP_HELPER;
>      }
> -    /* Trap always */
> -    if (TO(ctx->opcode) == 31) {
> +    /* Unconditionals */
> +    if (cond == TCG_COND_ALWAYS) {
>          gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
> -        return true;
> +        return TRAP_UNCOND;
>      }
> -    return false;
> +    if (cond == TCG_COND_NEVER) {
> +        return TRAP_UNCOND;
> +    }
> +    /* Invert the condition as we branch over the exception when the
> +     * condition is *not* met
> +     */
> +    return tcg_invert_cond(cond);
> +}
> +
> +static void gen_trap(DisasContext *ctx)
> +{
> +    TCGv_i32 t0, t1;
> +
> +    t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
> +    t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
> +    gen_update_nip(ctx, ctx->nip - 4);
> +    gen_helper_raise_exception_err(cpu_env, t0, t1);

gen_exception_err, as with the unconditional trap?

>  static void gen_tw(DisasContext *ctx)
>  {
> -    TCGv_i32 t0;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
> +    TCGv t0;
> +    TCGv t1;
>
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], trapop);
> +        tcg_temp_free_i32(trapop);
>          return;
>      }
> -    t0 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                  t0);
> -    tcg_temp_free_i32(t0);
> +    l1 = gen_new_label();
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> +    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
> +    tcg_gen_brcond_tl(cond, t0, t1, l1);

It's just as easy, and perhaps better, to truncate to 32 bits instead of 
extending to TL bits.


r~

Re: [Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[Benjamin Herrenschmidt]

On Sat, 2016-08-06 at 14:33 +0530, Richard Henderson wrote:
> On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> > 
> > Translate most conditions to TCG conditions and avoid the helper
> > for most of the common cases.
> > 
> > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >  target-ppc/translate.c | 168 ++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 132 insertions(+), 36 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 47eb9ed..561976f 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
> > 
> >  /***                                Trap                                   ***/
> > 
> > -/* Check for unconditional traps (always or never) */
> > -static bool check_unconditional_trap(DisasContext *ctx)
> > +static int TO2tcg[32] = {
> 
> Should be const.

Indeed.

> > 
> > +        -1,             /* 0x0f > or = or u> or u< */
> > +        -1,             /* 0x13 < or u< or u> -> weird */
> > +        -1,             /* 0x19 < or > or u> -> weird */
> > +        -1,             /* 0x1a < or > or u< -> weird */
> > +        -1,             /* 0x1b < or > or u> or u< -> weird */
> 
> Not that it matters much, but when you have both < and >, or both u< and u>, 
> you can collapse to NE, no matter what else is there.

Not necessarily, if = is there too it beomes always ;-) But as
a rule I bail to the helper for anything that mixes signed and
non-signed, in part bcs I was thinking of tracing them in case
anybody uses such oddball constructs. I expect them to never
happen unless somebody has a bug.

> > 
> > +#define TRAP_UNCOND     (-1)
> > +#define TRAP_HELPER     (-2)
> > +
> > +static int precheck_trap(DisasContext *ctx)
> >  {
> > -    /* Trap never */
> > -    if (TO(ctx->opcode) == 0) {
> > -        return true;
> > +    int cond = TO2tcg[TO(ctx->opcode)];
> > +
> > +    /* Weird traps go to helper */
> > +    if (cond < 0) {
> > +        return TRAP_HELPER;
> >      }
> > -    /* Trap always */
> > -    if (TO(ctx->opcode) == 31) {
> > +    /* Unconditionals */
> > +    if (cond == TCG_COND_ALWAYS) {
> >          gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
> > -        return true;
> > +        return TRAP_UNCOND;
> >      }
> > -    return false;
> > +    if (cond == TCG_COND_NEVER) {
> > +        return TRAP_UNCOND;
> > +    }
> > +    /* Invert the condition as we branch over the exception when the
> > +     * condition is *not* met
> > +     */
> > +    return tcg_invert_cond(cond);
> > +}
> > +
> > +static void gen_trap(DisasContext *ctx)
> > +{
> > +    TCGv_i32 t0, t1;
> > +
> > +    t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
> > +    t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
> > +    gen_update_nip(ctx, ctx->nip - 4);
> > +    gen_helper_raise_exception_err(cpu_env, t0, t1);
> 
> gen_exception_err, as with the unconditional trap?

No, we call it from gen_tw etc... but we may also branch
around that call, so we should not exit the TB with an
exception in the context.

> > 
> >  static void gen_tw(DisasContext *ctx)
> >  {
> > -    TCGv_i32 t0;
> > +    int cond = precheck_trap(ctx);
> > +    TCGLabel *l1;
> > +    TCGv t0;
> > +    TCGv t1;
> > 
> > -    if (check_unconditional_trap(ctx)) {
> > +    if (cond == TRAP_UNCOND) {
> > +        return;
> > +    } else if (cond == TRAP_HELPER) {
> > +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> > +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> > +                      cpu_gpr[rB(ctx->opcode)], trapop);
> > +        tcg_temp_free_i32(trapop);
> >          return;
> >      }
> > -    t0 = tcg_const_i32(TO(ctx->opcode));
> > -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> > -                  t0);
> > -    tcg_temp_free_i32(t0);
> > +    l1 = gen_new_label();
> > +    t0 = tcg_temp_new();
> > +    t1 = tcg_temp_new();
> > +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> > +    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
> > +    tcg_gen_brcond_tl(cond, t0, t1, l1);
> 
> It's just as easy, and perhaps better, to truncate to 32 bits instead of 
> extending to TL bits.

Ok, I'll have a look. I'm not actually *that* familiar with the various
ops in the IR, but I shall figure it out :-)

Cheers,
Ben.

> 
> r~

Re: [Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8510 bytes --]

On Sun, Jul 31, 2016 at 03:13:13PM +1000, Benjamin Herrenschmidt wrote:
> Translate most conditions to TCG conditions and avoid the helper
> for most of the common cases.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/translate.c | 168 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 132 insertions(+), 36 deletions(-)

I've merged 1-4 of this series into ppc-for-2.8.  I'm not really clear
whether a change is still needed on patch 5, so please resend either
way.

> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 47eb9ed..561976f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
>  
>  /***                                Trap                                   ***/
>  
> -/* Check for unconditional traps (always or never) */
> -static bool check_unconditional_trap(DisasContext *ctx)
> +static int TO2tcg[32] = {
> +        TCG_COND_NEVER, /* no condition */
> +        TCG_COND_GTU,   /* 0x01 u> */
> +        TCG_COND_LTU,   /* 0x02 u< */
> +        TCG_COND_NE,    /* 0x03 u< or u> -> NE */
> +        TCG_COND_EQ,    /* 0x04 = */
> +        TCG_COND_GEU,   /* 0x05 u> or = */
> +        TCG_COND_LEU,   /* 0x06 u< or = */
> +        TCG_COND_ALWAYS,/* 0x07 u< or u> or = -> ALWAYS */
> +        TCG_COND_GT,    /* 0x08 > */
> +        -1,             /* 0x09 > or u> -> weird */
> +        -1,             /* 0x0a > or u< -> weird */
> +        -1,             /* 0x0b > or u< or u> -> weird */
> +        TCG_COND_GE,    /* 0x0c > or = */
> +        -1,             /* 0x0d > or = or u> */
> +        -1,             /* 0x0e > or = or u< */
> +        -1,             /* 0x0f > or = or u> or u< */
> +        TCG_COND_LT,    /* 0x10 < */
> +        -1,             /* 0x11 < or u> -> weird */
> +        -1,             /* 0x12 < or u< -> weird */
> +        -1,             /* 0x13 < or u< or u> -> weird */
> +        TCG_COND_LE,    /* 0x14 < or = */
> +        -1,             /* 0x15 < or = or u> -> weird */
> +        -1,             /* 0x16 < or = or u< -> weird */
> +        TCG_COND_ALWAYS,/* 0x17 < or = or u< or u> -> ALWAYS */
> +        TCG_COND_NE,    /* 0x18 < or > -> NE */
> +        -1,             /* 0x19 < or > or u> -> weird */
> +        -1,             /* 0x1a < or > or u< -> weird */
> +        -1,             /* 0x1b < or > or u> or u< -> weird */
> +        TCG_COND_ALWAYS,/* 0x1c < or > or = -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1d < or > or = or u> -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1e < or > or = or u< -> ALWAYS */
> +        TCG_COND_ALWAYS,/* 0x1f < or > or = or u< -> ALWAYS */
> +};
> +
> +#define TRAP_UNCOND     (-1)
> +#define TRAP_HELPER     (-2)
> +
> +static int precheck_trap(DisasContext *ctx)
>  {
> -    /* Trap never */
> -    if (TO(ctx->opcode) == 0) {
> -        return true;
> +    int cond = TO2tcg[TO(ctx->opcode)];
> +
> +    /* Weird traps go to helper */
> +    if (cond < 0) {
> +        return TRAP_HELPER;
>      }
> -    /* Trap always */
> -    if (TO(ctx->opcode) == 31) {
> +    /* Unconditionals */
> +    if (cond == TCG_COND_ALWAYS) {
>          gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
> -        return true;
> +        return TRAP_UNCOND;
>      }
> -    return false;
> +    if (cond == TCG_COND_NEVER) {
> +        return TRAP_UNCOND;
> +    }
> +    /* Invert the condition as we branch over the exception when the
> +     * condition is *not* met
> +     */
> +    return tcg_invert_cond(cond);
> +}
> +
> +static void gen_trap(DisasContext *ctx)
> +{
> +    TCGv_i32 t0, t1;
> +
> +    t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
> +    t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
> +    gen_update_nip(ctx, ctx->nip - 4);
> +    gen_helper_raise_exception_err(cpu_env, t0, t1);
> +    tcg_temp_free_i32(t0);
> +    tcg_temp_free_i32(t1);
>  }
>  
>  /* tw */
>  static void gen_tw(DisasContext *ctx)
>  {
> -    TCGv_i32 t0;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
> +    TCGv t0;
> +    TCGv t1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], trapop);
> +        tcg_temp_free_i32(trapop);
>          return;
>      }
> -    t0 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                  t0);
> -    tcg_temp_free_i32(t0);
> +    l1 = gen_new_label();
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> +    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
> +    tcg_gen_brcond_tl(cond, t0, t1, l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
> +    tcg_temp_free(t0);
> +    tcg_temp_free(t1);
>  }
>  
>  /* twi */
>  static void gen_twi(DisasContext *ctx)
>  {
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>      TCGv t0;
> -    TCGv_i32 t1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        t0 = tcg_const_tl(SIMM(ctx->opcode));
> +        gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
> +        tcg_temp_free_i32(trapop);
> +        tcg_temp_free(t0);
>          return;
>      }
> -    t0 = tcg_const_tl(SIMM(ctx->opcode));
> -    t1 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
> +    l1 = gen_new_label();
> +    t0 = tcg_temp_new();
> +    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> +    tcg_gen_brcondi_tl(cond, t0, SIMM(ctx->opcode), l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>      tcg_temp_free(t0);
> -    tcg_temp_free_i32(t1);
>  }
>  
>  #if defined(TARGET_PPC64)
>  /* td */
>  static void gen_td(DisasContext *ctx)
>  {
> -    TCGv_i32 t0;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], trapop);
> +        tcg_temp_free_i32(trapop);
>          return;
>      }
> -    t0 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                  t0);
> -    tcg_temp_free_i32(t0);
> +    l1 = gen_new_label();
> +    tcg_gen_brcond_tl(cond, cpu_gpr[rA(ctx->opcode)],
> +                      cpu_gpr[rB(ctx->opcode)], l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>  }
>  
>  /* tdi */
>  static void gen_tdi(DisasContext *ctx)
>  {
> -    TCGv t0;
> -    TCGv_i32 t1;
> +    int cond = precheck_trap(ctx);
> +    TCGLabel *l1;
>  
> -    if (check_unconditional_trap(ctx)) {
> +    if (cond == TRAP_UNCOND) {
> +        return;
> +    } else if (cond == TRAP_HELPER) {
> +        TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> +        TCGv t0 = tcg_const_tl(SIMM(ctx->opcode));
> +        gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, trapop);
> +        tcg_temp_free_i32(trapop);
> +        tcg_temp_free(t0);
>          return;
>      }
> -    t0 = tcg_const_tl(SIMM(ctx->opcode));
> -    t1 = tcg_const_i32(TO(ctx->opcode));
> -    gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1);
> -    tcg_temp_free(t0);
> -    tcg_temp_free_i32(t1);
> +    l1 = gen_new_label();
> +    tcg_gen_brcondi_tl(cond, cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode), l1);
> +    gen_trap(ctx);
> +    gen_set_label(l1);
>  }
> -#endif
> +#endif /* defined(TARGET_PPC64) */
>  
>  /***                          Processor control                            ***/
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/5] ppc: Improve generation of conditional traps

[Benjamin Herrenschmidt]

On Tue, 2016-08-09 at 12:07 +1000, David Gibson wrote:
> On Sun, Jul 31, 2016 at 03:13:13PM +1000, Benjamin Herrenschmidt
> wrote:
> > 
> > Translate most conditions to TCG conditions and avoid the helper
> > for most of the common cases.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >  target-ppc/translate.c | 168
> > ++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 132 insertions(+), 36 deletions(-)
> 
> I've merged 1-4 of this series into ppc-for-2.8.  I'm not really
> clear
> whether a change is still needed on patch 5, so please resend either
> way.

What you merged works fine. Richard suggestions are refinements
I can do separately and apply on top.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 1/5] ppc: Don't generate dead code on unconditional branches

[Richard Henderson]

On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> We are always generating the "else" case of the condition even when
> generating an unconditional branch that will never hit it.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/translate.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~