From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58694)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZMNUY-0000Xx-Tb
	for qemu-devel@nongnu.org; Mon, 03 Aug 2015 17:41:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZMNUU-0005Gu-7L
	for qemu-devel@nongnu.org; Mon, 03 Aug 2015 17:41:54 -0400
Received: from mail-qg0-x231.google.com ([2607:f8b0:400d:c04::231]:35194)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZMNUU-0005Go-3C
	for qemu-devel@nongnu.org; Mon, 03 Aug 2015 17:41:50 -0400
Received: by qgii95 with SMTP id i95so98608083qgi.2
	for <qemu-devel@nongnu.org>; Mon, 03 Aug 2015 14:41:49 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
References: <1438627752-19903-2-git-send-email-rth@twiddle.net>
	<1438630553-21240-1-git-send-email-rth@twiddle.net>
	<20150803213128.GA19612@aurel32.net>
From: Richard Henderson <rth@twiddle.net>
Message-ID: <55BFE019.2080504@twiddle.net>
Date: Mon, 3 Aug 2015 14:41:45 -0700
MIME-Version: 1.0
In-Reply-To: <20150803213128.GA19612@aurel32.net>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 1/3] target-mips: Copy restrictions from
 ext/ins to dext/dins
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: Aurelien Jarno <aurelien@aurel32.net>
Cc: leon.alrae@imgtec.com, qemu-devel@nongnu.org

On 08/03/2015 02:31 PM, Aurelien Jarno wrote:
> On 2015-08-03 12:35, Richard Henderson wrote:
>>           if (msb != 31) {
>> -            tcg_gen_andi_tl(t0, t0, (1 << (msb + 1)) - 1);
>> +            tcg_gen_andi_tl(t0, t0, (1U << (msb + 1)) - 1);
>
> Is this change really needed?

msb == 30 means 1 << 31.  Which officially must be unsigned to be correct.  If 
we were to run under ubsan, this would trigger an error.

> Note that DEXT can't fail as both lsb and msb are in the range 0..31.
> DEXTU and DEXTM can.
...
> The same way DINSM can't fail.

Yes, I know.  But it seems cleaner to do the checks always, unifying all of the 
code.

> Should we try to get this one into 2.4, if not already too late?

Perhaps.  Otherwise via stable after the fact.


r~