From patchwork Tue Apr 11 10:32:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 749398 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3w2NfF5G6Fz9sNK for ; Tue, 11 Apr 2017 20:32:20 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="hmZYZwol"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=SmhbmRbftTJyNH5i UE34RgiznbSdUN4bmIvcYcN4d/EDLwH4bhgywV86CW9zE9G2o7NafL0J6oNpvh+G xhrWtinu5hIjTer8YDOBXHs56DeG/2YqFElMWl5TXbSEvF0kG8mOpkiEWhQUHZgy 2siEfUoNN8LieQGL5RNT3+L2GBg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=f8qew+962aIvYgncJfB55N wDCww=; b=hmZYZwolutxU6BLXYKvUJFqitJDp0hYi4pyUePeiGywBQTmqn8UgF+ rYTQdmI8fX0C8M8MAwZRS//VJrTUlkRNd88Gfg37ZBcvmS3pmy2BiX7uGfWccuct SeI6dn7Xomi5SIPM1D7plMljDpLVNfvbRwZv/CaznUfF09mOXPvxw= Received: (qmail 26369 invoked by alias); 11 Apr 2017 10:32:11 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 26328 invoked by uid 89); 11 Apr 2017 10:32:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=craft X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Apr 2017 10:32:08 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 53CF5ABBD; Tue, 11 Apr 2017 10:32:07 +0000 (UTC) Date: Tue, 11 Apr 2017 12:32:06 +0200 (CEST) From: Richard Biener To: Jason Merrill cc: Bernd Edlinger , Jakub Jelinek , Jonathan Wakely , Florian Weimer , GCC Patches , Jeff Law Subject: Re: [PATCH] Add a new type attribute always_alias (PR79671) In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 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_STORAGE > >> > 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 =================================================================== --- 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) = TYPE_SIZE (m); TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m); + TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m); } TYPE_MAIN_VARIANT (t) = 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. Thanks, Richard. 2017-04-11 Richard Biener PR c++/79671 cp/ * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE for arrays of unsigned char or std::byte. * tree-core.h (tree_type_common): Add typeless_storage flag. * tree.h (TYPE_TYPELESS_STORAGE): New macro. (canonical_type_used_p): Add arrays with TYPE_TYPELESS_STORAGE. * alias.c (alias_set_entry): Add has_typeless_storage member. (alias_set_subset_of): Handle it. (alias_sets_conflict_p): Likewise. (init_alias_set_entry): Initialize it. (get_alias_set): For ARRAY_TYPEs handle TYPE_TYPELESS_STORAGE. (record_alias_subset): Likewise. * lto-streamer-out.c (hash_tree): Hash TYPE_TYPELESS_STORAGE. * tree-streamer-in.c (unpack_ts_type_common_value_fields): Stream TYPE_TYPELESS_STORAGE. * tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise. lto/ * lto.c (compare_tree_sccs_1): Compare TYPE_TYPELESS_STORAGE. * g++.dg/torture/pr79671.C: New testcase. Index: gcc/alias.c =================================================================== --- gcc/alias.c (revision 246832) +++ gcc/alias.c (working copy) @@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry { bool is_pointer; /* Nonzero if is_pointer or if one of childs have has_pointer set. */ bool has_pointer; + /* Nonzero if we have a child serving as typeless storage (or are + such storage ourselves). */ + bool has_typeless_storage; /* The children of the alias set. These are not just the immediate children, but, in fact, all descendants. So, if we have: @@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1 /* Check if set1 is a subset of set2. */ ase2 = get_alias_set_entry (set2); if (ase2 != 0 - && (ase2->has_zero_child + && (ase2->has_typeless_storage + || ase2->has_zero_child || (ase2->children && ase2->children->get (set1)))) return true; @@ -480,7 +484,8 @@ alias_sets_conflict_p (alias_set_type se /* See if the first alias set is a subset of the second. */ ase1 = get_alias_set_entry (set1); if (ase1 != 0 - && ase1->children && ase1->children->get (set2)) + && (ase1->has_typeless_storage + || (ase1->children && ase1->children->get (set2)))) { ++alias_stats.num_dag; return 1; @@ -489,7 +494,8 @@ alias_sets_conflict_p (alias_set_type se /* Now do the same, but with the alias sets reversed. */ ase2 = get_alias_set_entry (set2); if (ase2 != 0 - && ase2->children && ase2->children->get (set1)) + && (ase2->has_typeless_storage + || (ase2->children && ase2->children->get (set1)))) { ++alias_stats.num_dag; return 1; @@ -825,6 +831,7 @@ init_alias_set_entry (alias_set_type set ase->has_zero_child = false; ase->is_pointer = false; ase->has_pointer = false; + ase->has_typeless_storage = false; gcc_checking_assert (!get_alias_set_entry (set)); (*alias_sets)[set] = ase; return ase; @@ -955,6 +962,7 @@ get_alias_set (tree t) Just be pragmatic here and make sure the array and its element type get the same alias set assigned. */ else if (TREE_CODE (t) == ARRAY_TYPE + && ! TYPE_TYPELESS_STORAGE (t) && (!TYPE_NONALIASED_COMPONENT (t) || TYPE_STRUCTURAL_EQUALITY_P (t))) set = get_alias_set (TREE_TYPE (t)); @@ -1094,6 +1102,15 @@ get_alias_set (tree t) TYPE_ALIAS_SET (t) = set; + if (TREE_CODE (t) == ARRAY_TYPE + && TYPE_TYPELESS_STORAGE (t)) + { + alias_set_entry *ase = get_alias_set_entry (set); + if (!ase) + ase = init_alias_set_entry (set); + ase->has_typeless_storage = true; + } + /* If this is an aggregate type or a complex type, we must record any component aliasing information. */ if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE) @@ -1173,6 +1190,8 @@ record_alias_subset (alias_set_type supe superset_entry->has_zero_child = true; if (subset_entry->has_pointer) superset_entry->has_pointer = true; + if (subset_entry->has_typeless_storage) + superset_entry->has_typeless_storage = true; if (subset_entry->children) { Index: gcc/cp/tree.c =================================================================== --- gcc/cp/tree.c (revision 246832) +++ gcc/cp/tree.c (working copy) @@ -949,6 +949,13 @@ build_cplus_array_type (tree elt_type, t else { t = build_array_type (elt_type, index_type); + if (elt_type == unsigned_char_type_node + || elt_type == signed_char_type_node + || elt_type == char_type_node + || (TREE_CODE (elt_type) == ENUMERAL_TYPE + && TYPE_CONTEXT (elt_type) == std_node + && !strcmp ("byte", TYPE_NAME_STRING (elt_type)))) + TYPE_TYPELESS_STORAGE (t) = 1; } /* Now check whether we already have this array variant. */ @@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t as it will overwrite alignment etc. of all variants. */ TYPE_SIZE (t) = TYPE_SIZE (m); TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m); + TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m); } TYPE_MAIN_VARIANT (t) = m; Index: gcc/lto/lto.c =================================================================== --- gcc/lto/lto.c (revision 246832) +++ gcc/lto/lto.c (working copy) @@ -1161,7 +1161,10 @@ compare_tree_sccs_1 (tree t1, tree t2, t compare_values (TYPE_FINAL_P); } else if (code == ARRAY_TYPE) - compare_values (TYPE_NONALIASED_COMPONENT); + { + compare_values (TYPE_NONALIASED_COMPONENT); + compare_values (TYPE_TYPELESS_STORAGE); + } compare_values (TYPE_PACKED); compare_values (TYPE_RESTRICT); compare_values (TYPE_USER_ALIGN); Index: gcc/lto-streamer-out.c =================================================================== --- gcc/lto-streamer-out.c (revision 246832) +++ gcc/lto-streamer-out.c (working copy) @@ -1142,7 +1142,10 @@ hash_tree (struct streamer_tree_cache_d hstate.add_flag (TYPE_FINAL_P (t)); } else if (code == ARRAY_TYPE) - hstate.add_flag (TYPE_NONALIASED_COMPONENT (t)); + { + hstate.add_flag (TYPE_NONALIASED_COMPONENT (t)); + hstate.add_flag (TYPE_TYPELESS_STORAGE (t)); + } hstate.commit_flag (); hstate.add_int (TYPE_PRECISION (t)); hstate.add_int (TYPE_ALIGN (t)); Index: gcc/testsuite/g++.dg/torture/pr79671.C =================================================================== --- gcc/testsuite/g++.dg/torture/pr79671.C (nonexistent) +++ gcc/testsuite/g++.dg/torture/pr79671.C (working copy) @@ -0,0 +1,25 @@ +// { dg-do run } + +void *operator new(__SIZE_TYPE__, void *p2) { return p2; } +struct B { B(int i_) : i(i_) {} int i; }; +struct X +{ + unsigned char buf[sizeof (B)]; +}; + +int __attribute__((noinline)) foo() +{ + X x alignas(B), y alignas(B); + new (&x) B (0); + y = x; + B *q = reinterpret_cast (&y); + asm volatile ("" : "=r" (q) : "r" (q)); + return q->i; +} + +int main() +{ + if (foo() != 0) + __builtin_abort (); + return 0; +} Index: gcc/tree-core.h =================================================================== --- gcc/tree-core.h (revision 246832) +++ gcc/tree-core.h (working copy) @@ -1511,7 +1511,9 @@ struct GTY(()) tree_type_common { so we need to store the value 32 (not 31, as we need the zero as well), hence six bits. */ unsigned align : 6; - unsigned spare : 25; + unsigned typeless_storage : 1; + unsigned spare : 24; + alias_set_type alias_set; tree pointer_to; tree reference_to; Index: gcc/tree-streamer-in.c =================================================================== --- gcc/tree-streamer-in.c (revision 246832) +++ gcc/tree-streamer-in.c (working copy) @@ -375,7 +375,10 @@ unpack_ts_type_common_value_fields (stru TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1); } else if (TREE_CODE (expr) == ARRAY_TYPE) - TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1); + { + TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1); + TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1); + } TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp); SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp)); #ifdef ACCEL_COMPILER Index: gcc/tree-streamer-out.c =================================================================== --- gcc/tree-streamer-out.c (revision 246832) +++ gcc/tree-streamer-out.c (working copy) @@ -327,7 +327,10 @@ pack_ts_type_common_value_fields (struct bp_pack_value (bp, TYPE_FINAL_P (expr), 1); } else if (TREE_CODE (expr) == ARRAY_TYPE) - bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1); + { + bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1); + bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1); + } bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr)); bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr)); } Index: gcc/tree.h =================================================================== --- gcc/tree.h (revision 246832) +++ gcc/tree.h (working copy) @@ -2035,6 +2035,9 @@ extern machine_mode element_mode (const_ #define TYPE_NONALIASED_COMPONENT(NODE) \ (ARRAY_TYPE_CHECK (NODE)->type_common.transparent_aggr_flag) +#define TYPE_TYPELESS_STORAGE(NODE) \ + (ARRAY_TYPE_CHECK (NODE)->type_common.typeless_storage) + /* Indicated that objects of this type should be laid out in as compact a way as possible. */ #define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->base.u.bits.packed_flag) @@ -4914,7 +4917,7 @@ inline bool canonical_type_used_p (const_tree t) { return !(POINTER_TYPE_P (t) - || TREE_CODE (t) == ARRAY_TYPE + || (TREE_CODE (t) == ARRAY_TYPE && ! TYPE_TYPELESS_STORAGE (t)) || TREE_CODE (t) == VECTOR_TYPE); }