[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix |
Date: |
Wed, 04 Nov 2015 18:53:06 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Paolo Bonzini <address@hidden> writes:
> On 04/11/2015 15:07, Markus Armbruster wrote:
>> Paolo Bonzini <address@hidden> writes:
>>
>>> On 04/11/2015 12:05, Richard Henderson wrote:
>>>> On 11/04/2015 11:45 AM, Paolo Bonzini wrote:
>>>>>>>>>>> int32_t src = rs2 >> (word * 32);
>>>>>>>>>>> - int64_t scaled = src << scale;
>>>>>>>>>>> + int64_t scaled = (int64_t)src << scale;
>>>>>>>>>>> int64_t from_fixed = scaled >> 16;
>>>> ...
>>>>>>
>>>>>> I do think we'd be better served by casting to uint64_t on that line.
>>>>>> Note that fpackfix requires the same correction. And it wouldn't hurt
>>>>>> to cast to uint32_t in fpack16, lest we anger the self-same shifting
>>>>>> gods.
>>>>>
>>>>> Hmmm.. say src = -0x80000000, scale = 1;
>>>>>
>>>>> scaled = (uint64_t)-0x8000000 << 1 = 0xffffffff00000000
>>>>> from_fixed = 0xffffffff00000000 >> 16 = 0x0000ffffffff0000
>>>>>
>>>>> Now from_fixed is positive and you get 32767 instead of -32768. In
>>>>> other words, we would have to cast to uint64_t on the scaled assignment,
>>>>> and back to int64_t on the from_fixed assignment. I must be
>>>>> misunderstanding your suggestion.
>>>>
>>>> int64_t scaled = (uint64_t)src << scale;
>>>>
>>>> I.e. one explicit conversion and one implicit conversion.
>>>
>>> That does the job, but it also does look like a typo...
>>
>> Make the implicit conversion explicit then.
>
> Sorry, but I'll say it again: there's _no way_ that a sane compiler will
> _ever_ use this particular bit of undefined behavior.
>
> I'm generally against uglifying the code to placate ubsan, but
> especially so in this case: it is not common code and it would only
> affect people running fpackfix under ubsan.
Oh, I don't disagree at all with "let's not uglify code".
I wish compilers hadn't become so nasty, though. I wish they had more
respect for the imperfect real-world code they compile, and less
benchmark worship.
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, (continued)
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Peter Maydell, 2015/11/02
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/02
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Peter Maydell, 2015/11/02
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/02
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Richard Henderson, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Richard Henderson, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Markus Armbruster, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Mark Cave-Ayland, 2015/11/04
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Richard Henderson, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Richard Henderson, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/05
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Mark Cave-Ayland, 2015/11/06
- Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, Paolo Bonzini, 2015/11/06