From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=50025 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PpLFM-00076Z-An
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 08:47:17 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpLFL-0005vh-0P
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 08:47:16 -0500
Received: from eu1sys200aog110.obsmtp.com ([207.126.144.129]:55838)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1PpLFK-0005vV-Md
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 08:47:14 -0500
Message-ID: <4D5A83DF.9090104@st.com>
Date: Tue, 15 Feb 2011 14:47:11 +0100
From: Christophe Lyon <christophe.lyon@st.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/6] target-arm: fix Neon right shifts with
	shift amount == input width.
References: <1297437062-6118-1-git-send-email-christophe.lyon@st.com>	<1297437062-6118-3-git-send-email-christophe.lyon@st.com>
	<AANLkTim7nSROGMAJ_mnhznBsxUHENZQX=gR2y4f3ZzRf@mail.gmail.com>
In-Reply-To: <AANLkTim7nSROGMAJ_mnhznBsxUHENZQX=gR2y4f3ZzRf@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>


>> -        dest = src1 >> (tmp - 1); \
>> +        dest = src1 >> (-tmp - 1); \
>>         dest++; \
>>         dest >>= 1; \
> 
> Again, these three lines have the same effect as dest = 0,
> so we can fold into the previous if().
> 
>>     } else if (tmp < 0) { \
>> @@ -594,7 +594,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
>>         val = 0;
>>     } else if (shift < -64) {
>>         val >>= 63;
> 
> You didn't change this case, but it is the wrong answer:
> should be 0.
> 
>> -    } else if (shift == -63) {
>> +    } else if (shift == -64) {
>>         val >>= 63;
>>         val++;
>>         val >>= 1;
> 
> Always results in 0.
> 


Oops sorry, in these 3 cases, I just fixed obvious typos but didn't question the actual code.