[PATCH 0/4] target/openrisc: Use tcg_constant_*

[PATCH 0/4] target/openrisc: Use tcg_constant_*

[Richard Henderson]

Replace use of tcg_const_*, which makes a copy into a temp
which must be freed, with direct use of the constant.

r~

Richard Henderson (4):
  target/openrisc: Use tcg_constant_*
  target/openrisc: Use tcg_constant_tl for dc->R0
  target/openrisc: Cache constant 0 in DisasContext
  target/openrisc: Use dc->zero in gen_add, gen_addc

 target/openrisc/translate.c | 68 +++++++++++--------------------------
 1 file changed, 19 insertions(+), 49 deletions(-)

-- 
2.25.1

[PATCH 1/4] target/openrisc: Use tcg_constant_*

[Richard Henderson]

Replace uses of tcg_const_* with the allocate and free close together.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/translate.c | 42 ++++++++-----------------------------
 1 file changed, 9 insertions(+), 33 deletions(-)

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index a9c81f8bd5..1e437d2f9d 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -130,9 +130,7 @@ void openrisc_translate_init(void)
 
 static void gen_exception(DisasContext *dc, unsigned int excp)
 {
-    TCGv_i32 tmp = tcg_const_i32(excp);
-    gen_helper_exception(cpu_env, tmp);
-    tcg_temp_free_i32(tmp);
+    gen_helper_exception(cpu_env, tcg_constant_i32(excp));
 }
 
 static void gen_illegal_exception(DisasContext *dc)
@@ -539,13 +537,11 @@ static bool trans_l_extbz(DisasContext *dc, arg_da *a)
 
 static bool trans_l_cmov(DisasContext *dc, arg_dab *a)
 {
-    TCGv zero;
+    TCGv zero = tcg_constant_tl(0);
 
     check_r0_write(dc, a->d);
-    zero = tcg_const_tl(0);
     tcg_gen_movcond_tl(TCG_COND_NE, cpu_R(dc, a->d), cpu_sr_f, zero,
                        cpu_R(dc, a->a), cpu_R(dc, a->b));
-    tcg_temp_free(zero);
     return true;
 }
 
@@ -633,15 +629,11 @@ static bool trans_l_jal(DisasContext *dc, arg_l_jal *a)
 static void do_bf(DisasContext *dc, arg_l_bf *a, TCGCond cond)
 {
     target_ulong tmp_pc = dc->base.pc_next + a->n * 4;
-    TCGv t_next = tcg_const_tl(dc->base.pc_next + 8);
-    TCGv t_true = tcg_const_tl(tmp_pc);
-    TCGv t_zero = tcg_const_tl(0);
+    TCGv t_next = tcg_constant_tl(dc->base.pc_next + 8);
+    TCGv t_true = tcg_constant_tl(tmp_pc);
+    TCGv t_zero = tcg_constant_tl(0);
 
     tcg_gen_movcond_tl(cond, jmp_pc, cpu_sr_f, t_zero, t_true, t_next);
-
-    tcg_temp_free(t_next);
-    tcg_temp_free(t_true);
-    tcg_temp_free(t_zero);
     dc->delayed_branch = 2;
 }
 
@@ -814,44 +806,28 @@ static bool trans_l_adrp(DisasContext *dc, arg_l_adrp *a)
 
 static bool trans_l_addi(DisasContext *dc, arg_rri *a)
 {
-    TCGv t0;
-
     check_r0_write(dc, a->d);
-    t0 = tcg_const_tl(a->i);
-    gen_add(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), t0);
-    tcg_temp_free(t0);
+    gen_add(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), tcg_constant_tl(a->i));
     return true;
 }
 
 static bool trans_l_addic(DisasContext *dc, arg_rri *a)
 {
-    TCGv t0;
-
     check_r0_write(dc, a->d);
-    t0 = tcg_const_tl(a->i);
-    gen_addc(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), t0);
-    tcg_temp_free(t0);
+    gen_addc(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), tcg_constant_tl(a->i));
     return true;
 }
 
 static bool trans_l_muli(DisasContext *dc, arg_rri *a)
 {
-    TCGv t0;
-
     check_r0_write(dc, a->d);
-    t0 = tcg_const_tl(a->i);
-    gen_mul(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), t0);
-    tcg_temp_free(t0);
+    gen_mul(dc, cpu_R(dc, a->d), cpu_R(dc, a->a), tcg_constant_tl(a->i));
     return true;
 }
 
 static bool trans_l_maci(DisasContext *dc, arg_l_maci *a)
 {
-    TCGv t0;
-
-    t0 = tcg_const_tl(a->i);
-    gen_mac(dc, cpu_R(dc, a->a), t0);
-    tcg_temp_free(t0);
+    gen_mac(dc, cpu_R(dc, a->a), tcg_constant_tl(a->i));
     return true;
 }
 
