On Thu, 13 Jun 2019, Jan Hubicka wrote:
> Hi,
> after spending some time on the view converting MEM_REFs, I noticed that
> most of reasons we give up on view converts is not actually MEM_REF created
> but test
> same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (base2))
> in indirect_ref_may_alias_decl_p
>
> Here base2 is VAR_DECL while dbase2 is MEM_REF used to access it.
>
> In the testcase:
> struct a {int a1; int a2;};
> struct b:a {};
>
> struct b bvar,*bptr2;
> int
> test(void)
> {
> struct a *bptr = &bvar;
> bptr->a2=0;
> bptr2->a1=1;
> return bptr->a2;
> }
>
> We have variable of type b, while we access it via its basetype.
> This mean that TREE_TYPE (base) is "struct b" while TREE_TYPE (dbase)
> is "struct a" which is perfectly normal and we could to the access path
> tests at least in the same strength as we would do if bptr=$bvar was
> not visible to compiler (in that case we optimize things correctly).
>
> Of course later in aliasing_component_refs_p we should not assume that
> "struct a" is the type of memory slot since the access path may contain
> b, but I think we can not assume that about "struct b" either, see below.
>
> We should not give up on this case and just proceed the same way as
> indirect_refs_may_alias_p does. In fact I would like to commonize the
> access path oracle of these functions incremetally but first I want to
> drop main differences. In particular
>
> 1) indirect_refs_may_alias_decl_p passing ref2_is_decl as true to
> aliasing_component_refs_p.
>
> This makes aliasing_component_refs_p to assume that all access paths
> conflicting with REF2 must start by type of BASE2 or its subtype.
>
> IMO this is not quite right in gimple memory model where decls are just
> untyped memory slots, since I can, for example, I can rewrite decl
> of type a by a new data of type struct b {struct a a;};
> which will confuse this logic.
The above check you complain about guards against this.
> I will try to get rid of this incrementally - I would like to have it
> logged how much optimization we lose here.
>
> 2) indirect_refs_may_alias_decl_p does
>
> if ((TREE_CODE (base1) != TARGET_MEM_REF
>
> || (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
>
> && same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (dbase2)) == 1
>
> && (TREE_CODE (TREE_TYPE (base1)) != ARRAY_TYPE
>
> || (TYPE_SIZE (TREE_TYPE (base1))
>
> && TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) == INTEGER_CST)))
>
> return ranges_maybe_overlap_p (doffset1, max_size1, doffset2, max_size2);
>
>
> while indirect_refs_may_alias_p does:
>
> if ((TREE_CODE (base1) != TARGET_MEM_REF
>
> || (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
>
> && (TREE_CODE (base2) != TARGET_MEM_REF
>
> || (!TMR_INDEX (base2) && !TMR_INDEX2 (base2)))
>
> && same_type_for_tbaa (TREE_TYPE (ptrtype1),
>
> TREE_TYPE (ptrtype2)) == 1
>
> /* But avoid treating arrays as "objects", instead assume they
>
> can overlap by an exact multiple of their element size.
>
> See gcc.dg/torture/alias-2.c. */
>
> && TREE_CODE (TREE_TYPE (ptrtype1)) != ARRAY_TYPE)
>
> return ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2);
>
>
> Sincce we already checked that TREEE_TYPE (ptrtype) is same as TREE_TYPE
> (base1)
> the same_type_for_tbaa check is equivalent in both.
>
> Notice however that first tests that array is VLA, while other
> supports partial overlaps on all array types. I suppose we want to
> choose which way we support that and go with one or another.
Let's go with the stricter check for the purpose of unification and work
on this issue as followup.
> Of course even in that case overlap check is not completely lost,
> I attached testcase for that to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90869
> All we need is to wrap the checks by array size.
>
> With these differences sorted out I think both functions may dispatch to
> common access path oracle after doing the case specific work.
>
> Bootstrapped/regtested x86_64-linux, makes sense?
Yes, the patch below makes sense.
Thanks,
Richard.
> PR tree-optimize/90869
> * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Watch for view
> converts in MEM_REF referencing decl rather than view converts
> from decl type to MEM_REF type.
>
> * g++.dg/tree-ssa/alias-access-path-1.C: New testcase.
> Index: tree-ssa-alias.c
> ===================================================================
> --- tree-ssa-alias.c (revision 272037)
> +++ tree-ssa-alias.c (working copy)
> @@ -1370,11 +1410,16 @@ indirect_ref_may_alias_decl_p (tree ref1
> poly_offset_int doffset2 = offset2;
> if (TREE_CODE (dbase2) == MEM_REF
> || TREE_CODE (dbase2) == TARGET_MEM_REF)
> - doffset2 -= mem_ref_offset (dbase2) << LOG2_BITS_PER_UNIT;
> + {
> + doffset2 -= mem_ref_offset (dbase2) << LOG2_BITS_PER_UNIT;
> + tree ptrtype2 = TREE_TYPE (TREE_OPERAND (dbase2, 1));
> + /* If second reference is view-converted, give up now. */
> + if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) != 1)
> + return true;
> + }
>
> - /* If either reference is view-converted, give up now. */
> - if (same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) != 1
> - || same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (base2)) != 1)
> + /* If first reference is view-converted, give up now. */
> + if (same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) != 1)
> return true;
>
> /* If both references are through the same type, they do not alias
> @@ -1408,7 +1457,13 @@ indirect_ref_may_alias_decl_p (tree ref1
> offset1, max_size1,
> ref2,
> ref2_alias_set, base2_alias_set,
> - offset2, max_size2, true);
> + offset2, max_size2,
> + /* Only if the other reference is actual
> + decl we can safely check only toplevel
> + part of access path 1. */
> + same_type_for_tbaa (TREE_TYPE (dbase2),
> + TREE_TYPE (base2))
> + == 1);
>
> return true;
> }
> Index: testsuite/g++.dg/tree-ssa/alias-access-path-1.C
> ===================================================================
> --- testsuite/g++.dg/tree-ssa/alias-access-path-1.C (nonexistent)
> +++ testsuite/g++.dg/tree-ssa/alias-access-path-1.C (working copy)
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fdump-tree-fre1" } */
> +
> +struct a {int a1; int a2;};
> +struct b:a {};
> +
> +struct b bvar,*bptr2;
> +int
> +test(void)
> +{
> + struct a *bptr = &bvar;
> + bptr->a2=0;
> + bptr2->a1=1;
> + return bptr->a2;
> +}
> +int
> +test2(void)
> +{
> + struct b *bptr = &bvar;
> + bptr->a2=0;
> + bptr2->a1=1;
> + return bptr->a2;
> +}
> +/* { dg-final { scan-tree-dump-times "return 0" 2 "fre1" } } */
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
