On Fri, Aug 5, 2011 at 9:42 AM, Diego Novillo <dnovi...@google.com> wrote: > On Thu, Aug 4, 2011 at 17:47, <gch...@google.com> wrote: >> So if I understand correctly, the reference system doesn't work yet, you >> just put out the design in this patch and you'll fill up functionality >> in an upcoming patch so that's it's clearer what you're adding? > > Right, the 'if (0)' in pph_in_includes() disables support for it. > >> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph-streamer-in.c >> File gcc/cp/pph-streamer-in.c (right): >> >> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph-streamer-in.c#newcode1491 >> gcc/cp/pph-streamer-in.c:1491: pph_add_include (stream); >> does this mean the included pph has the include information for all of >> the headers in it's transitive closure? i.e. we don't need to load child >> information of a child? if so, isn't this bad from a dependency point of >> view? > > Hmm? Each header knows what other images it includes, so before > reading its own content it reads the contents from the others. The > dependencies don't change. The parent will not have the physical > representation of the ASTs from the children, just references to them. >
What I mean is if you have the following include structure A.c -> B.h -> C.h -> D.h Would B.h hold references to both C.h and D.h or only to C which in turn knows about D? From what I understood it seemed like all of the children in the transitive closure of B will be referenced in B and that B will not need to look at references in C when we read it (which is great from an I/O standpoint, but maybe not from a dependency point of view? just trying to understand the design) >> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph.c >> File gcc/cp/pph.c (right): >> >> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph.c#newcode164 >> gcc/cp/pph.c:164: if (pph_out_file != NULL) >> shouldn't you use your new pph_enabled_p function here? > > No. I need to know whether we are *creating* a PPH image. Perhaps I > should add a different predicate. > Ah ok I see, ya perhaps another inline function could clarify.. >> http://codereview.appspot.com/4847044/diff/1/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc >> File gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc (right): >> >> http://codereview.appspot.com/4847044/diff/1/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc#newcode1 >> gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc:1: /* { dg-options "-O >> -Wno-psabi -mtune=generic" } */ >> Is this related to the rest of the changes in this patch? I don't see it >> mentioned in your patch comments > > Gah. I was going to commit this separately but forgot. Sorry about that. > K, why does this fix the test? seems weird that we had an infinite loop before and changing the flags gets rid of it? Thanks, Gab
