From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2978 invoked by alias); 12 Jul 2011 20:43:51 -0000 Received: (qmail 2963 invoked by uid 22791); 12 Jul 2011 20:43:50 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Jul 2011 20:43:34 +0000 Received: from wpaz21.hot.corp.google.com (wpaz21.hot.corp.google.com [172.24.198.85]) by smtp-out.google.com with ESMTP id p6CKhXot002075 for ; Tue, 12 Jul 2011 13:43:33 -0700 Received: from yib18 (yib18.prod.google.com [10.243.65.82]) by wpaz21.hot.corp.google.com with ESMTP id p6CKhVH5018275 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Tue, 12 Jul 2011 13:43:32 -0700 Received: by yib18 with SMTP id 18so2251711yib.6 for ; Tue, 12 Jul 2011 13:43:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.101.45.2 with SMTP id x2mr396876anj.86.1310503411755; Tue, 12 Jul 2011 13:43:31 -0700 (PDT) Received: by 10.100.254.9 with HTTP; Tue, 12 Jul 2011 13:43:31 -0700 (PDT) In-Reply-To: References: <20110709012047.0D90B1C36BA@gchare.mtv.corp.google.com> Date: Tue, 12 Jul 2011 20:56:00 -0000 Message-ID: Subject: Re: [pph] Stream DECL_CHAIN only for VAR/FUNCTION_DECLs that are part of a RECORD_OR_UNION_TYPE (issue4672055) From: Gabriel Charette To: Diego Novillo Cc: reply@codereview.appspotmail.com, crowl@google.com, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes 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 X-SW-Source: 2011-07/txt/msg00962.txt.bz2 The reason I put that patch out is that sometimes, when we stream an actual chain, lto_input_chain is going to rebuild the new chain it's meant to be, but then pph_read_tree (which is called after by the name_hook to finish reading special parts of the tree) overwrites the DECL_CHAIN that was introduced by lto_input_chain. The only time we need to actually stream in/out the DECL_CHAIN is when streaming unions/structs because from what I looked at in lto it looks like we are not doing lto_output_chain, but lto_output_tree on the first member of the fields' chain (not sure how that even works in lto... but in pph we used to only get the first member of structs streamed and streaming DECL_CHAIN was the fix for it...) I introduced this fix because it broke my patch trying to stream out the chains backwards (as it would overwrite the chain I was trying to create backwards on input, I think this didn't show up before because the chain being built on input was the same as the one existing on output (thus overwriting with the same value...) ) Even if this doesn't break tests anymore, we probably still want this, no point adding stuff to the pph image that is not needed... Any idea why lto doesn't call lto_output_chain, but simply lto_output_tree to output the chains for struct/union? Gab On Tue, Jul 12, 2011 at 12:21 PM, Diego Novillo wrote: > On Fri, Jul 8, 2011 at 21:20, Gabriel Charette wrote: > >> 2011-07-08 =A0Gabriel Charette =A0 >> >> =A0 =A0 =A0 =A0* pph-streamer-in.c (pph_in_function_decl): Stream in >> =A0 =A0 =A0 =A0DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD= _OR_UNION_TYPE >> =A0 =A0 =A0 =A0(pph_read_tree): Stream in DECL_CHAIN of VAR_DECL only if= it's part >> =A0 =A0 =A0 =A0of a RECORD_OR_UNION_TYPE. >> =A0 =A0 =A0 =A0* pph-streamer-out.c (pph_out_function_decl): Stream out >> =A0 =A0 =A0 =A0DECL_CHAIN of FUNCTION_DECL only if it's part of a RECORD= _OR_UNION_TYPE >> =A0 =A0 =A0 =A0(pph_write_tree): Stream out DECL_CHAIN of VAR_DECL only = if it's part >> =A0 =A0 =A0 =A0of a RECORD_OR_UNION_TYPE. > > Gab, do you still need this patch? =A0In principle, it doesn't make a > lot of sense to restrict when we save the DECL_CHAIN in this way. > It's not obvious what this would fix or help with. > > > Diego. >