From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56268)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1Zuj1z-0004FI-Qp
	for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:34:24 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1Zuj1w-0002HV-I4
	for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:34:23 -0500
Received: from s16892447.onlinehome-server.info ([82.165.15.123]:48260)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mark.cave-ayland@ilande.co.uk>) id 1Zuj1w-0002Bz-BO
	for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:34:20 -0500
Message-ID: <563CC859.8090300@ilande.co.uk>
Date: Fri, 06 Nov 2015 15:33:45 +0000
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
MIME-Version: 1.0
References: <1446473134-4330-1-git-send-email-pbonzini@redhat.com>
	<CAFEAcA97rV3sZTZ085uLKnTc5_hQeJVrnN0CHt97ryzxk5iwGA@mail.gmail.com>
	<563777D5.6050000@redhat.com>
	<CAFEAcA-e23HiTvxji+y5LetLY2S66MGPEdfok4vLjJFou_6FKw@mail.gmail.com>
	<5639DA14.3020507@twiddle.net> <5639E1C2.80902@redhat.com>
	<5639E691.4050203@twiddle.net> <563A968D.705@ilande.co.uk>
	<563B1D91.5010701@redhat.com> <563B1F65.5000603@twiddle.net>
	<563B206F.3020800@redhat.com>
In-Reply-To: <563B206F.3020800@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in
	fpackfix
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: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Peter Maydell <peter.maydell@linaro.org>
Cc: Blue Swirl <blauwirbel@gmail.com>, QEMU Developers <qemu-devel@nongnu.org>

On 05/11/15 09:25, Paolo Bonzini wrote:

> On 05/11/2015 10:20, Richard Henderson wrote:
>>
>>>     /* Ugly code */
>>>     int64_t scaled = (uint64_t)(int64_t)src << scale;
>>
>> You mean
>>
>>   int64_t scaled = (int64_t)((uint64_t)src << scale);
> 
> No, that also looks like a typo.
> 
> I mean:
> 
> - unnecessary cast to int64_t to get the sign extension while avoiding
> the impression of a typo
> 
> - cast to uint64_t to avoid overflow
> 
> - the shift is done in the uint64_t type
> 
> - finally there is an implicit cast to int64_t

I would say that Richard's version above is the most readable to me,
however from what you're saying this would cause the compiler to produce
much less efficient code?

If this is the case then I could live with your second choice ("Seems
like a typo") with an appropriate comment if this maintains the
efficiency of generated code whilst also having well-defined behaviour
between compilers. Out of interest has anyone tried these alternatives
on clang?


ATB,

Mark.