Re: [Qemu-devel] [PATCH] target-arm: implement ARMv8 VSEL instruction

[Peter Maydell]

On 21 June 2013 00:25, Måns Rullgård <address@hidden> wrote:
> Peter Maydell <address@hidden> writes:
>> On 18 June 2013 15:30, Mans Rullgard <address@hidden> wrote:
>>> @@ -6699,6 +6742,12 @@ static void disas_arm_insn(CPUARMState * env, 
>>> DisasContext *s)
>>>              }
>>>              return; /* v7MP: Unallocated memory hint: must NOP */
>>>          }
>>> +        if ((insn & 0x0f000010) == 0x0e000000) {
>>> +            /* cdp2 */
>>> +            if (disas_coproc_insn(env, s, insn))
>>> +                goto illegal_op;
>>> +            return;
>>> +        }
>>
>> This hunk is oddly placed, because it's neither next to the neon
>> decode (which is further up) nor the mrc2/mcr2 decode (which is
>> further down).
>
> That's because it is neither.  It is CDP2, previously not decoded at all.
> This seemed as logical a place as any to me.  If you disagree, please
> say where you'd prefer that it go.

Well, if we're treating it in the same way as Neon (ie pulling
the VFP decode out of the decode_coproc() function as I suggest),
then the logical place to put VFP decode is next to the Neon
decode.

At that point the need to actually decode CDP2 into a call
to disas_coproc_insn() goes away, but if we did want to include
it then the logical place to put that decode is next to the
MCR2/MRC2 decode, because both CDP2 and MCR2/MRC2 deal with
coprocessor instructions and are in the same table in the ARM ARM.

-- PMM