[PATCH 15/16][fold-const.c] Fix bigendian HFmode in native_interpret_real

[Richard Biener]

On Thu, Jul 9, 2015 at 11:34 AM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> Jeff Law wrote:
>>
>> On 07/08/2015 03:43 AM, Richard Biener wrote:
>>>
>>> On Wed, Jul 8, 2015 at 12:07 AM, Jeff Law <law@redhat.com> wrote:
>>>>
>>>> On 07/07/2015 06:37 AM, Alan Lawrence wrote:
>>>>>
>>>>> As per https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01346.html. Fixes
>>>>> FAIL of advsimd-intrinsics vcreate.c on aarch64_be-none-elf from
>>>>> previous patch.
>>>>>
>>>>> 15_native_interpret_real.patch
>>>>>
>>>>>
>>>>> commit e2e7ca148960a82fc88128820f17e7cbd14173cb
>>>>> Author: Alan Lawrence<alan.lawrence@arm.com>
>>>>> Date:   Thu Apr 9 10:54:40 2015 +0100
>>>>>
>>>>>       Fix native_interpret_real for HFmode floats on Bigendian with
>>>>> UNITS_PER_WORD>=4
>>>>>
>>>>>       (with missing space)
>>>>
>>>> OK with ChangeLog in proper form.
>>>
>>> Err - but now offset can become negative?  Shouldn't it rather error out
>>> before as it requires at least 4 bytes for big-endian?
>>
>> I managed to convince myself the value wouldn't be negative when
>> reviewing.
>>
>>> That said - the whole thing looks it doesn't expect GET_MODE_SIZE < 4
>>> and your "fix" is just very obfuscated (if it really is a fix).
>>
>> While I couldn't convince myself the function as a whole was prepared for
>> smaller objects, I don't think Alan's patch made things worse.  One could
>> argue the whole bloody thing ought to be rewritten though.
>>
>> I'd also managed to convince myself the other instances of "3" weren't
>> problematical.
>>
>> jeff
>>
>
> In response to Richard's comments, may I propose the attached patch instead?
>
> I used the term "long" because of the earlier comment:
>   /* There are always 32 bits in each long, no matter the size of
>      the hosts long.  We handle floating point representations with
>      up to 192 bits.  */
> with which I really don't think I want to mess at this point ;). However,
> I'd be happy to change my use of "long" to "32 bits", "4 bytes", or "group
> of 4" instead if one of those was preferable!
>
> Bootstrapped + check-gcc on x86_64 (no change); cross-tested on
> aarch64_be-none-elf (where it fixes the advsimd-intrinsics float16 test)
>
> gcc/ChangeLog (as before):
>
>         * fold-const.c (native_interpret_real): Fix HFmode for bigendian
> where
>         UNITS_PER_WORD >= 4.

Looks good to me.  I think you can remove the assert, indeed offset
cannot become negative (but the compiler can't see that).

I wonder why wi::from_buffer doesn't have the same issue though
for HImode ints.  It's structured differently, without magic '4's as well.

Thanks,
Richard.