From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100587 invoked by alias); 11 Apr 2017 18:47:02 -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 99396 invoked by uid 89); 11 Apr 2017 18:46:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy= X-HELO: mail-io0-f176.google.com Received: from mail-io0-f176.google.com (HELO mail-io0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Apr 2017 18:46:52 +0000 Received: by mail-io0-f176.google.com with SMTP id l7so13083956ioe.3 for ; Tue, 11 Apr 2017 11:46:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ECgGpwJI1Wrs8GrwHYiSJKDNTx/7V08H2U6lgEzTFsc=; b=bPdsVgS/7mBXIUpbhIF3fbe0CuWjIAubsDDfj2GRV1sEAQBkrgkKspaeuTQYBxq3m6 0zDsUQCYazu+qBnz181+2WRmCtpiQfO+mr/sSQvkwUW0F1gTr/YB5Yw0BrdUgukjObqj tRT02t2kxDKTmNcDmIE30y7NhohkvTY47f2zlMRf5Y/TKxqgB1pKcz0oyqCUGcICM5RO 3ZclW1I8FZbfG6K8OOPrR3r0JcZ7MEOCWacrgHPpYWoUWDA6LtwvNyU3YolhVn4DL1PY A0g520VSKMzheyK3cl/R6NVqLLdVv5BwQK3JrnVViaK7KBfkHJfg74dUfeD2tIkMWki2 ksyw== X-Gm-Message-State: AN3rC/5cBffRsdvVCp8Wy703O0abglBwDYh65IXYGT5+akLhbP1YiNBK e/AbTE+QV9fMyu0UoEU36eMdhGr5zvBI X-Received: by 10.36.33.67 with SMTP id e64mr17417785ita.42.1491936410105; Tue, 11 Apr 2017 11:46:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.59.23 with HTTP; Tue, 11 Apr 2017 11:46:29 -0700 (PDT) In-Reply-To: References: From: Jason Merrill Date: Tue, 11 Apr 2017 18:47:00 -0000 Message-ID: Subject: Re: [PATCH] Add a new type attribute always_alias (PR79671) To: Richard Biener Cc: Bernd Edlinger , Jakub Jelinek , Jonathan Wakely , Florian Weimer , GCC Patches , Jeff Law , Jan Hubicka Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00546.txt.bz2 On Tue, Apr 11, 2017 at 9:35 AM, Richard Biener wrote: > On Tue, 11 Apr 2017, Richard Biener wrote: > >> On Tue, 11 Apr 2017, Richard Biener wrote: >> >> > On Mon, 10 Apr 2017, Jason Merrill wrote: >> > >> > > On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener = wrote: >> > > > On Mon, 10 Apr 2017, Jason Merrill wrote: >> > > >> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener wrote: >> > > >> > * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_ST= ORAGE >> > > >> > for arrays of unsigned char or std::byte. >> > > >> >> > > >> I think it would be good to have a flag to select whether these >> > > >> semantics apply to any char variant and std::byte, only unsigned = char >> > > >> and std::byte, or only std::byte. >> > > > >> > > > Any suggestion? Not sure we really need it (I'm hesitant to write >> > > > all the testcases to verify it actually works). >> > > >> > > Well, there's existing code that uses plain char (e.g. boost) that I >> > > want to support. If there's a significant optimization penalty for >> > > allowing that, we probably also want to be able to limit the impact. >> > > If there isn't much penalty, let's just support all char variants. >> > >> > I haven't assessed the penalty involved but it can't be worse than >> > the situation we had in GCC 6. So I think it's reasonable to support >> > all char variants for now. One could add some instrumenting to >> > alias_set_subset_of / alias_sets_conflict_p but it would only yield >> > an upper bound on the number of failed queries (TBAA is a quite early >> > out before PTA info is used for example). >> > >> > The following variant -- added missing >> > >> > Index: gcc/cp/tree.c >> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> > --- gcc/cp/tree.c (revision 246832) >> > +++ gcc/cp/tree.c (working copy) >> > @@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t >> > as it will overwrite alignment etc. of all variants. = */ >> > TYPE_SIZE (t) =3D TYPE_SIZE (m); >> > TYPE_SIZE_UNIT (t) =3D TYPE_SIZE_UNIT (m); >> > + TYPE_TYPELESS_STORAGE (t) =3D TYPE_TYPELESS_STORAGE (m); >> > } >> > >> > TYPE_MAIN_VARIANT (t) =3D m; >> > >> > that caused LTO bootstrap to fail and removed the tree-ssa-structalias= .c >> > change (committed separately) [LTO] bootstrapped and tested ok on >> > x86_64-unknown-linux-gnu. >> > >> > I've tested some template examples and they seem to work fine. >> > >> > Ok for trunk? >> > >> > Disclaimer: there might still be an issue with cross-language LTO >> > support, but it might at most result in TYPE_TYPELESS_STORAGE >> > getting lost. Trying to craft a testcase to verify my theory. >> >> Not too difficult in the end (see below). A fix (below) is to >> not treat arrays with differing TYPE_TYPELESS_STORAGE as >> compatible for the purpose of computing TYPE_CANONICAL (and >> thus recursively structures containing them). While they'd >> still alias each other (because currently a TYPE_TYPELESS_STORAGE >> member makes the whole thing effectively alias anything) this >> results in warnings in case such a type is used in the interface >> between C and C++ (it's also considered a ODR type). >> >> t.C:17:17: warning: type of =E2=80=98bar=E2=80=99 does not match origina= l declaration >> [-Wlto-type-mismatch] >> extern "C" void bar (X *); >> ^ >> t2.c:5:6: note: =E2=80=98bar=E2=80=99 was previously declared here >> void bar (struct X *p) {} >> ^ >> >> if you add a struct X * parameter to bar(). >> >> So it's not the optimal solution here. Another fix would be to >> somehow make TYPE_TYPELESS_STORAGE "sticky" when merging canonical >> types but there's not precedent in doing this kind of thing and >> I'm not sure we'll get everything merged before computing alias >> sets. >> >> CCing Honza. > > So after discussion and some more thinking we don't really benefit > (and really can't) from having different aggregates with such > members distignuishable via their alias set. So the following > simplifies the approach and makes the above LTO bits trivial > by more following Bernds approach of assigning the types alias > set zero and instead of in the alias machinery propagate the > flag on the types. > > It should also make reference_alias_ptr_type correct and it > does the flag propagation in type layout. > > [LTO] bootstrap and regtest running on x86_64-unknown-linux-gnu. > > The C++ bits are unchanged. The C++ bits are OK. Jason