On Fri, Dec 20, 2019 at 12:08 PM Richard Biener
<richard.guent...@gmail.com> 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.
So clearly something is wrong:
<component_ref 0x7ffff695d240
type <real_type 0x7ffff682d3f0 long double sizes-gimplified XF
size <integer_cst 0x7ffff680dd20 constant 128>
unit-size <integer_cst 0x7ffff680dd38 constant 16>
align:128 warn_if_not_align:0 symtab:0 alias-set 2
canonical-type 0x7ffff682d3f0 precision:80
pointer_to_this <pointer_type 0x7ffff682d930>>
and thus GET_MODE_SIZE (XFmode) == 16. The target cannot possibly
just store 12 bytes here,
it lies. Why's XFmode not 12 bytes but with 8/16 byte alignment?
Does the ABI say sizeof(long double) is 16?
That said, a mode-has-padding or whatever should be reflected in
TYPE_SIZE & friends as well, inconsistencies
there just make things worse. Now they're at least consistently wrong.
Richard.
> Thanks,
> Richard.
>
> >
> >Jeff
>
