| Message ID | 57F508EE.6020804@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 10/05/2016 04:06 PM, Kyrill Tkachov wrote: > The fix in this patch makes the logic in native_encode_real match the > logic in native_interpret_real, > I just copied the logic across. Looks reasonable to me. Ok. Bernd
On Wed, Oct 5, 2016 at 4:06 PM, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi all, > > I encountered a wrong-code issue with my WIP store merging pass when it was > trying to encode HFmode constants. > I am using native_encode_real to write the constants to a byte array and > it's breaking on big-endian. > For a 2-byte constant it ended up writing bytes at offsets 3 and 2 rather > than 1 and 0. > > The fix in this patch makes the logic in native_encode_real match the logic > in native_interpret_real, > I just copied the logic across. > > I don't have a testcase against clean trunk that demonstrates the issue but > with my store merging patch Might be sth simple as unsigned short foo (void) { unsigned short bar; typedef HFmode alias_HF __attribute__((may_alias)); *(ailas_HFmode *)bar = HFmode constant; return bar; } which should get a bogus integer representation from FRE. > the testcase gcc.target/aarch64/advsimd-intrinsics/vldX.c starts failing > without this patch because > adjacent 16-bit float constants are not being merged correctly. > > Bootstrapped and tested on aarch64-none-linux-gnu. > As this patch only affects the big-endian code path I also tested it on > aarch64_be-none-elf. > > Ok for trunk? Do you think this needs to be backported? > > Thanks, > Kyrill > > 2016-10-05 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * fold-const.c (native_encode_real): Fix logic for selecting offset > to write to when BYTES_BIG_ENDIAN.
diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 81671e9fec68a3281b744626db1b94f26d19aabc..564d086e9636743104d629cd6f5620ac2ee6a544 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -7142,7 +7142,16 @@ native_encode_real (const_tree expr, unsigned char *ptr, int len, int off) offset += byte % UNITS_PER_WORD; } else - offset = BYTES_BIG_ENDIAN ? 3 - byte : byte; + { + offset = byte; + if (BYTES_BIG_ENDIAN) + { + /* Reverse bytes within each long, or within the entire float + if it's smaller than a long (for HFmode). */ + offset = MIN (3, total_bytes - 1) - offset; + gcc_assert (offset >= 0); + } + } offset = offset + ((bitpos / BITS_PER_UNIT) & ~3); if (offset >= off && offset - off < len)
Hi all, I encountered a wrong-code issue with my WIP store merging pass when it was trying to encode HFmode constants. I am using native_encode_real to write the constants to a byte array and it's breaking on big-endian. For a 2-byte constant it ended up writing bytes at offsets 3 and 2 rather than 1 and 0. The fix in this patch makes the logic in native_encode_real match the logic in native_interpret_real, I just copied the logic across. I don't have a testcase against clean trunk that demonstrates the issue but with my store merging patch the testcase gcc.target/aarch64/advsimd-intrinsics/vldX.c starts failing without this patch because adjacent 16-bit float constants are not being merged correctly. Bootstrapped and tested on aarch64-none-linux-gnu. As this patch only affects the big-endian code path I also tested it on aarch64_be-none-elf. Ok for trunk? Do you think this needs to be backported? Thanks, Kyrill 2016-10-05 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * fold-const.c (native_encode_real): Fix logic for selecting offset to write to when BYTES_BIG_ENDIAN.