On Tue, 21 Jan 2014, Aldy Hernandez wrote: > Hi folks. > > The problem here is that while streaming the DECL_NAME with stream_read_tree() > and consequently lto_output_tree(), we trigger an ICE because we are recursing > on the DFS walk: > > else > { > /* This is the first time we see EXPR, write all reachable > trees to OB. */ > static bool in_dfs_walk; > > /* Protect against recursion which means disconnect between > what tree edges we walk in the DFS walk and what edges > we stream out. */ > gcc_assert (!in_dfs_walk); > > In the namelist fortran testcases, this is the first time we see the > DECL_NAMEs, so we ICE. I fixed this by outputting the DECL_NAME's string with > streamer_write_string() instead. > > [I honestly wondered why we don't stream the entire NAMELIST_DECL the same way > we stream IMPORTED_DECL, all in one go, instead of piecemeal like the present > code does. But LTO is this complicated black box in my head that I try not to > think about too much...the current patch touches as little as possible.] > > This change alone fixes the problems in the PR, but I also found another ICE > now that streaming actually works: dwarf. It turns out, that the way the > CONSTRUCTOR elements in the NAMELIST_DECL are streamed, a PARM_DECL that has > been previously seen (in the function's DECL_ARGUMENTS) will be streamed with > different reference magic (or whatever streamed references or ids are called > in LTO speak). So when we read the CONSTRUCTOR elements in the LTO read > phase, we end up with different pointers for a PARM_DECL in the constructor > for the seemingly same PARM_DECL in the function's arguments (DECL_ARGUMENTS). > > I don't understand LTO very well, but I do see that using stream_read_tree > (lto_output_tree) caches the entries, so it seems like a good fit to avoid > writing two distinct items for the same PARM_DECL. And since the constructor > elements have been previously seen, we won't hit the aforementioned DFS > recursion ICE. > > It'd be great if the LTO gods could illuminate this abyss (that's you Mr. > Biener ;-)), but the attached patch fixes all the LTO problems exhibited by: > > make check-fortran RUNTESTFLAGS="--target_board=unix'{-flto}' > dg.exp=*namelist*" > > As an aside, we really should test some subset of the LTO tests while testing > Fortran, or this oversight will surely repeat itself on any changes to the > Fortran streamer bits. > > Does this help? OK?
The patch doesn't make much sense to me. I think the problem is that NAMELIST_DECL is output in a ref section (LTO_namelist_decl_ref) but the output routine writes other refs (via stream_write_tree and outputting the constructor). lto_output_tree_ref may not do this kind of stuff. Instead the contents of a NAMELIST_DECL need to be output from the generic tree writing routines. Where are NAMELIST_DECLs possibly refered from? Richard.