[PATCH][simplify-rtx] Zero-initialise local array in simplify_immed_subreg

Kyrill Tkachov kyrylo.tkachov@foss.arm.com
Mon Oct 10 08:09:00 GMT 2016


On 07/10/16 18:56, Andrew Pinski wrote:
> On Fri, Oct 7, 2016 at 10:55 AM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Fri, Oct 7, 2016 at 7:08 AM, Kyrill Tkachov
>> <kyrylo.tkachov@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.tkachov@arm.com>
>>>
>>>      * simplify-rtx.c (simplify_immed_subreg): Zero-initialize tmp array
>>>      before merging in bytes to pass down to real_from_target.



More information about the Gcc-patches mailing list