Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

[Jan Hubicka]

Hi,
while looking for the reason for extra disambiguations in
aliasing_component_refs I have noticed an odd case.  We newly disambiguate:

MEM[(Element_t *)_27 + 4B];

s.globalIDDataBase_m;

 <mem_ref 0x7fffe2f68f28
    type <integer_type 0x7ffff63ffd20 Element_t sizes-gimplified type_6 SI
        size <integer_cst 0x7ffff70ff078 constant 32>
        unit-size <integer_cst 0x7ffff70ff090 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff70fb5e8 precision:32 min <integer_cst 0x7ffff70ff030 -2147483648> max <integer_cst 0x7ffff70ff048 2147483647>
        pointer_to_this <pointer_type 0x7fffe55a3dc8>>
    tree_0 tree_2
    arg:0 <ssa_name 0x7fffe2f666c0
        type <pointer_type 0x7ffff62df888 type <record_type 0x7ffff658d930 Domain>
            public unsigned DI
            size <integer_cst 0x7ffff70dee28 constant 64>
            unit-size <integer_cst 0x7ffff70dee40 constant 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set 216 canonical-type 0x7ffff62df888>
        visited
        def_stmt _27 = &MEM[(struct OneDomain_t *)&s].D.113982.domain_m[i_26].D.110783.D.44395;
        version:27
        ptr-info 0x7ffff5c66540>
    arg:1 <integer_cst 0x7fffe98babe8 type <pointer_type 0x7fffe55a3dc8> constant 4>>
 <component_ref 0x7ffff1b8d030
    type <pointer_type 0x7ffff494bf18
        type <record_type 0x7ffff494b7e0 GlobalIDDataBase addressable needs-constructing type_1 type_4 type_5 type_6 BLK
            size <integer_cst 0x7ffff4fe92d0 constant 576>
            unit-size <integer_cst 0x7ffff4fe9048 constant 72>
            align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff494b7e0 fields <function_decl 0x7fffecee2000 __dt > context <translation_unit_decl 0x7ffff70eb168 /aux/hubicka/tramp3d-v4b.cpp>
            full-name "class GlobalIDDataBase"
            needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
            pointer_to_this <pointer_type 0x7ffff494bf18> reference_to_this <reference_type 0x7fffecededc8> chain <type_decl 0x7ffff4948b48 GlobalIDDataBase>>
        sizes-gimplified public unsigned type_6 DI
        size <integer_cst 0x7ffff70dee28 constant 64>
        unit-size <integer_cst 0x7ffff70dee40 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set 484 canonical-type 0x7ffff494bf18
        pointer_to_this <pointer_type 0x7fffe4f220a8>>
   
    arg:0 <mem_ref 0x7fffe2f68fc8
        type <record_type 0x7ffff294d888 INode sizes-gimplified addressable needs-constructing type_1 type_4 type_5 type_6 BLK
            size <integer_cst 0x7ffff6f1aed0 constant 320>
            unit-size <integer_cst 0x7ffff72d4078 constant 40>
            align:64 warn_if_not_align:0 symtab:0 alias-set 604 canonical-type 0x7ffff294d888 fields <const_decl 0x7fffee5b8f50 dimensions> context <translation_unit_decl 0x7ffff70eb168 /aux/hubicka/tramp3d-v4b.cpp>
            full-name "class INode<3>"
            needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0 use_template=1 interface-unknown
            pointer_to_this <pointer_type 0x7fffee1a1f18> reference_to_this <reference_type 0x7fffee1a61f8> chain <type_decl 0x7ffff294b850 INode>>
        tree_1 tree_2
        arg:0 <addr_expr 0x7ffff56641c0 type <pointer_type 0x7fffee1a1f18>
            arg:0 <var_decl 0x7fffec51a5a0 s>>
        arg:1 <integer_cst 0x7fffe945f960 constant 0>>
    arg:1 <field_decl 0x7fffede17c78 globalIDDataBase_m type <pointer_type 0x7ffff494bf18>
        used private unsigned nonlocal decl_3 DI /aux/hubicka/tramp3d-v4b.cpp:5583:21 size <integer_cst 0x7ffff70dee28 64> unit-size <integer_cst 0x7ffff70dee40 8>
        align:64 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff70dee88 constant 16> bit-offset <integer_cst 0x7ffff70dee28 64> context <record_type 0x7ffff294d888 INode>
        chain <field_decl 0x7fffede17d10 key_m type <integer_type 0x7fffede19690 NodeKey_t>
            used private nonlocal decl_3 SI /aux/hubicka/tramp3d-v4b.cpp:5584:13
            size <integer_cst 0x7ffff70ff078 constant 32>
            unit-size <integer_cst 0x7ffff70ff090 constant 4>
            align:32 warn_if_not_align:0 offset_align 128
            offset <integer_cst 0x7ffff70ff288 constant 32>
            bit-offset <integer_cst 0x7ffff70deea0 constant 0> context <record_type 0x7ffff294d888 INode> chain <type_decl 0x7fffede17428 INode>>>
    /aux/hubicka/tramp3d-v4b.cpp:5457:24 start: /aux/hubicka/tramp3d-v4b.cpp:5457:24 finish: /aux/hubicka/tramp3d-v4b.cpp:5457:24>

