On Thu, Jul 9, 2015 at 11:34 AM, Alan Lawrence <alan.lawre...@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 <l...@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.lawre...@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.
