diff mbox

[v2,04/11] target-arm: Use setcond and movcond for csel

Message ID 1441216660-8717-5-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Sept. 2, 2015, 5:57 p.m. UTC 
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-arm/translate-a64.c | 87 +++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 36 deletions(-)

Comments

Peter Maydell Sept. 7, 2015, 5:17 p.m. UTC | #1 
On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-arm/translate-a64.c | 87 +++++++++++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 36 deletions(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 1587ab5..dcac490 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -166,6 +166,33 @@ void gen_a64_set_pc_im(uint64_t val)
>      tcg_gen_movi_i64(cpu_pc, val);
>  }
>
> +typedef struct DisasCompare64 {
> +    TCGCond cond;
> +    TCGv_i64 value;
> +} DisasCompare64;
> +
> +static void a64_test_cc(DisasCompare64 *c64, int cc)
> +{
> +    DisasCompare c32;
> +
> +    arm_test_cc(&c32, cc);
> +
> +    c64->value = tcg_temp_new_i64();
> +    c64->cond = c32.cond;
> +    if (c32.cond == TCG_COND_EQ || c32.cond == TCG_COND_NE) {
> +        tcg_gen_extu_i32_i64(c64->value, c32.value);
> +    } else {
> +        tcg_gen_ext_i32_i64(c64->value, c32.value);
> +    }

Signed extend would work for EQ and NE as well, wouldn't it?
Why prefer the unsigned extension in those cases? If there's
a reason, it could do with being commented.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Richard Henderson Sept. 8, 2015, 5:12 a.m. UTC | #2 
On 09/07/2015 10:17 AM, Peter Maydell wrote:
>> +    c64->value = tcg_temp_new_i64();
>> +    c64->cond = c32.cond;
>> +    if (c32.cond == TCG_COND_EQ || c32.cond == TCG_COND_NE) {
>> +        tcg_gen_extu_i32_i64(c64->value, c32.value);
>> +    } else {
>> +        tcg_gen_ext_i32_i64(c64->value, c32.value);
>> +    }
>
> Signed extend would work for EQ and NE as well, wouldn't it?

Yes.

> Why prefer the unsigned extension in those cases? If there's
> a reason, it could do with being commented.

I dunno what I was really thinking there.


r~
diff mbox

Patch

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 1587ab5..dcac490 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -166,6 +166,33 @@  void gen_a64_set_pc_im(uint64_t val)
     tcg_gen_movi_i64(cpu_pc, val);
 }
 
+typedef struct DisasCompare64 {
+    TCGCond cond;
+    TCGv_i64 value;
+} DisasCompare64;
+
+static void a64_test_cc(DisasCompare64 *c64, int cc)
+{
+    DisasCompare c32;
+
+    arm_test_cc(&c32, cc);
+
+    c64->value = tcg_temp_new_i64();
+    c64->cond = c32.cond;
+    if (c32.cond == TCG_COND_EQ || c32.cond == TCG_COND_NE) {
+        tcg_gen_extu_i32_i64(c64->value, c32.value);
+    } else {
+        tcg_gen_ext_i32_i64(c64->value, c32.value);
+    }
+
+    arm_free_cc(&c32);
+}
+
+static void a64_free_cc(DisasCompare64 *c64)
+{
+    tcg_temp_free_i64(c64->value);
+}
+
 static void gen_exception_internal(int excp)
 {
     TCGv_i32 tcg_excp = tcg_const_i32(excp);
@@ -3587,7 +3614,8 @@  static void disas_cc(DisasContext *s, uint32_t insn)
 static void disas_cond_select(DisasContext *s, uint32_t insn)
 {
     unsigned int sf, else_inv, rm, cond, else_inc, rn, rd;
-    TCGv_i64 tcg_rd, tcg_src;
+    TCGv_i64 tcg_rd, zero;
+    DisasCompare64 c;
 
     if (extract32(insn, 29, 1) || extract32(insn, 11, 1)) {
         /* S == 1 or op2<1> == 1 */
@@ -3602,48 +3630,35 @@  static void disas_cond_select(DisasContext *s, uint32_t insn)
     rn = extract32(insn, 5, 5);
     rd = extract32(insn, 0, 5);
 
-    if (rd == 31) {
-        /* silly no-op write; until we use movcond we must special-case
-         * this to avoid a dead temporary across basic blocks.
-         */
-        return;
-    }
-
     tcg_rd = cpu_reg(s, rd);
 
-    if (cond >= 0x0e) { /* condition "always" */
-        tcg_src = read_cpu_reg(s, rn, sf);
-        tcg_gen_mov_i64(tcg_rd, tcg_src);
-    } else {
-        /* OPTME: we could use movcond here, at the cost of duplicating
-         * a lot of the arm_gen_test_cc() logic.
-         */
-        TCGLabel *label_match = gen_new_label();
-        TCGLabel *label_continue = gen_new_label();
-
-        arm_gen_test_cc(cond, label_match);
-        /* nomatch: */
-        tcg_src = cpu_reg(s, rm);
+    a64_test_cc(&c, cond);
+    zero = tcg_const_i64(0);
 
+    if (rn == 31 && rm == 31 && (else_inc ^ else_inv)) {
+        /* CSET & CSETM.  */
+        tcg_gen_setcond_i64(tcg_invert_cond(c.cond), tcg_rd, c.value, zero);
+        if (else_inv) {
+            tcg_gen_neg_i64(tcg_rd, tcg_rd);
+        }
+    } else {
+        TCGv_i64 t_true = cpu_reg(s, rn);
+        TCGv_i64 t_false = read_cpu_reg(s, rm, 1);
         if (else_inv && else_inc) {
-            tcg_gen_neg_i64(tcg_rd, tcg_src);
+            tcg_gen_neg_i64(t_false, t_false);
         } else if (else_inv) {
-            tcg_gen_not_i64(tcg_rd, tcg_src);
+            tcg_gen_not_i64(t_false, t_false);
         } else if (else_inc) {
-            tcg_gen_addi_i64(tcg_rd, tcg_src, 1);
-        } else {
-            tcg_gen_mov_i64(tcg_rd, tcg_src);
-        }
-        if (!sf) {
-            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+            tcg_gen_addi_i64(t_false, t_false, 1);
         }
-        tcg_gen_br(label_continue);
-        /* match: */
-        gen_set_label(label_match);
-        tcg_src = read_cpu_reg(s, rn, sf);
-        tcg_gen_mov_i64(tcg_rd, tcg_src);
-        /* continue: */
-        gen_set_label(label_continue);
+        tcg_gen_movcond_i64(c.cond, tcg_rd, c.value, zero, t_true, t_false);
+    }
+
+    tcg_temp_free_i64(zero);
+    a64_free_cc(&c);
+
+    if (!sf) {
+        tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
     }
 }