that did not make sense to me becaue ref1 type is integer_type and ref2 is
pointer_type so these should be disambiguated by usual TBAA querry earlier.

What happens is the following.  The analysis happens via vn_reference_lookup
which does
      if (! tbaa_p)
        r.ref_alias_set = r.base_alias_set = 0;
later this is passed as ref1 but because it reffers to decl the order is
exchanged so it appears as ref2 in in indirect_ref_may_alias_decl_p.
Now there is test:
  /* If the alias set for a pointer access is zero all bets are off.  */       
  if (base1_alias_set == 0)                                                    
    return true;                                                               
which is ok, since base1_alias_set is non-0 (it is coming from indirect_ref).

I think the code is not supposed to work this way :)

I not convinced we realy want to give up on TBAA when ref is actual
decl?  At least polymorphic call analysis is using the assumption that
placement news do not happen here.

This patch fixes the odd case in conservative way and also reduces
number of disambiguations noticeably:

from
  aliasing_component_ref_p: 636 disambiguations, 15844 queries
to
  aliasing_component_ref_p: 136 disambiguations, 13943 queries

Can we construct valid placement new testcase where this matters?

Honza

	* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
	TBAA path when base2_alias_set is 0.
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c	(revision 271813)
+++ tree-ssa-alias.c	(working copy)
@@ -1295,7 +1296,7 @@ indirect_ref_may_alias_decl_p (tree ref1
   ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
 
   /* If the alias set for a pointer access is zero all bets are off.  */
-  if (base1_alias_set == 0)
+  if (base1_alias_set == 0 || base2_alias_set == 0)
     return true;
 
   /* When we are trying to disambiguate an access with a pointer dereference

Re: Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 7997 bytes --]

On Sat, 1 Jun 2019, Jan Hubicka wrote:

> Hi,
> while looking for the reason for extra disambiguations in
> aliasing_component_refs I have noticed an odd case.  We newly disambiguate:
> 
> MEM[(Element_t *)_27 + 4B];
> 
> s.globalIDDataBase_m;
> 
>  <mem_ref 0x7fffe2f68f28
>     type <integer_type 0x7ffff63ffd20 Element_t sizes-gimplified type_6 SI
>         size <integer_cst 0x7ffff70ff078 constant 32>
>         unit-size <integer_cst 0x7ffff70ff090 constant 4>
>         align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff70fb5e8 precision:32 min <integer_cst 0x7ffff70ff030 -2147483648> max <integer_cst 0x7ffff70ff048 2147483647>
>         pointer_to_this <pointer_type 0x7fffe55a3dc8>>
>     tree_0 tree_2
>     arg:0 <ssa_name 0x7fffe2f666c0
>         type <pointer_type 0x7ffff62df888 type <record_type 0x7ffff658d930 Domain>
>             public unsigned DI
>             size <integer_cst 0x7ffff70dee28 constant 64>
>             unit-size <integer_cst 0x7ffff70dee40 constant 8>
>             align:64 warn_if_not_align:0 symtab:0 alias-set 216 canonical-type 0x7ffff62df888>
>         visited
>         def_stmt _27 = &MEM[(struct OneDomain_t *)&s].D.113982.domain_m[i_26].D.110783.D.44395;
>         version:27
>         ptr-info 0x7ffff5c66540>
>     arg:1 <integer_cst 0x7fffe98babe8 type <pointer_type 0x7fffe55a3dc8> constant 4>>
>  <component_ref 0x7ffff1b8d030
>     type <pointer_type 0x7ffff494bf18
>         type <record_type 0x7ffff494b7e0 GlobalIDDataBase addressable needs-constructing type_1 type_4 type_5 type_6 BLK
>             size <integer_cst 0x7ffff4fe92d0 constant 576>
>             unit-size <integer_cst 0x7ffff4fe9048 constant 72>
>             align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff494b7e0 fields <function_decl 0x7fffecee2000 __dt > context <translation_unit_decl 0x7ffff70eb168 /aux/hubicka/tramp3d-v4b.cpp>
>             full-name "class GlobalIDDataBase"
>             needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
>             pointer_to_this <pointer_type 0x7ffff494bf18> reference_to_this <reference_type 0x7fffecededc8> chain <type_decl 0x7ffff4948b48 GlobalIDDataBase>>
>         sizes-gimplified public unsigned type_6 DI
>         size <integer_cst 0x7ffff70dee28 constant 64>
>         unit-size <integer_cst 0x7ffff70dee40 constant 8>
>         align:64 warn_if_not_align:0 symtab:0 alias-set 484 canonical-type 0x7ffff494bf18
>         pointer_to_this <pointer_type 0x7fffe4f220a8>>
>    
>     arg:0 <mem_ref 0x7fffe2f68fc8
>         type <record_type 0x7ffff294d888 INode sizes-gimplified addressable needs-constructing type_1 type_4 type_5 type_6 BLK
>             size <integer_cst 0x7ffff6f1aed0 constant 320>
>             unit-size <integer_cst 0x7ffff72d4078 constant 40>
>             align:64 warn_if_not_align:0 symtab:0 alias-set 604 canonical-type 0x7ffff294d888 fields <const_decl 0x7fffee5b8f50 dimensions> context <translation_unit_decl 0x7ffff70eb168 /aux/hubicka/tramp3d-v4b.cpp>
>             full-name "class INode<3>"
>             needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0 use_template=1 interface-unknown
>             pointer_to_this <pointer_type 0x7fffee1a1f18> reference_to_this <reference_type 0x7fffee1a61f8> chain <type_decl 0x7ffff294b850 INode>>
>         tree_1 tree_2
>         arg:0 <addr_expr 0x7ffff56641c0 type <pointer_type 0x7fffee1a1f18>
>             arg:0 <var_decl 0x7fffec51a5a0 s>>
>         arg:1 <integer_cst 0x7fffe945f960 constant 0>>
>     arg:1 <field_decl 0x7fffede17c78 globalIDDataBase_m type <pointer_type 0x7ffff494bf18>
>         used private unsigned nonlocal decl_3 DI /aux/hubicka/tramp3d-v4b.cpp:5583:21 size <integer_cst 0x7ffff70dee28 64> unit-size <integer_cst 0x7ffff70dee40 8>
>         align:64 warn_if_not_align:0 offset_align 128
>         offset <integer_cst 0x7ffff70dee88 constant 16> bit-offset <integer_cst 0x7ffff70dee28 64> context <record_type 0x7ffff294d888 INode>
>         chain <field_decl 0x7fffede17d10 key_m type <integer_type 0x7fffede19690 NodeKey_t>
>             used private nonlocal decl_3 SI /aux/hubicka/tramp3d-v4b.cpp:5584:13
>             size <integer_cst 0x7ffff70ff078 constant 32>
>             unit-size <integer_cst 0x7ffff70ff090 constant 4>
>             align:32 warn_if_not_align:0 offset_align 128
>             offset <integer_cst 0x7ffff70ff288 constant 32>
>             bit-offset <integer_cst 0x7ffff70deea0 constant 0> context <record_type 0x7ffff294d888 INode> chain <type_decl 0x7fffede17428 INode>>>
>     /aux/hubicka/tramp3d-v4b.cpp:5457:24 start: /aux/hubicka/tramp3d-v4b.cpp:5457:24 finish: /aux/hubicka/tramp3d-v4b.cpp:5457:24>
> 
> that did not make sense to me becaue ref1 type is integer_type and ref2 is
> pointer_type so these should be disambiguated by usual TBAA querry earlier.
> 
> What happens is the following.  The analysis happens via vn_reference_lookup
> which does
>       if (! tbaa_p)
>         r.ref_alias_set = r.base_alias_set = 0;
> later this is passed as ref1 but because it reffers to decl the order is
> exchanged so it appears as ref2 in in indirect_ref_may_alias_decl_p.
> Now there is test:
>   /* If the alias set for a pointer access is zero all bets are off.  */       
>   if (base1_alias_set == 0)                                                    
>     return true;                                                               
> which is ok, since base1_alias_set is non-0 (it is coming from indirect_ref).
> 
> I think the code is not supposed to work this way :)
> 
> I not convinced we realy want to give up on TBAA when ref is actual
> decl?  At least polymorphic call analysis is using the assumption that
> placement news do not happen here.
> 
> This patch fixes the odd case in conservative way and also reduces
> number of disambiguations noticeably:
> 
> from
>   aliasing_component_ref_p: 636 disambiguations, 15844 queries
> to
>   aliasing_component_ref_p: 136 disambiguations, 13943 queries
> 
> Can we construct valid placement new testcase where this matters?

