From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129179 invoked by alias); 13 Jun 2019 12:15:35 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 124421 invoked by uid 89); 13 Jun 2019 12:15:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Jun 2019 12:15:15 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 58E32AE1F; Thu, 13 Jun 2019 12:15:09 +0000 (UTC) Date: Thu, 13 Jun 2019 12:15:00 -0000 From: Richard Biener To: Jan Hubicka cc: gcc-patches@gcc.gnu.org, d@dcepelik.cz Subject: Re: indirect_ref_may_alias_decl_p fix In-Reply-To: <20190613120550.7wtzpieeszazjn7i@kam.mff.cuni.cz> Message-ID: References: <20190613120550.7wtzpieeszazjn7i@kam.mff.cuni.cz> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: multipart/mixed; BOUNDARY="-1609908220-24107733-1560428109=:10704" X-SW-Source: 2019-06/txt/msg00764.txt.bz2 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1609908220-24107733-1560428109=:10704 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT Content-length: 7566 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 SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg) ---1609908220-24107733-1560428109=:10704--