From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=43903 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PpLXg-00086k-Ly
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 09:06:13 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpLXb-0001pU-1o
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 09:06:12 -0500
Received: from eu1sys200aog112.obsmtp.com ([207.126.144.133]:39550)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpLXa-0001p8-Mn
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 09:06:06 -0500
Message-ID: <4D5A884C.6000104@st.com>
Date: Tue, 15 Feb 2011 15:06:04 +0100
From: Christophe Lyon <christophe.lyon@st.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for
	Neon right shifts.
References: <1297437062-6118-1-git-send-email-christophe.lyon@st.com>	<1297437062-6118-5-git-send-email-christophe.lyon@st.com>
	<AANLkTimbqACSVJGjnd6wmHb3x2LDW=dBqFrT63xi7gbi@mail.gmail.com>
In-Reply-To: <AANLkTimbqACSVJGjnd6wmHb3x2LDW=dBqFrT63xi7gbi@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>


>> @@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t valop, uint32_t shiftop)
>>         dest = val << shift;
>>         if ((dest >> shift) != val) {
>>             SET_QC();
>> -            dest = (uint32_t)(1 << (sizeof(val) * 8 - 1)) - (val > 0 ? 1 : 0);
>> +            if (val < 0) {
>> +                dest = INT32_MIN;
>> +            } else {
>> +                dest = INT32_MAX;
>> +            }
> 
> Again, right answers but the way most of the rest of the code
> forces a 32 bit value to signed saturation is
>    dest = (val >> 31) ^ ~SIGNBIT;
> 

Indeed; I hadn't given a close look at how saturation is performed elsewhere.
Anyway I find my version more readable ;-)

Quite a few times, I have wondered whether there are rules in QEmu development helping decide between performance of the code vs readability/maintainability?

Christophe.