I think the patch is correct.  Consider the decl being accessed
via memcpy which will result in base2_alias_set == 0.  The
GIMPLE memory model says decls are just random storage and thus
their dynamic type can change.  This is also why there's the
"strange" (and also incomplete...) give-ups on "this looks like
a view-conversion" are there.

Note it's easiest to build GIMPLE testcases for simple disambiguations
these days ;)  It's also best to quote what we disambiguate by
dumping the stmts with TDF_GIMPLE since that shows everything
semantically important.

But does the patch really make a difference?

  if (base1_alias_set != base2_alias_set
      && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
    return false;

Should catch this since everything conflicts with zero?  So I
wonder how your statistcs can be correct?  The base1_alias_set == 0
check is redundant as well apart from the case where both
are zero which is when we end up skipping the above test.

Richard.


> Honza
> 
> 	* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
> 	TBAA path when base2_alias_set is 0.
> Index: tree-ssa-alias.c
> ===================================================================
> --- tree-ssa-alias.c	(revision 271813)
> +++ tree-ssa-alias.c	(working copy)
> @@ -1295,7 +1296,7 @@ indirect_ref_may_alias_decl_p (tree ref1
>    ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
>  
>    /* If the alias set for a pointer access is zero all bets are off.  */
> -  if (base1_alias_set == 0)
> +  if (base1_alias_set == 0 || base2_alias_set == 0)
>      return true;
>  
>    /* When we are trying to disambiguate an access with a pointer dereference
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

Re: Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

[Jan Hubicka]

> I think the patch is correct.  Consider the decl being accessed
> via memcpy which will result in base2_alias_set == 0.  The
> GIMPLE memory model says decls are just random storage and thus
> their dynamic type can change.  This is also why there's the
> "strange" (and also incomplete...) give-ups on "this looks like
> a view-conversion" are there.

In decl variant of oracle we test:

  /* If the alias set for a pointer access is zero all bets are off.  */
  if (base1_alias_set == -1)
    base1_alias_set = get_deref_alias_set (ptrtype1);
  if (base1_alias_set == 0)
    return true;
  if (base2_alias_set == -1)
    base2_alias_set = get_alias_set (base2);

While for two indirect refs we test:

 /* If the alias set for a pointer access is zero all bets are off.  */
  if (base1_alias_set == -1)
    base1_alias_set = get_deref_alias_set (ptrtype1);
  if (base1_alias_set == 0)
    return true;
  if (base2_alias_set == -1)
    base2_alias_set = get_deref_alias_set (ptrtype2);
  if (base2_alias_set == 0)
    return true;

Now assume that I have decl of some type, use it for a while and then
memcpy completely different data type to it (my understand is that you
want to support this in GIMPLE memory model)

Now VN sees the original type of decl in the original access. It knows
it does not want to use TBAA and to disable it sets base2_alias_set to 0.
(It sets base1_alias_set but things are swapped earlier in
refs_may_alias_p_1.

Now since base1!=0, alias oracle continues. TBAA intersection checks
fails, but eventually it succeeds with disambiguation based on
access path.

I do not think it makes sense to trust access path from the original
access to decl when we expect dynamic type to change.  For me this has
shown as unexpected sucesses of the access path disambiguation which is
only possible if alias set check was disabled.
> 
> Note it's easiest to build GIMPLE testcases for simple disambiguations
> these days ;)  It's also best to quote what we disambiguate by
> dumping the stmts with TDF_GIMPLE since that shows everything
> semantically important.
> 
> But does the patch really make a difference?
> 
>   if (base1_alias_set != base2_alias_set
>       && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
>     return false;
> 
> Should catch this since everything conflicts with zero?  So I
> wonder how your statistcs can be correct?  The base1_alias_set == 0
> check is redundant as well apart from the case where both
> are zero which is when we end up skipping the above test.

Everything conflicts with 0 and thus the check fails and
oracle continues eventually to aliasing_component_refs which returns false :)

Honza
> 
> Richard.
> 
> 
> > Honza
> > 
> > 	* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
> > 	TBAA path when base2_alias_set is 0.
> > Index: tree-ssa-alias.c
> > ===================================================================
> > --- tree-ssa-alias.c	(revision 271813)
> > +++ tree-ssa-alias.c	(working copy)
> > @@ -1295,7 +1296,7 @@ indirect_ref_may_alias_decl_p (tree ref1
> >    ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
> >  
> >    /* If the alias set for a pointer access is zero all bets are off.  */
> > -  if (base1_alias_set == 0)
> > +  if (base1_alias_set == 0 || base2_alias_set == 0)
> >      return true;
> >  
> >    /* When we are trying to disambiguate an access with a pointer dereference
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> GF: Felix ImendĂśrffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NĂźrnberg)

Re: Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

[Jan Hubicka]

Hi,
as discussed on IRC today, after all this patch should be correct.  I
have re-tested it with x86_64-linux in the following variant which also
moves load of ptrtype1 that is unnecesarily early.

Bootstrapped/regtested x86_64-linux, OK?

	* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
	TBAA path when base2_alias_set is 0.

Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c	(revision 272614)
+++ tree-ssa-alias.c	(working copy)
@@ -1458,10 +1466,8 @@ indirect_ref_may_alias_decl_p (tree ref1
   if (!flag_strict_aliasing || !tbaa_p)
     return true;
 
-  ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
-
   /* If the alias set for a pointer access is zero all bets are off.  */
-  if (base1_alias_set == 0)
+  if (base1_alias_set == 0 || base2_alias_set == 0)
     return true;
 
   /* When we are trying to disambiguate an access with a pointer dereference
@@ -1479,6 +1485,9 @@ indirect_ref_may_alias_decl_p (tree ref1
   if (base1_alias_set != base2_alias_set
       && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
     return false;
+
+  ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
+
   /* If the size of the access relevant for TBAA through the pointer
      is bigger than the size of the decl we can't possibly access the
      decl via that pointer.  */

Re: Odd behaviour of indirect_ref_may_alias_decl_p in vn oracle

[Richard Biener]

On Mon, 24 Jun 2019, Jan Hubicka wrote:

> Hi,
> as discussed on IRC today, after all this patch should be correct.  I
> have re-tested it with x86_64-linux in the following variant which also
> moves load of ptrtype1 that is unnecesarily early.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Richard.

> 	* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
> 	TBAA path when base2_alias_set is 0.
> 
> Index: tree-ssa-alias.c
> ===================================================================
> --- tree-ssa-alias.c	(revision 272614)
> +++ tree-ssa-alias.c	(working copy)
> @@ -1458,10 +1466,8 @@ indirect_ref_may_alias_decl_p (tree ref1
>    if (!flag_strict_aliasing || !tbaa_p)
>      return true;
>  
> -  ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
> -
>    /* If the alias set for a pointer access is zero all bets are off.  */
> -  if (base1_alias_set == 0)
> +  if (base1_alias_set == 0 || base2_alias_set == 0)
>      return true;
>  
>    /* When we are trying to disambiguate an access with a pointer dereference
> @@ -1479,6 +1485,9 @@ indirect_ref_may_alias_decl_p (tree ref1
>    if (base1_alias_set != base2_alias_set
>        && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
>      return false;
> +
> +  ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
> +
>    /* If the size of the access relevant for TBAA through the pointer
>       is bigger than the size of the decl we can't possibly access the
>       decl via that pointer.  */