diff mbox

[v2,02/11] target-arm: Introduce DisasCompare

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

Commit Message

Richard Henderson Sept. 2, 2015, 5:57 p.m. UTC 
Split arm_gen_test_cc into 3 functions, so that it can be reused
for non-branch TCG comparisons.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-arm/translate.c | 110 ++++++++++++++++++++++++++++---------------------
 target-arm/translate.h |   9 ++++
 2 files changed, 73 insertions(+), 46 deletions(-)

Comments

Peter Maydell Sept. 7, 2015, 5:09 p.m. UTC | #1 
On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote:
> Split arm_gen_test_cc into 3 functions, so that it can be reused
> for non-branch TCG comparisons.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-arm/translate.c | 110 ++++++++++++++++++++++++++++---------------------
>  target-arm/translate.h |   9 ++++
>  2 files changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 3826a02..1f43777 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -738,81 +738,99 @@ static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv_i32 a, TCGv_i32 b)
>  #undef PAS_OP
>
>  /*
> - * generate a conditional branch based on ARM condition code cc.
> + * Generate a conditional based on ARM condition code cc.
>   * This is common between ARM and Aarch64 targets.
>   */
> -void arm_gen_test_cc(int cc, TCGLabel *label)
> +void arm_test_cc(DisasCompare *cmp, int cc)
>  {
> -    TCGv_i32 tmp;
> -    TCGLabel *inv;
> +    TCGv_i32 value;
> +    TCGCond cond;
> +    bool global = true;
>
>      switch (cc) {
>      case 0: /* eq: Z */
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
> -        break;
>      case 1: /* ne: !Z */
> -        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_ZF, 0, label);
> +        cond = TCG_COND_EQ;
> +        value = cpu_ZF;
>          break;
> +
>      case 2: /* cs: C */
> -        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_CF, 0, label);
> -        break;
>      case 3: /* cc: !C */
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, label);
> +        cond = TCG_COND_NE;
> +        value = cpu_CF;
>          break;
> +
>      case 4: /* mi: N */
> -        tcg_gen_brcondi_i32(TCG_COND_LT, cpu_NF, 0, label);
> -        break;
>      case 5: /* pl: !N */
> -        tcg_gen_brcondi_i32(TCG_COND_GE, cpu_NF, 0, label);
> +        cond = TCG_COND_LT;
> +        value = cpu_NF;
>          break;
> +
>      case 6: /* vs: V */
> -        tcg_gen_brcondi_i32(TCG_COND_LT, cpu_VF, 0, label);
> -        break;
>      case 7: /* vc: !V */
> -        tcg_gen_brcondi_i32(TCG_COND_GE, cpu_VF, 0, label);
> +        cond = TCG_COND_LT;
> +        value = cpu_VF;
>          break;
> +
>      case 8: /* hi: C && !Z */
> -        inv = gen_new_label();
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, inv);
> -        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_ZF, 0, label);
> -        gen_set_label(inv);
> -        break;
> -    case 9: /* ls: !C || Z */
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, label);
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
> +    case 9: /* ls: !C || Z -> !(C && !Z) */
> +        cond = TCG_COND_NE;
> +        value = tcg_temp_new_i32();
> +        global = false;
> +        tcg_gen_neg_i32(value, cpu_CF);
> +        tcg_gen_and_i32(value, value, cpu_ZF);
>          break;

The comment says hi is C && !Z, but the code
doesn't seem to line up with that. At least part
of that is presumably because we store ZF inverted,
but why are we negating CF here?

> +
>      case 10: /* ge: N == V -> N ^ V == 0 */
> -        tmp = tcg_temp_new_i32();
> -        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
> -        tcg_gen_brcondi_i32(TCG_COND_GE, tmp, 0, label);
> -        tcg_temp_free_i32(tmp);
> -        break;
>      case 11: /* lt: N != V -> N ^ V != 0 */
> -        tmp = tcg_temp_new_i32();
> -        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
> -        tcg_gen_brcondi_i32(TCG_COND_LT, tmp, 0, label);
> -        tcg_temp_free_i32(tmp);
> +        cond = TCG_COND_GE;
> +        value = tcg_temp_new_i32();
> +        global = false;
> +        tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
>          break;
> +
>      case 12: /* gt: !Z && N == V */
> -        inv = gen_new_label();
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, inv);
> -        tmp = tcg_temp_new_i32();
> -        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
> -        tcg_gen_brcondi_i32(TCG_COND_GE, tmp, 0, label);
> -        tcg_temp_free_i32(tmp);
> -        gen_set_label(inv);
> -        break;
>      case 13: /* le: Z || N != V */
> -        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
> -        tmp = tcg_temp_new_i32();
> -        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
> -        tcg_gen_brcondi_i32(TCG_COND_LT, tmp, 0, label);
> -        tcg_temp_free_i32(tmp);
> +        cond = TCG_COND_NE;
> +        value = tcg_temp_new_i32();
> +        global = false;
> +        tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
> +        tcg_gen_sari_i32(value, value, 31);
> +        tcg_gen_andc_i32(value, cpu_ZF, value);

