From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 12BAB3858D1E for ; Mon, 6 Feb 2023 09:31:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 12BAB3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id ABAF3387D8; Mon, 6 Feb 2023 09:31:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1675675882; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Uv1cJ3NPHdC6Ia1n1oDqdXYMQEhLaQJUe3kBybDXDN0=; b=bpJbNI27ComLJCOecTrZZ05niGxnCe+1FuBXWmbmnkz2CvfWfluDWLv/B3zADCmACIV3pN NzJW4WhFpQPv9Xk2BGXCIt84FW8c/7WIQRLIHv01kpNFIVd2VGeEf5U+msppcAXmzjLBf/ ZBR/wkomxnGVxQAyh1WkZuq1y+fsiq0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1675675882; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Uv1cJ3NPHdC6Ia1n1oDqdXYMQEhLaQJUe3kBybDXDN0=; b=WkJRG2aI+scxt5BSXZ+IMZ7ZmLKlqPpdW5pognKDHOkcyxtKU8GFBDGDNyk5wENTb5S+gc H30zmHFf9iThddAw== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 14F042C141; Mon, 6 Feb 2023 09:31:22 +0000 (UTC) Date: Mon, 6 Feb 2023 09:31:22 +0000 (UTC) From: Richard Biener To: Qing Zhao cc: "gcc-patches@gcc.gnu.org" , "siddhesh@gotplt.org" , "keescook@chromium.org" , "Joseph S. Myers" Subject: Re: [PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] In-Reply-To: Message-ID: References: <20230131141140.3610133-1-qing.zhao@oracle.com> <20230131141140.3610133-2-qing.zhao@oracle.com> <812910BC-870E-4432-870D-538024F1A510@oracle.com> <3B30CFBF-5004-41A4-940D-1F23C010403B@oracle.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 3 Feb 2023, Qing Zhao wrote: > > > > On Feb 3, 2023, at 2:49 AM, Richard Biener wrote: > > > > On Thu, 2 Feb 2023, Qing Zhao wrote: > > > >> > >> > >>> On Feb 2, 2023, at 8:54 AM, Richard Biener wrote: > >>> > >>> On Thu, 2 Feb 2023, Qing Zhao wrote: > >>> > >>>> > >>>> > > > > [...] > > > >>>>>>>> + return flexible_size_type_p (TREE_TYPE (last)); > >>>>>>> > >>>>>>> For types with many members this can become quite slow (IIRC we had > >>>>>>> bugs about similar walks of all fields in types), and this function > >>>>>>> looks like it's invoked multiple times on the same type per TU. > >>>>>>> > >>>>>>> In principle the property is fixed at the time we lay out a record > >>>>>>> type, so we might want to compute it at that time and record the > >>>>>>> result. > >>>>>> > >>>>>> You mean in FE? > >>>>> > >>>>> Yes, either in the frontend or in the middle-ends layout_type. > >>>>> > >>>>>> Yes, that?s better and cleaner. > >>>>>> > >>>>>> I will add one more field in the TYPE structure to record this information and check this field during middle end. > >>>>>> > >>>>>> I had the same thought in the beginning, but not sure whether adding a > >>>>>> new field in IR is necessary or not, other places in middle end might > >>>>>> not use this new field. > >>>>> > >>>>> It might be interesting to search for other code walking all fields of > >>>>> a type to determine this or similar info. > >>>> > >>>> There is one which is defined in tree.cc but only is referenced in c/c-decl.cc: > >>>> > >>>> /* Determine whether TYPE is a structure with a flexible array member, > >>>> or a union containing such a structure (possibly recursively). */ > >>>> flexible_array_type_p > >>>> > >>>> However, this routine is a little different than the one I tried to add: > >>>> > >>>> In the current routine ?flexible_array_type_p?, only one level nesting in the structure is accepted, multiple nesting in structure is not permitted. > >>>> > >>>> So, my question is: shall we accept multiple nesting in structure? i.e. > >>> > >>> If we don't reject the testcase with an error, then yes. > >> > >> Gcc currently accepts the multiple nesting in structure without error. > >> So, we will continue to accept such extension as long as the flex array > >> is at the end of the structure. At the same time, for the case the flex > >> array is in the middle of the structure, issue additional warnings now > >> to discourage such usage, and deprecate this case in a future release. > >> > >> Does this sound reasonable? > > > > Please don't mix several issues - I think the flex array in the > > middle of a structure is separate and we shouldn't report that > > as flexible_array_type_p or flexible_size_type_p since the size > > of the containing structure is not variable. > Agreed on this. > > My major question here is (for documentation change, sorry for mixing > this thread with the documentation change): do we need to document this > case together with the case in which struct with flex array is embedded > into another structure? (As a GCC extension?) I think this should be Josephs call - documenting this might encourage people to use such an extension, even if it's a bad one we want to get rid of. Maybe the easiest thing is to come up with a patch documenting it which we can then turn into a deprecation note depending on this outcome. Richard.