Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix

[Paolo Bonzini]

On 06/11/2015 16:33, Mark Cave-Ayland 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?

No, they should all be the same.

Let's go with the "seems like a typo" version :) with a comment to say
that no, it's not a typo.

Paolo

> 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?