On December 20, 2019 8:25:18 AM GMT+01:00, Jeff Law <l...@redhat.com> wrote:
>On Fri, 2019-12-20 at 08:09 +0100, Richard Biener wrote:
>> On December 20, 2019 3:20:40 AM GMT+01:00, Jeff Law <l...@redhat.com>
>wrote:
>> > I need a sanity check here.
>> >
>> > Given this code:
>> >
>> > > typedef union { long double value; unsigned int word[4]; }
>> > memory_long_double;
>> > > static unsigned int ored_words[4];
>> > > static void add_to_ored_words (long double x)
>> > > {
>> > > memory_long_double m;
>> > > size_t i;
>> > > memset (&m, 0, sizeof (m));
>> > > m.value = x;
>> > > for (i = 0; i < 4; i++)
>> > > {
>> > > ored_words[i] |= m.word[i];
>> > > }
>> > > }
>> > >
>> >
>> > DSE is removing the memset as it thinks the assignment to m.value
>is
>> > going to set the entire union.
>> >
>> > But when we translate that into RTL we use XFmode:
>> >
>> > > ;; m.value ={v} x_6(D);
>> > >
>> > > (insn 7 6 0 (set (mem/v/j/c:XF (plus:DI (reg/f:DI 77
>> > virtual-stack-vars)
>> > > (const_int -16 [0xfffffffffffffff0])) [2
>m.value+0
>> > S16 A128])
>> > > (reg/v:XF 86 [ x ])) "j.c":13:11 -1
>> > > (nil))
>> > >
>> >
>> > That (of course) only writes 80 bits of data because of XFmode,
>leaving
>> > 48 bits uninitialized. We then read those bits, or-ing the
>> > uninitialized data into ored_words and all hell breaks loose later.
>> >
>> > Am I losing my mind? ISTM that dse and the expander have to agree
>on
>> > how much data is written by the store to m.value.
>>
>> It looks like MEM_SIZE is wrong here, so you need to figure how we
>arrive at this (I guess TYPE_SIZE vs. MODE_SIZE mismatch is biting us
>here?)
>>
>> That is, either the MEM should have BLKmode or the mode size should
>match
>> MEM_SIZE. Maybe DSE can avoid looking at MEM_SIZE for non-BLKmode
>MEMs?
>It's gimple DSE that removes the memset, so it shouldn't be mucking
>around with modes at all. stmt_kills_ref_p seems to think the
>assignment to m.value sets all of m.
>
>The ao_ref for memset looks reasonable:
>
>> (gdb) p *ref
>> $14 = {ref = 0x0, base = 0x7ffff7ffbea0, offset = {<poly_int_pod<1,
>long>> = {coeffs = {0}}, <No data fields>},
>> size = {<poly_int_pod<1, long>> = {coeffs = {128}}, <No data
>fields>}, max_size = {<poly_int_pod<1, long>> = {
>> coeffs = {128}}, <No data fields>}, ref_alias_set = 0,
>base_alias_set = 0, volatile_p = false}
>>
>128 bits with a base of VAR_DECL m.
>
>We looking to see if this statement will kill the ref:
>
>> (gdb) p debug_gimple_stmt (stmt)
>> # .MEM_8 = VDEF <.MEM_6>
>> m.value ={v} x_7(D);
>> $21 = void
>> (gdb) p debug_tree (lhs)
>> <component_ref 0x7fffea97da50
>> type <real_type 0x7fffea988690 long double sizes-gimplified
>volatile XF
>> size <integer_cst 0x7fffea7f3d20 constant 128>
>> unit-size <integer_cst 0x7fffea7f3d38 constant 16>
>> align:128 warn_if_not_align:0 symtab:0 alias-set -1
>canonical-type 0x7fffea988690 precision:80>
>> side-effects volatile
>> arg:0 <var_decl 0x7ffff7ffbea0 m
>> type <union_type 0x7fffea9882a0 memory_long_double
>sizes-gimplified volatile type_0 BLK size <integer_cst 0x7fffea7f3d20
>128> unit-size <integer_cst 0x7fffea7f3d38 16>
>> align:128 warn_if_not_align:0 symtab:0 alias-set -1
>canonical-type 0x7fffea988348 fields <field_decl 0x7fffea9527b8 value>
>context <translation_unit_decl 0x7fffea974168 j.i>
>> pointer_to_this <pointer_type 0x7fffea9883f0>>
>> side-effects addressable volatile used read BLK j.c:10:31
>size <integer_cst 0x7fffea7f3d20 128> unit-size <integer_cst
>0x7fffea7f3d38 16>
>> align:128 warn_if_not_align:0 context <function_decl
>0x7fffea97bd00 add_to_ored_words>
>> chain <var_decl 0x7ffff7ffbf30 i type <integer_type
>0x7fffea9430a8 size_t>
>> used unsigned read DI j.c:11:10
>> size <integer_cst 0x7fffea7f3cd8 constant 64>
>> unit-size <integer_cst 0x7fffea7f3cf0 constant 8>
>> align:64 warn_if_not_align:0 context <function_decl
>0x7fffea97bd00 add_to_ored_words>>>
>> arg:1 <field_decl 0x7fffea9527b8 value
>> type <real_type 0x7fffea8133f0 long double sizes-gimplified
>XF size <integer_cst 0x7fffea7f3d20 128> unit-size <integer_cst
>0x7fffea7f3d38 16>
>> align:128 warn_if_not_align:0 symtab:0 alias-set -1
>canonical-type 0x7fffea8133f0 precision:80
>> pointer_to_this <pointer_type 0x7fffea813930>>
>> XF j.c:6:29 size <integer_cst 0x7fffea7f3d20 128> unit-size
><integer_cst 0x7fffea7f3d38 16>
>> align:128 warn_if_not_align:0 offset_align 128
>> offset <integer_cst 0x7fffea7f3d08 constant 0>
>> bit-offset <integer_cst 0x7fffea7f3d50 constant 0> context
><union_type 0x7fffea981e70>
>> chain <field_decl 0x7fffea952850 word type <array_type
>0x7fffea981f18>
>> TI j.c:6:49 size <integer_cst 0x7fffea7f3d20 128>
>unit-size <integer_cst 0x7fffea7f3d38 16>
>> align:32 warn_if_not_align:0 offset_align 128 offset
><integer_cst 0x7fffea7f3d08 0> bit-offset <integer_cst 0x7fffea7f3d50
>0> context <union_type 0x7fffea981e70>>>
>> j.c:13:4 start: j.c:13:3 finish: j.c:13:9>
>> $22 = void
>>
>
>stmt_kills_ref_p calls get_ref_base_and_extent on that LHS object. THe
>returned base is the same as ref->base. The returned offset is zero
>with size/max_size of 128 bits. So according to
>get_ref_base_and_extent the assignment is going to write 128 bits and
>thus kills the memset.
>
>One might argue that's where the problems start -- somewhere in
>get_ref_base_and_extent.
>
>I'm largely offline the next couple weeks...
>
>I don't have any "real" failures I'm tracking because of this, but it
>does cause some configure generated tests to give the wrong result.
>Thankfully the inconsistency just doesn't matter for any of the
>packages that are affected.
It's certainly something to look at. I'm largely offline already so please file
a bug report so we don't forget. I'll have a detailed look next year.
Thanks,
Richard.
>
>Jeff
