From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43006)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZPNKT-0005yS-4d
	for qemu-devel@nongnu.org; Wed, 12 Aug 2015 00:07:53 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZPNKO-0005OG-2K
	for qemu-devel@nongnu.org; Wed, 12 Aug 2015 00:07:53 -0400
Received: from mail-qg0-x236.google.com ([2607:f8b0:400d:c04::236]:35776)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZPNKN-0005OB-Tk
	for qemu-devel@nongnu.org; Wed, 12 Aug 2015 00:07:48 -0400
Received: by qgj62 with SMTP id 62so3669557qgj.2
	for <qemu-devel@nongnu.org>; Tue, 11 Aug 2015 21:07:47 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
References: <1439151229-27747-1-git-send-email-laurent@vivier.eu>
	<1439151229-27747-6-git-send-email-laurent@vivier.eu>
From: Richard Henderson <rth@twiddle.net>
Message-ID: <55CAC68E.60601@twiddle.net>
Date: Tue, 11 Aug 2015 21:07:42 -0700
MIME-Version: 1.0
In-Reply-To: <1439151229-27747-6-git-send-email-laurent@vivier.eu>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes
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: Laurent Vivier <laurent@vivier.eu>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, Andreas Schwab <schwab@linux-m68k.org>, gerg@uclinux.org

On 08/09/2015 01:13 PM, Laurent Vivier wrote:
> -#define OS_BYTE 0
> -#define OS_WORD 1
> -#define OS_LONG 2
> -#define OS_SINGLE 4
> -#define OS_DOUBLE 5
> +#define OS_BYTE     1
> +#define OS_WORD     2
> +#define OS_LONG     3
> +#define OS_SINGLE   4
> +#define OS_DOUBLE   5
> +#define OS_EXTENDED 6
> +#define OS_PACKED   7
>

Is there a reason you've skipped the 0 value when adding the new values?

> +static inline int insn_opsize(int insn, int pos)
> +{
> +    switch ((insn >> pos) & 3) {


In particular, that change means that insn_opsize is more complicated than 
needed.  Further, is there any reason for POS to be a varable?  Isn't it at the 
same place for all insns?

> +static inline int ext_opsize(int ext, int pos)

This should probably wait until the fp insns get added.


r~