-- 
2.25.1

Re: [PATCH 1/4] target/openrisc: Use tcg_constant_*

[Philippe Mathieu-Daudé]

On 7/8/21 11:37 PM, Richard Henderson wrote:
> Replace uses of tcg_const_* with the allocate and free close together.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/openrisc/translate.c | 42 ++++++++-----------------------------
>  1 file changed, 9 insertions(+), 33 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/4] target/openrisc: Use tcg_constant_*

[Stafford Horne]

On Thu, Jul 08, 2021 at 02:37:51PM -0700, Richard Henderson wrote:
> Replace uses of tcg_const_* with the allocate and free close together.

I read this as tcg_const_* is being replaced with allocate and free.  Perhaps we
can say:

Replace uses of tcg_const_* allocate and free close together with
tcg_constant_*.

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Stafford Horne <shorne@gmail.com>

[PATCH 2/4] target/openrisc: Use tcg_constant_tl for dc->R0

[Richard Henderson]

The temp allocated for tcg_const_tl is auto-freed at branches,
but pure constants are not.  So we can remove the extra hoop
jumping in trans_l_swa.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/translate.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 1e437d2f9d..96df513fd3 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -733,12 +733,6 @@ static bool trans_l_swa(DisasContext *dc, arg_store *a)
     ea = tcg_temp_new();
     tcg_gen_addi_tl(ea, cpu_R(dc, a->a), a->i);
 
-    /* For TB_FLAGS_R0_0, the branch below invalidates the temporary assigned
-       to cpu_regs[0].  Since l.swa is quite often immediately followed by a
-       branch, don't bother reallocating; finish the TB using the "real" R0.
-       This also takes care of RB input across the branch.  */
-    dc->R0 = cpu_regs[0];
-
     lab_fail = gen_new_label();
     lab_done = gen_new_label();
     tcg_gen_brcond_tl(TCG_COND_NE, ea, cpu_lock_addr, lab_fail);
@@ -746,7 +740,7 @@ static bool trans_l_swa(DisasContext *dc, arg_store *a)
 
     val = tcg_temp_new();
     tcg_gen_atomic_cmpxchg_tl(val, cpu_lock_addr, cpu_lock_value,
-                              cpu_regs[a->b], dc->mem_idx, MO_TEUL);
+                              cpu_R(dc, a->b), dc->mem_idx, MO_TEUL);
     tcg_gen_setcond_tl(TCG_COND_EQ, cpu_sr_f, val, cpu_lock_value);
     tcg_temp_free(val);
 
@@ -1602,7 +1596,7 @@ static void openrisc_tr_tb_start(DisasContextBase *db, CPUState *cs)
     /* Allow the TCG optimizer to see that R0 == 0,
        when it's true, which is the common case.  */
     if (dc->tb_flags & TB_FLAGS_R0_0) {
-        dc->R0 = tcg_const_tl(0);
+        dc->R0 = tcg_constant_tl(0);
     } else {
         dc->R0 = cpu_regs[0];
     }
-- 
2.25.1

Re: [PATCH 2/4] target/openrisc: Use tcg_constant_tl for dc->R0

[Stafford Horne]

On Thu, Jul 08, 2021 at 02:37:52PM -0700, Richard Henderson wrote:
> The temp allocated for tcg_const_tl is auto-freed at branches,
> but pure constants are not.  So we can remove the extra hoop
> jumping in trans_l_swa.

This is nice.

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Stafford Horne <shorne@gmail.com>

[PATCH 3/4] target/openrisc: Cache constant 0 in DisasContext

[Richard Henderson]

We are virtually certain to have fetched constant 0 once, at the
beginning of the TB, so we might as well use it elsewhere.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/translate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 96df513fd3..3d86bdffaa 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -53,6 +53,8 @@ typedef struct DisasContext {
 
     /* The temporary corresponding to register 0 for this compilation.  */
     TCGv R0;
+    /* The constant zero. */
+    TCGv zero;
 } DisasContext;
 
 static inline bool is_user(DisasContext *dc)
