Re: [pph] Stream first/weak_object_global_name (issue4641086)

[gchare@google.com]

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
<mailto:dnovillo@google.com> wrote:
> > On Thu, Jun 30, 2011 at 16:24, Gabriel Charette
<mailto:gchare@google.com> 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. &nbsp;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. &nbsp;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. &nbsp;One thing you could do is set a
> > breakpoint in notice_global_symbol. &nbsp;The two compilers should
be
> > calling it in the same order.
> >
> >
> > Diego.
> >



http://codereview.appspot.com/4641086/