Re: PR59723: fix LTO + fortran namelist ICEs

[Richard Biener <rguenther at suse dot de>]

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.