From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9242 invoked by alias); 15 Feb 2014 10:03:06 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 9219 invoked by uid 89); 15 Feb 2014 10:03:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Sat, 15 Feb 2014 10:03:03 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id D9864ABA6; Sat, 15 Feb 2014 10:03:00 +0000 (UTC) Date: Sat, 15 Feb 2014 10:03:00 -0000 From: Richard Biener To: Jan Hubicka cc: gcc@gcc.gnu.org, jason@redhat.com Subject: Re: TYPE_BINFO and canonical types at LTO In-Reply-To: <20140214165219.GA3380@kam.mff.cuni.cz> Message-ID: References: <20140214004827.GA30858@kam.mff.cuni.cz> <20140214165219.GA3380@kam.mff.cuni.cz> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2014-02/txt/msg00228.txt.bz2 On Fri, 14 Feb 2014, Jan Hubicka wrote: > > > This smells bad, since it is given a canonical type that is after the > > > structural equivalency merging that ignores BINFOs, so it may be completely > > > different class with completely different bases than the original. Bases are > > > structuraly merged, too and may be exchanged for normal fields because > > > DECL_ARTIFICIAL (that separate bases and fields) does not seem to be part of > > > the canonical type definition in LTO. > > > > Can you elaborate on that DECL_ARTIFICIAL thing? That is, what is broken > > by considering all fields during that merging? > > To make the code work with LTO, one can not merge > struct B {struct A a} > struct B: A {} > > these IMO differ only by DECL_ARTIFICIAL flag on the fields. "The code" == that BINFO walk? Is that because we walk a completely unrelated BINFO chain? I'd say we should have merged its types so that difference shouldn't matter. Hopefully ;) > > > > Note that the BINFO walk below only adds extra aliasing - it should > > be harmless correctness-wise, no? > > If it is needed for the second case, then it we will produce wrong code > with LTO when we merge first and second case toghetr. If it is not > needed, then we are safe but we don't really need the loop then. > > Having the testcase where the BINFO walk is needed for corectness, I can turn > it into wrong code in LTO by interposing the class by structure. > > I am rebuilding firefox with sanity checking that the second loop never adds anything > useful. Lets see. > > > > > I wonder if that code is needed after all: > > > case QUAL_UNION_TYPE: > > > /* Recursively record aliases for the base classes, if there are any. */ > > > if (TYPE_BINFO (type)) > > > { > > > int i; > > > tree binfo, base_binfo; > > > > > > for (binfo = TYPE_BINFO (type), i = 0; > > > BINFO_BASE_ITERATE (binfo, i, base_binfo); i++) > > > record_alias_subset (superset, > > > get_alias_set (BINFO_TYPE (base_binfo))); > > > } > > > for (field = TYPE_FIELDS (type); field != 0; field = DECL_CHAIN (field)) > > > if (TREE_CODE (field) == FIELD_DECL && !DECL_NONADDRESSABLE_P (field)) > > > record_alias_subset (superset, get_alias_set (TREE_TYPE (field))); > > > break; > > > all bases are also fields of within the type, so the second loop should notice > > > all the types seen by first loop if I am correct? > > > So perhaps the loop can be dropped at first place. > > > > Yeah, I remember seeing that code and thinking the same multiple times. > > Though I also vaguely remember that removing that loop regressed > > something. How is virtual inheritance represented in the fields? > > struct A {int a;}; > struct B: virtual A {}; > struct C: virtual A {}; > struct D: B, C {}; > struct D *d; > struct B *b; > int t(void) > { > d->a=1; > return b->a; > } > > I srepresented as: > > size constant 192> > unit size constant 24> > align 64 symtab 0 alias set 6 canonical type 0x7ffff6c58000 > fields type size > unit size > align 64 symtab 0 alias set 3 canonical type 0x7ffff6c452a0 fields context > full-name "struct B" > needs-constructor X() X(constX&) this=(X&) n_parents=1 use_template=0 interface-unknown > pointer_to_this chain > > ignored decl_6 BLK file t.C line 4 col 8 > size > unit size > align 64 offset_align 128 > offset > bit offset context > chain > ignored decl_6 BLK file t.C line 4 col 8 size unit size > align 64 offset_align 128 offset bit offset context chain >> context > full-name "struct D" > needs-constructor X() X(constX&) this=(X&) n_parents=2 use_template=0 interface-unknown > pointer_to_this chain > > > > You my end up with A being a field, too, but that only bypas the recursion. > > > > > But I'd be happy if this BINFO user would go away ;) > > Me too, indeed. > > > > (similar in general for the get_alias_set langhook - with LTO we > > only preserve extra alias-set zero answers from that) > > I see, I was surprised we construct alias sets at LTO time, I believed we > always stream them in&out. No, we stream out a flag basically, get_alias_set () == 0. We don't have a way to merge the alias set forests. Richard. > Honza