From patchwork Tue Jul 8 10:58:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 367862 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 B8ECA1400B5 for ; Tue, 8 Jul 2014 20:58:22 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=FYI9/va7ZCtTEZBHTT jTLuohLoOh7IGX0KPN+gCPYbyC1v7eu9Hf0ImvPKI/Ao6sX9xkl9W8ro8pKatkMn 05EAPeEOyfpoig2zbVn1B9HPbvn4yZ42+38DyLkPiCQV8V4X/tyLN3fr4ZWDa4Tv pND1l5oy7NRjKDWkzZwuTjzhM= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=LLbodZti0fzLwwiiTArbTXlZ lnE=; b=t0G/Z5PvpvT1qs8tOFiFCy+GhEkpu1LZFUO0AKJRPOJ185SdWSTAJfJR X1gCZE6s7GEIYC2FPc8FFhr8/+6KxlPScOd8YzkG/gdbIAIWdVyC4FFaWLrYwCZo dR+l8URgxSk+Fgl4Tz1zge2AmADw4ESlxcJNY1AqwObZ2IB+VoE= Received: (qmail 556 invoked by alias); 8 Jul 2014 10:58:14 -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 539 invoked by uid 89); 8 Jul 2014 10:58:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f175.google.com Received: from mail-we0-f175.google.com (HELO mail-we0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 08 Jul 2014 10:58:11 +0000 Received: by mail-we0-f175.google.com with SMTP id k48so5687574wev.6 for ; Tue, 08 Jul 2014 03:58:08 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.195.12.97 with SMTP id ep1mr22729733wjd.26.1404817088188; Tue, 08 Jul 2014 03:58:08 -0700 (PDT) Received: by 10.195.11.202 with HTTP; Tue, 8 Jul 2014 03:58:08 -0700 (PDT) In-Reply-To: <20140708002540.GE12716@kam.mff.cuni.cz> References: <20140628182230.GA2531@kam.mff.cuni.cz> <20140708002540.GE12716@kam.mff.cuni.cz> Date: Tue, 8 Jul 2014 12:58:08 +0200 Message-ID: Subject: Re: Fix representation of gcov_info_type From: Richard Biener To: Jan Hubicka Cc: GCC Patches X-IsSubscribed: yes On Tue, Jul 8, 2014 at 2:25 AM, Jan Hubicka wrote: >> > Index: stor-layout.c >> > =================================================================== >> > --- stor-layout.c (revision 212098) >> > +++ stor-layout.c (working copy) >> > @@ -2065,7 +2065,7 @@ void >> > finish_builtin_struct (tree type, const char *name, tree fields, >> > tree align_type) >> > { >> > - tree tail, next; >> > + tree tail, next, variant; >> > >> > for (tail = NULL_TREE; fields; tail = fields, fields = next) >> > { >> > @@ -2074,6 +2074,10 @@ finish_builtin_struct (tree type, const >> > DECL_CHAIN (fields) = tail; >> > } >> > TYPE_FIELDS (type) = tail; >> > + for (variant = TYPE_MAIN_VARIANT (type); >> > + variant != 0; >> > + variant = TYPE_NEXT_VARIANT (variant)) >> > + TYPE_FIELDS (variant) = tail; >> >> I think that's a bogus place to fix that. Instead the caller should >> use build_variant_type_copy. Especially that the fixup above >> depends on all variants being added before finish_builtin_struct >> is called. >> >> Please revert the above. > > Sorry, I missed this email at the airport. Will test revert overnight. > > I do not understand how you propose to fix it. > > The variant is produced before the builtin structure is finished and it thus > have FIELDS and SIZE NULL (as the pointer to struct itself is field of the > struct): > > /* function_info pointer pointer */ > fn_info_ptr_type = build_pointer_type > (build_qualified_type (fn_info_ptr_type, TYPE_QUAL_CONST)); > field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE, > fn_info_ptr_type); > DECL_CHAIN (field) = fields; > fields = field; > > finish_builtin_struct (type, "__gcov_info", fields, NULL_TREE); > > which leads to build_variant_type_copy. > > Once the struct is finished, we need to update the variants somewhere. Same > loop walking the variants appears in finalize_type_size that is transitively > called by finish_builtin_struct: > > /* Also layout any other variants of the type. */ > if (TYPE_NEXT_VARIANT (type) > || type != TYPE_MAIN_VARIANT (type)) > { > tree variant; > /* Record layout info of this variant. */ > tree size = TYPE_SIZE (type); > tree size_unit = TYPE_SIZE_UNIT (type); > unsigned int align = TYPE_ALIGN (type); > unsigned int user_align = TYPE_USER_ALIGN (type); > enum machine_mode mode = TYPE_MODE (type); > > /* Copy it into all variants. */ > for (variant = TYPE_MAIN_VARIANT (type); > variant != 0; > variant = TYPE_NEXT_VARIANT (variant)) > { > TYPE_SIZE (variant) = size; > TYPE_SIZE_UNIT (variant) = size_unit; > TYPE_ALIGN (variant) = align; > TYPE_USER_ALIGN (variant) = user_align; > SET_TYPE_MODE (variant, mode); > } > } > > Difference to my hunk is that the code works when called on non-main variant, > but I think it makes sense to always finish main variant of builtin structs. > (in fact I do not see why one would finalize size on non-main variants given > that the sizes must match either) So the issue seems to be: gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE); gcov_fn_info_type = lang_hooks.types.make_type (RECORD_TYPE); gcov_fn_info_ptr_type = build_pointer_type (build_qualified_type (gcov_fn_info_type, TYPE_QUAL_CONST)); build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type); build_info_type (gcov_info_type, gcov_fn_info_ptr_type); that __gcov_info has a member of type const __gcov_info * and that rather than using the equivalent of struct __gcov_info; typedef const __gcov_info *gcov_fn_info_ptr_type; struct __gcov_info { ... gcov_fn_info_ptr_type x; }; we build the variant of the yet incomplete struct and complete it later. Sth like should fix it by delaying the variant build for one case after the record has been completed and retaining the incomplete declaration for the pointer type. Richard. > What would be correct fix then? > Honza > > >> >> Thanks, >> Richard. >> >> > if (align_type) >> > { Index: coverage.c =================================================================== --- coverage.c (revision 210965) +++ coverage.c (working copy) @@ -1078,9 +1078,10 @@ /* Build the info and fn_info types. These are mutually recursive. */ gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE); gcov_fn_info_type = lang_hooks.types.make_type (RECORD_TYPE); + build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type); + gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE); gcov_fn_info_ptr_type = build_pointer_type (build_qualified_type (gcov_fn_info_type, TYPE_QUAL_CONST)); - build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type); build_info_type (gcov_info_type, gcov_fn_info_ptr_type); /* Build the gcov info var, this is referred to in its own