On Fri, Oct 7, 2016 at 10:55 AM, Andrew Pinski <pins...@gmail.com> wrote: > On Fri, Oct 7, 2016 at 7:08 AM, Kyrill Tkachov > <kyrylo.tkac...@foss.arm.com> wrote: >> Hi all, >> >> I've encountered another wrong-code bug with the store merging pass. This >> time it's in RTL. >> The test gcc.target/aarch64/aapcs64/test_27.c on aarch64 merges a few __fp16 >> values at GIMPLE level but >> during RTL dse1 one of the constants generated gets wrongly misinterpreted >> from HImode to HFmode >> by simplify_immed_subreg. The HFmode value it ends up producing is >> completely bogus. >> >> By stepping through the code with GDB the problem is in the hunk touched by >> this patch when it >> fills in an array of longs with the value bytes before passing it down to >> real_from_target. >> It fills in the array by orring in each byte. >> >> However, the array it declared to use for this doesn not get properly >> zero-initialised for modes >> less that 32 bits wide (HFmode in this case). The fix in this patch is to >> just use an array initialiser >> to zero it out. This makes the failure go away. >> >> Bootstrapped and tested on aarch64, x86_64. >> >> Ok for trunk? > > Even though this has been approved I think it is best to do write this > code as this: > long tmp[MAX_BITSIZE_MODE_ANY_MODE / 32]; > /* real_from_target wants its input in words affected by > FLOAT_WORDS_BIG_ENDIAN. However, we ignore this, > and use WORDS_BIG_ENDIAN instead; see the documentation > of SUBREG in rtl.texi. */ > memset (tmp, 0, sizeof(tmp));
Also the / 32 should be changed to / (sizeof(long) * BITS_PER_CHAR) but that was there before hand. THanks, Andrew > > Thanks, > Andrew Pinski > > >> Thanks, >> Kyrill >> >> 2016-10-06 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >> >> * simplify-rtx.c (simplify_immed_subreg): Zero-initialize tmp array >> before merging in bytes to pass down to real_from_target.
