From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:35898)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1X4EjT-0006Nu-92
	for qemu-devel@nongnu.org; Mon, 07 Jul 2014 15:37:56 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1X4EjK-0006nG-7q
	for qemu-devel@nongnu.org; Mon, 07 Jul 2014 15:37:47 -0400
Received: from mail-qg0-x233.google.com ([2607:f8b0:400d:c04::233]:42689)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1X4EjK-0006mU-3p
	for qemu-devel@nongnu.org; Mon, 07 Jul 2014 15:37:38 -0400
Received: by mail-qg0-f51.google.com with SMTP id z60so4046872qgd.38
	for <qemu-devel@nongnu.org>; Mon, 07 Jul 2014 12:37:37 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <53BAF6FB.2000803@twiddle.net>
Date: Mon, 07 Jul 2014 12:37:31 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <1404756822-3253-1-git-send-email-kbastian@mail.uni-paderborn.de>
	<1404756822-3253-6-git-send-email-kbastian@mail.uni-paderborn.de>
In-Reply-To: <1404756822-3253-6-git-send-email-kbastian@mail.uni-paderborn.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 05/15] target-tricore: Add masks and
	opcodes for decoding
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: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org

On 07/07/2014 11:13 AM, Bastian Koppelmann wrote:
> +/* This creates a mask with bits start .. end set to 1 and applies it to op */
> +#define MASK_BITS(op, start, end) (op & (((1 << (end-start+1)) - 1) << start))

This is extract32(op, (start), (end) - (start)) << (start)

Do you ever use the unshifted bits, and if so, why?

> +#define MASK_BITS_SHIFT(op, start, end) (MASK_BITS(op, start, end) >> start)

This is extract32(op, (start), (end) - (start))

I note that quite a few times within the later patch you actually sign-extend
this result, duplicating the field width all over the place.  It might be
better to just use sextract32 as required in the first place to treat the
fields that ought to be sign-extended as sign-extended.


r~