On Fri, 2019-12-20 at 12:08 +0100, Richard Biener wrote: > 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. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93270
Jeff