From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=46046 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PpHoe-0001Z6-NL
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:07:29 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpHod-000321-EY
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:07:28 -0500
Received: from eu1sys200aog120.obsmtp.com ([207.126.144.149]:60787)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpHod-00031h-57
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:07:27 -0500
Message-ID: <4D5A505A.8060506@st.com>
Date: Tue, 15 Feb 2011 11:07:22 +0100
From: Christophe Lyon <christophe.lyon@st.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 1/6] target-arm: Fix rounding constant
	addition for Neon shift instructions.
References: <1297437062-6118-1-git-send-email-christophe.lyon@st.com>	<1297437062-6118-2-git-send-email-christophe.lyon@st.com>
	<AANLkTinPVdCstd=tfDugoo7zSMJ2GxrAMyzyR6aqOzSn@mail.gmail.com>
In-Reply-To: <AANLkTinPVdCstd=tfDugoo7zSMJ2GxrAMyzyR6aqOzSn@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>

On 14.02.2011 19:12, Peter Maydell wrote:
> On 11 February 2011 15:10,  <christophe.lyon@st.com> wrote:
>> +uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
>> +{
>> +    int32_t dest;
>> +    int32_t val = (int32_t)valop;
>> +    int8_t shift = (int8_t)shiftop;
>> +    if (shift >= 32) {
>> +        dest = 0;
>> +    } else if (shift < -32) {
>> +        dest = val >> 31;
> 
> This is the wrong answer: large rounding right shifts give zero.
> 
>> +    } else if (shift == -32) {
>> +        dest = val >> 31;
>> +        dest++;
>> +        dest >>= 1;
> 
> These three lines will always result in dest becoming
> 0 regardless of the input value.
> 

You are right. Actually, I just intended to fix the case where
-32 < shift < 0, and merely re-instanciated the preceding macro with a known size of 32.

You comments also apply to the 8 and 16 bits variants in that macro.

I am too respectful of existing code :-)

Christophe.