From patchwork Wed Apr 19 06:22:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 752103 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 3w7BkT03mfz9s2s for ; Wed, 19 Apr 2017 16:22:40 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="Rr8JonHh"; 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:message-id:reply-to:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=VUhAhU5WlLKCrO6n0 M6BVDRTsFymOnSGvojqNb6yLhuEmfNgscLHeB0es9vchYdDMWDXT1wEtnNEtnrD7 XrP9khJwb33xqVbl1RKZdr9ckdLd7JTf9v0AooeWltW8OHjlXmXKrJ+6SyADKxZh 6hwdl8vD+CUBnN77iEMkBWFetM= 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:message-id:reply-to:references:mime-version :content-type:in-reply-to; s=default; bh=aKG9dbcnPsdHkoEdeDo261N 2iv8=; b=Rr8JonHh3RNIxTRUFzaxHbbSoeSJArp2+kiOk0mW2MTzM3Knvy7dBbn JLBz+JFC8ZytdJ2oyJQ+wLeV66dskSCDq+kVYLfhm/Z9i0GWD+O6C5W9bNnJUZQp Nb3z4vQnT82Uxh6PlrH9RVCqChnvVNq1UKXxk+fkK183dapgax/A= Received: (qmail 95636 invoked by alias); 19 Apr 2017 06:22:30 -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 95621 invoked by uid 89); 19 Apr 2017 06:22:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-16.9 required=5.0 tests=BAYES_00, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Apr 2017 06:22:28 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6791580492; Wed, 19 Apr 2017 06:22:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6791580492 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6791580492 Received: from tucnak.zalov.cz (ovpn-116-29.ams2.redhat.com [10.36.116.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0BD6771C4E; Wed, 19 Apr 2017 06:22:27 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v3J6MO7v032026; Wed, 19 Apr 2017 08:22:25 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v3J6MNLd032025; Wed, 19 Apr 2017 08:22:23 +0200 Date: Wed, 19 Apr 2017 08:22:23 +0200 From: Jakub Jelinek To: Richard Biener Cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix TYPE_TYPELESS_STORAGE handling (PR middle-end/80423) Message-ID: <20170419062223.GC1777@tucnak> Reply-To: Jakub Jelinek References: <20170418151430.GZ1809@tucnak> <23D6ECD4-AA6B-4139-8E16-C9142B90B66D@suse.de> <20170419055630.GC1809@tucnak> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170419055630.GC1809@tucnak> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes On Wed, Apr 19, 2017 at 07:56:30AM +0200, Jakub Jelinek wrote: > On Wed, Apr 19, 2017 at 07:45:36AM +0200, Richard Biener wrote: > > >As mentioned in the PR, we now use TYPE_TYPELESS_STORAGE flag on > > >ARRAY_TYPEs to denote types that need the special C++ alias handling. > > >The problem is how is that created, we just use build_array_type and > > >set TYPE_TYPELESS_STORAGE on the result, but build_array_type uses type > > >caching, so we might modify that way some other array type. > > >If all the array type creation goes through build_cplus_array_type, > > >that > > >wouldn't be a problem, as that flag is dependent just on the element > > >type, but that is not the case, c-family as well as the middle-end has > > >lots of spots that also create array types. So in the end whether > > >one gets TYPE_TYPELESS_STORAGE flag or not is quite random, depends on > > >GC etc. > > > > > >The following patch attempts to resolve this, by making the type > > >hashing > > >take that flag into account. Bootstrapped/regtested on x86_64-linux > > >and > > >i686-linux, ok for trunk? > > > > When changing the C++ function I thought that calling build_array_type was > > wrong and it should instead do the same it does in the other places, use > > its raw creation routine and then the canonical type register stuff. But > > I was hesitant to change this at this point. > > The problem is that as the patch shows, we don't need it in a single place > (the C++ FE), but at least in two places (C++ FE and c-family), and it > wouldn't surprise me if we don't need it later on in further places > (e.g. in middle-end, if we have a TYPE_TYPELESS_STORAGE array and say DSE > wants to create a smaller one with the same property). > > Using a default argument to build_array_type is likely cleaner indeed, > I'd just then also swap the arguments to build_array_type_1 (the shared > vs. typeless_storage). Here in (so far untested) patch form: 2017-04-19 Jakub Jelinek PR middle-end/80423 * tree.h (build_array_type): Add typeless_storage default argument. * tree.c (type_cache_hasher::equal): Also compare TYPE_TYPELESS_STORAGE flag for ARRAY_TYPEs. (build_array_type): Add typeless_storage argument, set TYPE_TYPELESS_STORAGE to it, if shared also hash it, and pass to recursive call. (build_nonshared_array_type): Adjust build_array_type_1 caller. (build_array_type): Likewise. Add typeless_storage argument. c-family/ * c-common.c (complete_array_type): Preserve TYPE_TYPELESS_STORAGE. cp/ * tree.c (build_cplus_array_type): Call build_array_type with the intended TYPE_TYPELESS_STORAGE flag value, instead of calling build_array_type and modifying later TYPE_TYPELESS_STORAGE on the shared type. testsuite/ * g++.dg/other/pr80423.C: New test. Jakub --- gcc/tree.h.jj 2017-04-18 15:13:25.180398014 +0200 +++ gcc/tree.h 2017-04-19 08:16:12.859844328 +0200 @@ -4068,7 +4068,7 @@ extern tree build_truth_vector_type (uns extern tree build_same_sized_truth_vector_type (tree vectype); extern tree build_opaque_vector_type (tree innertype, int nunits); extern tree build_index_type (tree); -extern tree build_array_type (tree, tree); +extern tree build_array_type (tree, tree, bool = false); extern tree build_nonshared_array_type (tree, tree); extern tree build_array_type_nelts (tree, unsigned HOST_WIDE_INT); extern tree build_function_type (tree, tree); --- gcc/tree.c.jj 2017-04-18 15:13:25.158398308 +0200 +++ gcc/tree.c 2017-04-19 08:17:50.938542960 +0200 @@ -7073,7 +7073,9 @@ type_cache_hasher::equal (type_hash *a, break; return 0; case ARRAY_TYPE: - return TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type); + return (TYPE_TYPELESS_STORAGE (a->type) + == TYPE_TYPELESS_STORAGE (b->type) + && TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type)); case RECORD_TYPE: case UNION_TYPE: @@ -8350,10 +8352,12 @@ subrange_type_for_debug_p (const_tree ty /* Construct, lay out and return the type of arrays of elements with ELT_TYPE and number of elements specified by the range of values of INDEX_TYPE. + If TYPELESS_STORAGE is true, TYPE_TYPELESS_STORAGE flag is set on the type. If SHARED is true, reuse such a type that has already been constructed. */ static tree -build_array_type_1 (tree elt_type, tree index_type, bool shared) +build_array_type_1 (tree elt_type, tree index_type, bool typeless_storage, + bool shared) { tree t; @@ -8367,6 +8371,7 @@ build_array_type_1 (tree elt_type, tree TREE_TYPE (t) = elt_type; TYPE_DOMAIN (t) = index_type; TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (elt_type); + TYPE_TYPELESS_STORAGE (t) = typeless_storage; layout_type (t); /* If the element type is incomplete at this point we get marked for @@ -8381,6 +8386,7 @@ build_array_type_1 (tree elt_type, tree hstate.add_object (TYPE_HASH (elt_type)); if (index_type) hstate.add_object (TYPE_HASH (index_type)); + hstate.add_flag (typeless_storage); t = type_hash_canon (hstate.end (), t); } @@ -8396,7 +8402,7 @@ build_array_type_1 (tree elt_type, tree = build_array_type_1 (TYPE_CANONICAL (elt_type), index_type ? TYPE_CANONICAL (index_type) : NULL_TREE, - shared); + typeless_storage, shared); } return t; @@ -8405,9 +8411,9 @@ build_array_type_1 (tree elt_type, tree /* Wrapper around build_array_type_1 with SHARED set to true. */ tree -build_array_type (tree elt_type, tree index_type) +build_array_type (tree elt_type, tree index_type, bool typeless_storage) { - return build_array_type_1 (elt_type, index_type, true); + return build_array_type_1 (elt_type, index_type, typeless_storage, true); } /* Wrapper around build_array_type_1 with SHARED set to false. */ @@ -8415,7 +8421,7 @@ build_array_type (tree elt_type, tree in tree build_nonshared_array_type (tree elt_type, tree index_type) { - return build_array_type_1 (elt_type, index_type, false); + return build_array_type_1 (elt_type, index_type, false, false); } /* Return a representation of ELT_TYPE[NELTS], using indices of type --- gcc/c-family/c-common.c.jj 2017-04-18 15:14:29.383541136 +0200 +++ gcc/c-family/c-common.c 2017-04-19 08:15:55.346076712 +0200 @@ -6386,7 +6386,6 @@ complete_array_type (tree *ptype, tree i { tree maxindex, type, main_type, elt, unqual_elt; int failure = 0, quals; - hashval_t hashcode = 0; bool overflow_p = false; maxindex = size_zero_node; @@ -6480,13 +6479,15 @@ complete_array_type (tree *ptype, tree i TYPE_DOMAIN (main_type) = build_range_type (TREE_TYPE (maxindex), build_int_cst (TREE_TYPE (maxindex), 0), maxindex); + TYPE_TYPELESS_STORAGE (main_type) = TYPE_TYPELESS_STORAGE (type); layout_type (main_type); /* Make sure we have the canonical MAIN_TYPE. */ - hashcode = iterative_hash_object (TYPE_HASH (unqual_elt), hashcode); - hashcode = iterative_hash_object (TYPE_HASH (TYPE_DOMAIN (main_type)), - hashcode); - main_type = type_hash_canon (hashcode, main_type); + inchash::hash hstate; + hstate.add_object (TYPE_HASH (unqual_elt)); + hstate.add_object (TYPE_HASH (TYPE_DOMAIN (main_type))); + hstate.add_flag (TYPE_TYPELESS_STORAGE (main_type)); + main_type = type_hash_canon (hstate.end (), main_type); /* Fix the canonical type. */ if (TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (main_type)) @@ -6497,7 +6498,8 @@ complete_array_type (tree *ptype, tree i != TYPE_DOMAIN (main_type))) TYPE_CANONICAL (main_type) = build_array_type (TYPE_CANONICAL (TREE_TYPE (main_type)), - TYPE_CANONICAL (TYPE_DOMAIN (main_type))); + TYPE_CANONICAL (TYPE_DOMAIN (main_type)), + TYPE_TYPELESS_STORAGE (main_type)); else TYPE_CANONICAL (main_type) = main_type; --- gcc/cp/tree.c.jj 2017-04-18 15:13:27.136371909 +0200 +++ gcc/cp/tree.c 2017-04-19 08:15:39.191291063 +0200 @@ -949,14 +949,14 @@ 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; + bool typeless_storage + = (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)))); + t = build_array_type (elt_type, index_type, typeless_storage); } /* Now check whether we already have this array variant. */ --- gcc/testsuite/g++.dg/other/pr80423.C.jj 2017-04-19 08:14:42.539042760 +0200 +++ gcc/testsuite/g++.dg/other/pr80423.C 2017-04-19 08:14:42.539042760 +0200 @@ -0,0 +1,11 @@ +// PR middle-end/80423 +// { dg-do compile { target c++11 } } + +typedef unsigned char uint8_t; +struct A { + template A(unsigned char (&)[N]); +}; +void fn1(A) { + uint8_t a[]{0}; + fn1(a); +}