From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20344 invoked by alias); 2 Jul 2011 01:37:23 -0000 Received: (qmail 20336 invoked by uid 22791); 2 Jul 2011 01:37:23 -0000 X-SWARE-Spam-Status: No, hits=-1.5 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; Sat, 02 Jul 2011 01:37:07 +0000 Received: from wpaz33.hot.corp.google.com (wpaz33.hot.corp.google.com [172.24.198.97]) by smtp-out.google.com with ESMTP id p621b6ZE018339 for ; Fri, 1 Jul 2011 18:37:06 -0700 Received: from vws9 (vws9.prod.google.com [10.241.21.137]) by wpaz33.hot.corp.google.com with ESMTP id p621b52T030305 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Fri, 1 Jul 2011 18:37:05 -0700 Received: by vws9 with SMTP id 9so132645vws.11 for ; Fri, 01 Jul 2011 18:37:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.185.1 with SMTP id t1mr232007yhm.187.1309570625060; Fri, 01 Jul 2011 18:37:05 -0700 (PDT) Reply-To: gchare@google.com, crowl@google.com, dnovillo@google.com, gcc-patches@gcc.gnu.org, reply@codereview.appspotmail.com X-Google-Appengine-App-Id: codereview Message-ID: <20cf3040ea5aea429d04a70c2b98@google.com> Date: Sat, 02 Jul 2011 01:37:00 -0000 Subject: Re: [pph] Stream first/weak_object_global_name (issue4641086) From: gchare@google.com To: crowl@google.com, dnovillo@google.com Cc: gcc-patches@gcc.gnu.org, reply@codereview.appspotmail.com Content-Type: text/plain; charset=ISO-8859-1; format=flowed; delsp=yes 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/msg00122.txt.bz2 So I did find a better way of doing this, see Issue #4627087. I'll go ahead and close this issue. On 2011/07/01 01:27:26, Gabriel Charette wrote: > 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 whatever 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 asm > diffs this is the first thing that I hit in the compiler logic that was > different in the good compiler vs the pph compiler. With this fix this bit of > 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.  These 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.  They 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.  One thing you could do is set a > > breakpoint in notice_global_symbol.  The two compilers should be > > calling it in the same order. > > > > > > Diego. > > http://codereview.appspot.com/4641086/