From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76756 invoked by alias); 29 Oct 2019 20:26:21 -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 76745 invoked by uid 89); 29 Oct 2019 20:26:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=VEC, DECL_NAME, decl_name, HX-Received:4a15 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; Tue, 29 Oct 2019 20:26:18 +0000 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 65C688553B for ; Tue, 29 Oct 2019 20:26:17 +0000 (UTC) Received: by mail-qt1-f198.google.com with SMTP id c19so124803qtp.3 for ; Tue, 29 Oct 2019 13:26:17 -0700 (PDT) Received: from [192.168.1.130] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id v4sm93481qkj.28.2019.10.29.13.26.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Oct 2019 13:26:15 -0700 (PDT) Subject: Re: [C++ PATCH] Partial fix for a recent regression (PR c++/90947) To: Jakub Jelinek , Martin Sebor Cc: gcc-patches@gcc.gnu.org References: <20191022150345.GD2116@tucnak> From: Jason Merrill Message-ID: <4d7ed4e6-816a-4a15-27a7-b0180a4b108f@redhat.com> Date: Tue, 29 Oct 2019 20:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191022150345.GD2116@tucnak> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg02072.txt.bz2 On 10/22/19 11:03 AM, Jakub Jelinek wrote: > Hi! > > The following patch is just a partial fix for a regression introduced > in the PR90947 changes, the testcase is fixed for just C++17/20. > type_initializer_zero_p has been added to the generic code, supposedly > because similar initializer_zerop is in generic code too, but that > means it has a hand-written copy of next_initializable_field which can't > do exactly what next_initializable_field does. > > The following patch moves it into the C++ FE which is the only user of that > function and uses next_initializable_field in there. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > I'm afraid I'm lost on how to fix the C++11/14 case, the object > being initialized has std::atomic type, which doesn't have any > direct non-static data members, and has std::__atomic_base as base > class that has some field. The only FIELD_DECL in std::atomic > is DECL_ARTIFICIAL field for the base class, so next_initializable_field > or the hand-written variant thereof doesn't find any initializable fields > for 11/14. The initializer is initializer list { {1} } and the function > just returns true if it doesn't find any initializable fields, so in the end > we misoptimize it as { {0} }. I think type_initializer_zero_p should return false if CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will have the intended effect in that case. > 2019-10-22 Jakub Jelinek > > PR c++/90947 > * tree.h (type_initializer_zero_p): Remove. > * tree.c (type_initializer_zero_p): Remove. > cp/ > * cp-tree.h (type_initializer_zero_p): Declare. > * decl.c (reshape_init_array_1): Formatting fix. > * tree.c (type_initializer_zero_p): New function. Moved from > ../tree.c, use next_initializable_field, formatting fix. > > --- gcc/tree.h.jj 2019-09-20 12:25:13.737920929 +0200 > +++ gcc/tree.h 2019-10-22 10:07:26.411826804 +0200 > @@ -4690,12 +4690,6 @@ extern tree first_field (const_tree); > extern bool initializer_zerop (const_tree, bool * = NULL); > extern bool initializer_each_zero_or_onep (const_tree); > > -/* Analogous to initializer_zerop but also examines the type for > - which the initializer is being used. Unlike initializer_zerop, > - considers empty strings to be zero initializers for arrays and > - non-zero for pointers. */ > -extern bool type_initializer_zero_p (tree, tree); > - > extern wide_int vector_cst_int_elt (const_tree, unsigned int); > extern tree vector_cst_elt (const_tree, unsigned int); > > --- gcc/tree.c.jj 2019-10-19 09:22:14.830893404 +0200 > +++ gcc/tree.c 2019-10-22 10:08:36.501748481 +0200 > @@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre > } > } > > -/* Given an initializer INIT for a TYPE, return true if INIT is zero > - so that it can be replaced by value initialization. This function > - distinguishes betwen empty strings as initializers for arrays and > - for pointers (which make it return false). */ > - > -bool > -type_initializer_zero_p (tree type, tree init) > -{ > - if (type == error_mark_node || init == error_mark_node) > - return false; > - > - STRIP_NOPS (init); > - > - if (POINTER_TYPE_P (type)) > - return TREE_CODE (init) != STRING_CST && initializer_zerop (init); > - > - if (TREE_CODE (init) != CONSTRUCTOR) > - return initializer_zerop (init); > - > - if (TREE_CODE (type) == ARRAY_TYPE) > - { > - tree elt_type = TREE_TYPE (type); > - elt_type = TYPE_MAIN_VARIANT (elt_type); > - if (elt_type == char_type_node) > - return initializer_zerop (init); > - > - tree elt_init; > - unsigned HOST_WIDE_INT i; > - FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init) > - if (!type_initializer_zero_p (elt_type, elt_init)) > - return false; > - return true; > - } > - > - if (TREE_CODE (type) != RECORD_TYPE) > - return initializer_zerop (init); > - > - tree fld = TYPE_FIELDS (type); > - > - tree fld_init; > - unsigned HOST_WIDE_INT i; > - FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init) > - { > - /* Advance to the next member, skipping over everything that > - canot be initialized (including unnamed bit-fields). */ > - while (TREE_CODE (fld) != FIELD_DECL > - || DECL_ARTIFICIAL (fld) > - || (DECL_BIT_FIELD (fld) && !DECL_NAME (fld))) > - { > - fld = DECL_CHAIN (fld); > - if (!fld) > - return true; > - continue; > - } > - > - tree fldtype = TREE_TYPE (fld); > - if (!type_initializer_zero_p (fldtype, fld_init)) > - return false; > - > - fld = DECL_CHAIN (fld); > - if (!fld) > - break; > - } > - > - return true; > -} > - > /* Check if vector VEC consists of all the equal elements and > that the number of elements corresponds to the type of VEC. > The function returns first element of the vector > --- gcc/cp/cp-tree.h.jj 2019-10-22 08:15:53.810775827 +0200 > +++ gcc/cp/cp-tree.h 2019-10-22 10:10:57.265582861 +0200 > @@ -7379,6 +7379,11 @@ extern tree cxx_copy_lang_qualifiers (c > > extern void cxx_print_statistics (void); > extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t); > +/* Analogous to initializer_zerop but also examines the type for > + which the initializer is being used. Unlike initializer_zerop, > + considers empty strings to be zero initializers for arrays and > + non-zero for pointers. */ > +extern bool type_initializer_zero_p (tree, tree); > > /* in ptree.c */ > extern void cxx_print_xnode (FILE *, tree, int); > --- gcc/cp/decl.c.jj 2019-10-22 08:57:12.913654620 +0200 > +++ gcc/cp/decl.c 2019-10-22 10:13:39.038094034 +0200 > @@ -5982,9 +5982,8 @@ reshape_init_array_1 (tree elt_type, tre > /* Pointers initialized to strings must be treated as non-zero > even if the string is empty. */ > tree init_type = TREE_TYPE (elt_init); > - if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type))) > - last_nonzero = index; > - else if (!type_initializer_zero_p (elt_type, elt_init)) > + if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type) > + || !type_initializer_zero_p (elt_type, elt_init)) > last_nonzero = index; > > /* This can happen with an invalid initializer (c++/54501). */ > --- gcc/cp/tree.c.jj 2019-10-19 09:22:16.594866462 +0200 > +++ gcc/cp/tree.c 2019-10-22 10:10:29.151015397 +0200 > @@ -5527,6 +5527,65 @@ maybe_warn_zero_as_null_pointer_constant > return false; > } > > +/* Given an initializer INIT for a TYPE, return true if INIT is zero > + so that it can be replaced by value initialization. This function > + distinguishes betwen empty strings as initializers for arrays and > + for pointers (which make it return false). */ > + > +bool > +type_initializer_zero_p (tree type, tree init) > +{ > + if (type == error_mark_node || init == error_mark_node) > + return false; > + > + STRIP_NOPS (init); > + > + if (POINTER_TYPE_P (type)) > + return TREE_CODE (init) != STRING_CST && initializer_zerop (init); > + > + if (TREE_CODE (init) != CONSTRUCTOR) > + return initializer_zerop (init); > + > + if (TREE_CODE (type) == ARRAY_TYPE) > + { > + tree elt_type = TREE_TYPE (type); > + elt_type = TYPE_MAIN_VARIANT (elt_type); > + if (elt_type == char_type_node) > + return initializer_zerop (init); > + > + tree elt_init; > + unsigned HOST_WIDE_INT i; > + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init) > + if (!type_initializer_zero_p (elt_type, elt_init)) > + return false; > + return true; > + } > + > + if (TREE_CODE (type) != RECORD_TYPE) > + return initializer_zerop (init); > + > + tree fld = TYPE_FIELDS (type); > + > + tree fld_init; > + unsigned HOST_WIDE_INT i; > + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init) > + { > + fld = next_initializable_field (fld); > + if (!fld) > + return true; > + > + tree fldtype = TREE_TYPE (fld); > + if (!type_initializer_zero_p (fldtype, fld_init)) > + return false; > + > + fld = DECL_CHAIN (fld); > + if (!fld) > + break; > + } > + > + return true; > +} > + > #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007) > /* Complain that some language-specific thing hanging off a tree > node has been accessed improperly. */ > > Jakub >