On 07/10/16 18:56, Andrew Pinski wrote:
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.
Agreed about this, but I'm not sure about the memset.
long tmp[<size>] = { 0 };
looks shorter and cleaner to me and is guaranteed to do the right
thing as well, no?
Thanks,
Kyrill
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.