I think this is correct, but it could use some commentary
to explain what it's doing.

Otherwise looks good.

thanks
-- PMM
Richard Henderson Sept. 8, 2015, 5:09 a.m. UTC | #2 
On 09/07/2015 10:09 AM, Peter Maydell wrote:
> On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote:
>> +    case 9: /* ls: !C || Z -> !(C && !Z) */
>> +        cond = TCG_COND_NE;
>> +        value = tcg_temp_new_i32();
>> +        global = false;
>> +        tcg_gen_neg_i32(value, cpu_CF);
>> +        tcg_gen_and_i32(value, value, cpu_ZF);
>>           break;
>
> The comment says hi is C && !Z, but the code
> doesn't seem to line up with that. At least part
> of that is presumably because we store ZF inverted,
> but why are we negating CF here?

We're computing CF ? -1 : 0.  ANDing that with !Z (aka cpu_ZF) gets us C & !Z.

>>       case 12: /* gt: !Z && N == V */
>>       case 13: /* le: Z || N != V */
>> +        cond = TCG_COND_NE;
>> +        value = tcg_temp_new_i32();
>> +        global = false;
>> +        tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
>> +        tcg_gen_sari_i32(value, value, 31);
>> +        tcg_gen_andc_i32(value, cpu_ZF, value);
>
> I think this is correct, but it could use some commentary
> to explain what it's doing.

Fair enough.


r~
Peter Maydell Sept. 8, 2015, 8:13 a.m. UTC | #3 
On 8 September 2015 at 06:09, Richard Henderson <rth@twiddle.net> wrote:
> On 09/07/2015 10:09 AM, Peter Maydell wrote:
>>
>> On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote:
>>>
>>> +    case 9: /* ls: !C || Z -> !(C && !Z) */
>>> +        cond = TCG_COND_NE;
>>> +        value = tcg_temp_new_i32();
>>> +        global = false;
>>> +        tcg_gen_neg_i32(value, cpu_CF);
>>> +        tcg_gen_and_i32(value, value, cpu_ZF);
>>>           break;
>>
>>
>> The comment says hi is C && !Z, but the code
>> doesn't seem to line up with that. At least part
>> of that is presumably because we store ZF inverted,
>> but why are we negating CF here?
>
>
> We're computing CF ? -1 : 0.  ANDing that with !Z (aka cpu_ZF) gets us C &
> !Z.

Ah yes. As with the case below, a comment would be helpful.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3826a02..1f43777 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -738,81 +738,99 @@  static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv_i32 a, TCGv_i32 b)
 #undef PAS_OP
 
 /*
- * generate a conditional branch based on ARM condition code cc.
+ * Generate a conditional based on ARM condition code cc.
  * This is common between ARM and Aarch64 targets.
  */
-void arm_gen_test_cc(int cc, TCGLabel *label)
+void arm_test_cc(DisasCompare *cmp, int cc)
 {
-    TCGv_i32 tmp;
-    TCGLabel *inv;
+    TCGv_i32 value;
+    TCGCond cond;
+    bool global = true;
 
     switch (cc) {
     case 0: /* eq: Z */
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
-        break;
     case 1: /* ne: !Z */
-        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_ZF, 0, label);
+        cond = TCG_COND_EQ;
+        value = cpu_ZF;
         break;
+
     case 2: /* cs: C */
-        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_CF, 0, label);
-        break;
     case 3: /* cc: !C */
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, label);
+        cond = TCG_COND_NE;
+        value = cpu_CF;
         break;
+
     case 4: /* mi: N */
-        tcg_gen_brcondi_i32(TCG_COND_LT, cpu_NF, 0, label);
-        break;
     case 5: /* pl: !N */
