On Thu, Dec 19, 2019 at 11:25 PM 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.
Shouldn't stmt_kills_ref_p be checking TYPE_PRECISION here?
Though x86_64 target might be the only one where MEM_SIZE/TYPE_SIZE is
not fully written to when TYPE_PRECISION!=TYPE_SIZE though.
I think I heard m68k writes the full 96bits.
Thanks,
Andrew Pinski
>
> 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
>
