From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54754)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Uu0Lb-0000rU-K4
	for qemu-devel@nongnu.org; Tue, 02 Jul 2013 09:10:21 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Uu0LW-0002iq-Vu
	for qemu-devel@nongnu.org; Tue, 02 Jul 2013 09:10:19 -0400
Received: from mail-la0-f47.google.com ([209.85.215.47]:57335)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Uu08o-0004jY-Bw
	for qemu-devel@nongnu.org; Tue, 02 Jul 2013 08:57:06 -0400
Received: by mail-la0-f47.google.com with SMTP id fe20so5549153lab.34
	for <qemu-devel@nongnu.org>; Tue, 02 Jul 2013 05:57:05 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <yw1xbo70mkjm.fsf@unicorn.mansr.com>
References: <1371565848-9297-1-git-send-email-mans@mansr.com>
	<CAFEAcA_bz1WEKy+r+d63w_AVUfhLs8Ns+nM5yXS-kfK49c1NeQ@mail.gmail.com>
	<yw1xbo70mkjm.fsf@unicorn.mansr.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Tue, 2 Jul 2013 13:56:45 +0100
Message-ID: <CAFEAcA_5tvx9jDw0Ewi=QUoF2s9YTVCpKSYhVETm2erWWrKLGQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH] target-arm: implement ARMv8 VSEL
	instruction
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= <mans@mansr.com>
Cc: qemu-devel@nongnu.org

On 21 June 2013 00:25, M=C3=A5ns Rullg=C3=A5rd <mans@mansr.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> On 18 June 2013 15:30, Mans Rullgard <mans@mansr.com> wrote:
>>> @@ -6699,6 +6742,12 @@ static void disas_arm_insn(CPUARMState * env, Di=
sasContext *s)
>>>              }
>>>              return; /* v7MP: Unallocated memory hint: must NOP */
>>>          }
>>> +        if ((insn & 0x0f000010) =3D=3D 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