[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm: Fix SMLAD incorrect setting of Q bit
From: |
Peter Maydell |
Subject: |
Re: [PATCH] target/arm: Fix SMLAD incorrect setting of Q bit |
Date: |
Fri, 9 Oct 2020 19:47:17 +0100 |
On Fri, 9 Oct 2020 at 18:57, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/9/20 9:47 AM, Peter Maydell wrote:
> > + /*
> > + * t1 is the low half of the result which goes into Rd.
> > + * We have overflow and must set Q if the high half (t2)
> > + * is different from the sign-extension of t1.
> > + */
> > + t3 = tcg_temp_new_i32();
> > + tcg_gen_sari_i32(t3, t1, 31);
> > + qf = load_cpu_field(QF);
> > + one = tcg_const_i32(1);
> > + tcg_gen_movcond_i32(TCG_COND_NE, qf, t2, t3, one, qf);
> > + store_cpu_field(qf, QF);
>
> This works, however, QF is not restricted to {0,1}.
I'm not sure if you mean "this code doesn't maintain that
invariant" or "there is no such invariant". If the former,
the declaration of the field in cpu.h disagrees:
uint32_t QF; /* 0 or 1 */
If the latter, I think the code does preserve the invariant.
Either we set QF to 'one', or we write back the value it had
to start with (which must have been {0,1}, but we don't really
care, because we don't want to touch it.)
> Perhaps this is simpler?
>
> qf = load_cpu_field(QF);
> /* t1 is the low half; extract the sign bit */
> tcg_gen_shri_i32(t3, t1, 31);
> /* t2 is the high half; must be 0 or -1,
> and the extension of the sign bit. adding them
> must equal 0 (0 + 0 = 0; -1 + 1 = 0). */
> tcg_gen_add_i32(t3, t3, t2);
> /* Any non-zero result sets QF */
> tcg_gen_or_i32(qf, qf, t3);
> store_cpu_field(qf, QF);
This looks like it can write something to QF that isn't 0 or 1.
thanks
-- PMM