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.
Jeff
