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

[Mark Cave-Ayland]

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.