@@ -537,10 +539,8 @@ static bool trans_l_extbz(DisasContext *dc, arg_da *a)
 
 static bool trans_l_cmov(DisasContext *dc, arg_dab *a)
 {
-    TCGv zero = tcg_constant_tl(0);
-
     check_r0_write(dc, a->d);
-    tcg_gen_movcond_tl(TCG_COND_NE, cpu_R(dc, a->d), cpu_sr_f, zero,
+    tcg_gen_movcond_tl(TCG_COND_NE, cpu_R(dc, a->d), cpu_sr_f, dc->zero,
                        cpu_R(dc, a->a), cpu_R(dc, a->b));
     return true;
 }
@@ -631,9 +631,8 @@ static void do_bf(DisasContext *dc, arg_l_bf *a, TCGCond cond)
     target_ulong tmp_pc = dc->base.pc_next + a->n * 4;
     TCGv t_next = tcg_constant_tl(dc->base.pc_next + 8);
     TCGv t_true = tcg_constant_tl(tmp_pc);
-    TCGv t_zero = tcg_constant_tl(0);
 
-    tcg_gen_movcond_tl(cond, jmp_pc, cpu_sr_f, t_zero, t_true, t_next);
+    tcg_gen_movcond_tl(cond, jmp_pc, cpu_sr_f, dc->zero, t_true, t_next);
     dc->delayed_branch = 2;
 }
 
@@ -1595,8 +1594,9 @@ static void openrisc_tr_tb_start(DisasContextBase *db, CPUState *cs)
 
     /* Allow the TCG optimizer to see that R0 == 0,
        when it's true, which is the common case.  */
+    dc->zero = tcg_constant_tl(0);
     if (dc->tb_flags & TB_FLAGS_R0_0) {
-        dc->R0 = tcg_constant_tl(0);
+        dc->R0 = dc->zero;
     } else {
         dc->R0 = cpu_regs[0];
     }
-- 
2.25.1

Re: [PATCH 3/4] target/openrisc: Cache constant 0 in DisasContext

[Philippe Mathieu-Daudé]

On 7/8/21 11:37 PM, Richard Henderson wrote:
> We are virtually certain to have fetched constant 0 once, at the
> beginning of the TB, so we might as well use it elsewhere.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/openrisc/translate.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 3/4] target/openrisc: Cache constant 0 in DisasContext

[Stafford Horne]

On Thu, Jul 08, 2021 at 02:37:53PM -0700, Richard Henderson wrote:
> We are virtually certain to have fetched constant 0 once, at the
> beginning of the TB, so we might as well use it elsewhere.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Stafford Horne <shorne@gmail.com>

[PATCH 4/4] target/openrisc: Use dc->zero in gen_add, gen_addc

[Richard Henderson]

We still need the t0 temporary for computing overflow,
but we do not need to initialize it to zero first.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/translate.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 3d86bdffaa..3425fd881e 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -200,10 +200,10 @@ static void gen_ove_cyov(DisasContext *dc)
 
 static void gen_add(DisasContext *dc, TCGv dest, TCGv srca, TCGv srcb)
 {
-    TCGv t0 = tcg_const_tl(0);
+    TCGv t0 = tcg_temp_new();
     TCGv res = tcg_temp_new();
 
-    tcg_gen_add2_tl(res, cpu_sr_cy, srca, t0, srcb, t0);
+    tcg_gen_add2_tl(res, cpu_sr_cy, srca, dc->zero, srcb, dc->zero);
     tcg_gen_xor_tl(cpu_sr_ov, srca, srcb);
     tcg_gen_xor_tl(t0, res, srcb);
     tcg_gen_andc_tl(cpu_sr_ov, t0, cpu_sr_ov);
@@ -217,11 +217,11 @@ static void gen_add(DisasContext *dc, TCGv dest, TCGv srca, TCGv srcb)
 
 static void gen_addc(DisasContext *dc, TCGv dest, TCGv srca, TCGv srcb)
 {
-    TCGv t0 = tcg_const_tl(0);
+    TCGv t0 = tcg_temp_new();
     TCGv res = tcg_temp_new();
 
-    tcg_gen_add2_tl(res, cpu_sr_cy, srca, t0, cpu_sr_cy, t0);
-    tcg_gen_add2_tl(res, cpu_sr_cy, res, cpu_sr_cy, srcb, t0);
+    tcg_gen_add2_tl(res, cpu_sr_cy, srca, dc->zero, cpu_sr_cy, dc->zero);
+    tcg_gen_add2_tl(res, cpu_sr_cy, res, cpu_sr_cy, srcb, dc->zero);
     tcg_gen_xor_tl(cpu_sr_ov, srca, srcb);
     tcg_gen_xor_tl(t0, res, srcb);
     tcg_gen_andc_tl(cpu_sr_ov, t0, cpu_sr_ov);
-- 
2.25.1

Re: [PATCH 4/4] target/openrisc: Use dc->zero in gen_add, gen_addc

[Stafford Horne]

On Thu, Jul 08, 2021 at 02:37:54PM -0700, Richard Henderson wrote:
> We still need the t0 temporary for computing overflow,
> but we do not need to initialize it to zero first.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Stafford Horne <shorne@gmail.com>