[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/11] target-arm: Introduce DisasCompare
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/11] target-arm: Introduce DisasCompare |
Date: |
Mon, 7 Sep 2015 18:09:28 +0100 |
On 2 September 2015 at 18:57, Richard Henderson <address@hidden> 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 <address@hidden>
> ---
> 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