-        tcg_gen_brcondi_i32(TCG_COND_GE, cpu_NF, 0, label);
+        cond = TCG_COND_LT;
+        value = cpu_NF;
         break;
+
     case 6: /* vs: V */
-        tcg_gen_brcondi_i32(TCG_COND_LT, cpu_VF, 0, label);
-        break;
     case 7: /* vc: !V */
-        tcg_gen_brcondi_i32(TCG_COND_GE, cpu_VF, 0, label);
+        cond = TCG_COND_LT;
+        value = cpu_VF;
         break;
+
     case 8: /* hi: C && !Z */
-        inv = gen_new_label();
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, inv);
-        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_ZF, 0, label);
-        gen_set_label(inv);
-        break;
-    case 9: /* ls: !C || Z */
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_CF, 0, label);
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
+    case 9: /* ls: !C || Z -> !(C && !Z) */
+        cond = TCG_COND_NE;
+        value = tcg_temp_new_i32();
+        global = false;
+        tcg_gen_neg_i32(value, cpu_CF);
+        tcg_gen_and_i32(value, value, cpu_ZF);
         break;
+
     case 10: /* ge: N == V -> N ^ V == 0 */
-        tmp = tcg_temp_new_i32();
-        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
-        tcg_gen_brcondi_i32(TCG_COND_GE, tmp, 0, label);
-        tcg_temp_free_i32(tmp);
-        break;
     case 11: /* lt: N != V -> N ^ V != 0 */
-        tmp = tcg_temp_new_i32();
-        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
-        tcg_gen_brcondi_i32(TCG_COND_LT, tmp, 0, label);
-        tcg_temp_free_i32(tmp);
+        cond = TCG_COND_GE;
+        value = tcg_temp_new_i32();
+        global = false;
+        tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
         break;
+
     case 12: /* gt: !Z && N == V */
-        inv = gen_new_label();
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, inv);
-        tmp = tcg_temp_new_i32();
-        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
-        tcg_gen_brcondi_i32(TCG_COND_GE, tmp, 0, label);
-        tcg_temp_free_i32(tmp);
-        gen_set_label(inv);
-        break;
     case 13: /* le: Z || N != V */
-        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
-        tmp = tcg_temp_new_i32();
-        tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
-        tcg_gen_brcondi_i32(TCG_COND_LT, tmp, 0, label);
-        tcg_temp_free_i32(tmp);
+        cond = TCG_COND_NE;
+        value = tcg_temp_new_i32();
+        global = false;
+        tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
+        tcg_gen_sari_i32(value, value, 31);
+        tcg_gen_andc_i32(value, cpu_ZF, value);
         break;
+
     default:
         fprintf(stderr, "Bad condition code 0x%x\n", cc);
         abort();
     }
+
+    if (cc & 1) {
+        cond = tcg_invert_cond(cond);
+    }
+
+    cmp->cond = cond;
+    cmp->value = value;
+    cmp->value_global = global;
+}
+
+void arm_free_cc(DisasCompare *cmp)
+{
+    if (!cmp->value_global) {
+        tcg_temp_free_i32(cmp->value);
+    }
+}
+
+void arm_jump_cc(DisasCompare *cmp, TCGLabel *label)
+{
+    tcg_gen_brcondi_i32(cmp->cond, cmp->value, 0, label);
+}
+
+void arm_gen_test_cc(int cc, TCGLabel *label)
+{
+    DisasCompare cmp;
+    arm_test_cc(&cmp, cc);
+    arm_jump_cc(&cmp, label);
+    arm_free_cc(&cmp);
 }
 
 static const uint8_t table_logic_cc[16] = {
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 679bdbc..2f30862 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -62,6 +62,12 @@  typedef struct DisasContext {
     TCGv_i64 tmp_a64[TMP_A64_MAX];
 } DisasContext;
 
+typedef struct DisasCompare {
+    TCGCond cond;
+    TCGv_i32 value;
+    bool value_global;
+} DisasCompare;
+
 /* Share the TCG temporaries common between 32 and 64 bit modes.  */
 extern TCGv_ptr cpu_env;
 extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
@@ -143,6 +149,9 @@  static inline void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
 }
 #endif
 
+void arm_test_cc(DisasCompare *cmp, int cc);
+void arm_free_cc(DisasCompare *cmp);
+void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
 void arm_gen_test_cc(int cc, TCGLabel *label);
 
 #endif /* TARGET_ARM_TRANSLATE_H */