Re: [Qemu-devel] [PATCH 14/60] AArch64: Add orr instruction emulation

[Michael Matz]

Hi,

On Mon, 18 Nov 2013, Claudio Fontana wrote:

> >> +    case 3:
> >> +        tcg_gen_rotr_i64(r, cpu_reg(reg), tcg_shift);
> >> +        break;
> > 
> > Incorrect rotate for 32bit?

32bit rotates and shifts were fixed in a patch later than the 60er series 
Alex posted.  See attached.  (Generally there are many fixes to emulated 
instructions in that branch)

> >> +    if (!shift_amount && source == 0x1f) {
> 
> Besides the comment, is this correct?

No, it needs to check for opc == 1.

> >> +    tcg_dest = cpu_reg(dest);
> >> +    switch (opc) {
> >> +    case 0x0:
> >> +    case 0x3:
> >> +        tcg_gen_and_i64(tcg_dest, cpu_reg(source), tcg_op2);
> >> +        break;
> >> +    case 0x1:
> >> +        tcg_gen_or_i64(tcg_dest, cpu_reg(source), tcg_op2);
> >> +        break;
> >> +    case 0x2:
> >> +        tcg_gen_xor_i64(tcg_dest, cpu_reg(source), tcg_op2);
> >> +        break;
> >> +    }
> >> +
> >> +    if (is_32bit) {
> >> +        tcg_gen_ext32u_i64(tcg_dest, tcg_dest);
> >> +    }
> >> +
> >> +    if (setflags) {
> >> +        gen_helper_pstate_add(pstate, pstate, tcg_dest, cpu_reg(31), 
> >> tcg_dest);
> >> +    }
> > 
> > Incorrect flags generated.  They're different between add/sub and logical.
> > In particular, C and V are always zero.

That's done correctly with the fixed pstate helpers coming with a later 
patch (see attached as well).  reg31 is zero, so that's flags as if for 
"dest == dest + 0", and PSTATE_C and PSTATE_V will be zero.  That is, the 
logical flags are the same as the arithmetic flags for result plus zero 
with no carry_in.


Ciao,
Michael.

0001-Fix-32bit-rotates.patch
Description: Text Data

0002-Fix-the-pstate-flags-helpers.patch
Description: Text Data