From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15049 invoked by alias); 1 Jul 2011 01:27:42 -0000 Received: (qmail 15041 invoked by uid 22791); 1 Jul 2011 01:27:41 -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; Fri, 01 Jul 2011 01:27:27 +0000 Received: from hpaq5.eem.corp.google.com (hpaq5.eem.corp.google.com [172.25.149.5]) by smtp-out.google.com with ESMTP id p611RP9w008067 for ; Thu, 30 Jun 2011 18:27:26 -0700 Received: from yia13 (yia13.prod.google.com [10.243.65.13]) by hpaq5.eem.corp.google.com with ESMTP id p611R9pW022663 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 30 Jun 2011 18:27:24 -0700 Received: by yia13 with SMTP id 13so1415439yia.0 for ; Thu, 30 Jun 2011 18:27:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.55.32 with SMTP id d32mr2417643ana.42.1309483644162; Thu, 30 Jun 2011 18:27:24 -0700 (PDT) Received: by 10.100.92.7 with HTTP; Thu, 30 Jun 2011 18:27:24 -0700 (PDT) In-Reply-To: References: <20110630202408.D5A291C1942@gchare.mtv.corp.google.com> Date: Fri, 01 Jul 2011 01:27:00 -0000 Message-ID: Subject: Re: [pph] Stream first/weak_object_global_name (issue4641086) 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/msg00003.txt.bz2 notice_global_symbol is actually called in the parser (before we stream out= ). I just confirmed this by setting a break point on the function in the pph generation of c1varoder.pph and hitting the function twice for the two variables defined in c1varorder.h It looks like rtl and assembler_name are set BEFORE we stream out... so that we would need to stream them (looks like assembler_name is already streamed, but not rtl...). Not streaming rtl causes it to be recreated later (which i think might be the cause of the bad ordering). Now another problem is that when two pph are generated, they are not aware of each other (and of their respective first/weak_global_object_name...) so even if we stream those variables, their could still be conflicts (and/or asm diffs..) However, I'm not sure about how make_decl_rtl works, but if it can regenerate the same rtl we had before (and even potentially solve the conflicts mentioned in the previous paragraph), we may not need to actually stream the rtl, we would just need to fix the ordering as the pph rtl's are being regenerated. (they are regenerated because DECL_RTL(NODE) either returns (NODE)->decl_with_rtl.rtl or calls make_decl_rtl(NODE) if it's NULL...) If we do this, I think we wanna have the first/weak_object_global_name set to what it should be by streaming it in from the first pph, to ensure all the rtl and assembler_name choices are made correctly (I'm not sure I understand all of this, but it seems to rely on those fields). Gab On Thu, Jun 30, 2011 at 4:33 PM, Diego Novillo wrote: > On Thu, Jun 30, 2011 at 16:24, Gabriel Charette wrote: >> first/weak_global_object_name are part of the global state. >> >> This seems to be used to produce the assembler names. They are set only = once as early as possible in the parsing; thus we should define it to be wh= atever it was in the first pph (or even in the C file if it was set before = any pph was even loaded. >> >> I'm not sure exactly what this does, but trying to find a fix for the as= m diffs this is the first thing that I hit in the compiler logic that was d= ifferent in the good compiler vs the pph compiler. With this fix this bit o= f logic is now correct. >> However, this doesn't fix any pph test (nor does it even change any pph = asm (I diff'ed the old bad assemblies (*.s+pph) with the new ones)). > > This does not look right to me. =A0These two symbols are set when > calling notice_global_symbol, which is done during code generation > (you will see it called from cgraph_finalize_function, > cgraph_mark_reachable_node, etc). > > You are probably close to the cause of this relative ordering problem, > but streaming these two globals is not the solution. =A0They both should > be set in the same order by both compilers, but not by forcing them > this way. > > One thing that may be happening here is that the order in which we > call cgraph_finalize_function is different in the two compilers. > That's what needs to change. =A0One thing you could do is set a > breakpoint in notice_global_symbol. =A0The two compilers should be > calling it in the same order. > > > Diego. >