Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 28 Jan 2014, Jakub Jelinek wrote: On Tue, Jan 28, 2014 at 10:40:51AM +0100, Richard Biener wrote: 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? I think usually from BLOCK_VARS of some BLOCK. The following seems to fix things for me - bootstrap / regtest and SPEC 2k6 LTO -g build in progress. Now somebody could verify if LTO produces sensible debuginfo for NAMELIST_DECLs. Richard. 2014-02-04 Richard Biener rguent...@suse.de PR lto/59723 * lto-streamer-out.c (lto_output_tree_ref): Do not write trees from lto_output_tree_ref. Index: gcc/lto-streamer-out.c === *** gcc/lto-streamer-out.c (revision 207455) --- gcc/lto-streamer-out.c (working copy) *** lto_output_tree_ref (struct output_block *** 255,273 break; case NAMELIST_DECL: ! { ! unsigned i; ! tree value, tmp; ! ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! stream_write_tree (ob, DECL_NAME (expr), true); ! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr); ! gcc_assert (tmp != NULL_TREE); ! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)-length()); ! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value) ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, value); ! break; ! } case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref); --- 261,269 break; case NAMELIST_DECL: ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, expr); ! break; case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref);
Re: PR59723: fix LTO + fortran namelist ICEs
Richard, With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html I get multiple ICEs when testing with check-gfortran RUNTESTFLAGS=--target_board=unix'{-m32/-flto,-m64/-flto}' They are of the kind lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972 Note that the Aldy's patch at http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html fixed all the ICE in the same tests. Dominique
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Dominique Dhumieres wrote: Richard, With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html I get multiple ICEs when testing with check-gfortran RUNTESTFLAGS=--target_board=unix'{-m32/-flto,-m64/-flto}' They are of the kind lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972 Note that the Aldy's patch at http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html fixed all the ICE in the same tests. Yep, I'm collecting more fixes - see below for what I have. Aldy's fix is simply wrong (as is the namelist-decl-ref handling in the LTO code). With the patch below we can still hit an ICE in lto_fixup_prevailing_decls as we seem to arrive with a CONSTRUCTOR here. I'm waiting for Micha to show up to tell me if that's simply because CONSTRUCTOR isn't handled in the code (yet) or if there is a deeper reason. Richard. 2014-02-04 Richard Biener rguent...@suse.de PR lto/59723 * lto-streamer-out.c (lto_output_tree_ref): Do not write trees from lto_output_tree_ref. * lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref similar to LTO_imported_decl_ref. lto/ * lto.c (mentions_vars_p) Handle NAMELIST_DECL. Index: gcc/lto-streamer-out.c === *** gcc/lto-streamer-out.c (revision 207455) --- gcc/lto-streamer-out.c (working copy) *** lto_output_tree_ref (struct output_block *** 255,273 break; case NAMELIST_DECL: ! { ! unsigned i; ! tree value, tmp; ! ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! stream_write_tree (ob, DECL_NAME (expr), true); ! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr); ! gcc_assert (tmp != NULL_TREE); ! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)-length()); ! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value) ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, value); ! break; ! } case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref); --- 261,269 break; case NAMELIST_DECL: ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, expr); ! break; case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref); Index: gcc/lto/lto.c === *** gcc/lto/lto.c (revision 207455) --- gcc/lto/lto.c (working copy) *** mentions_vars_p (tree t) *** 926,931 --- 926,932 case RESULT_DECL: case IMPORTED_DECL: case NAMESPACE_DECL: + case NAMELIST_DECL: return mentions_vars_p_decl_common (t); case VAR_DECL: Index: gcc/lto-streamer-in.c === *** gcc/lto-streamer-in.c (revision 207455) --- gcc/lto-streamer-in.c (working copy) *** lto_input_tree_ref (struct lto_input_blo *** 244,275 case LTO_imported_decl_ref: case LTO_label_decl_ref: case LTO_translation_unit_decl_ref: ix_u = streamer_read_uhwi (ib); result = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); break; - case LTO_namelist_decl_ref: - { - tree tmp; - vecconstructor_elt, va_gc *nml_decls = NULL; - unsigned i, n; - - result = make_node (NAMELIST_DECL); - TREE_TYPE (result) = void_type_node; - DECL_NAME (result) = stream_read_tree (ib, data_in); - n = streamer_read_uhwi (ib); - for (i = 0; i n; i++) - { - ix_u = streamer_read_uhwi (ib); - tmp = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); - gcc_assert (tmp != NULL_TREE); - CONSTRUCTOR_APPEND_ELT (nml_decls, NULL_TREE, tmp); - } - NAMELIST_DECL_ASSOCIATED_DECL (result) = build_constructor (NULL_TREE, - nml_decls); - break; - } - default: gcc_unreachable (); } --- 244,254 case LTO_imported_decl_ref: case LTO_label_decl_ref: case LTO_translation_unit_decl_ref: + case LTO_namelist_decl_ref: ix_u = streamer_read_uhwi (ib); result = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); break; default: gcc_unreachable (); }
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Richard Biener wrote: On Tue, 4 Feb 2014, Dominique Dhumieres wrote: Richard, With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html I get multiple ICEs when testing with check-gfortran RUNTESTFLAGS=--target_board=unix'{-m32/-flto,-m64/-flto}' They are of the kind lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972 Note that the Aldy's patch at http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html fixed all the ICE in the same tests. Yep, I'm collecting more fixes - see below for what I have. Aldy's fix is simply wrong (as is the namelist-decl-ref handling in the LTO code). With the patch below we can still hit an ICE in lto_fixup_prevailing_decls as we seem to arrive with a CONSTRUCTOR here. I'm waiting for Micha to show up to tell me if that's simply because CONSTRUCTOR isn't handled in the code (yet) or if there is a deeper reason. That would be additionally Index: gcc/lto/lto.c === *** gcc/lto/lto.c (revision 207455) --- gcc/lto/lto.c (working copy) *** lto_fixup_prevailing_decls (tree t) *** 2597,2603 enum tree_code code = TREE_CODE (t); bool fixed = false; ! gcc_checking_assert (code != CONSTRUCTOR code != TREE_BINFO); LTO_NO_PREVAIL (TREE_TYPE (t)); if (CODE_CONTAINS_STRUCT (code, TS_COMMON)) LTO_NO_PREVAIL (TREE_CHAIN (t)); --- 2598,2604 enum tree_code code = TREE_CODE (t); bool fixed = false; ! gcc_checking_assert (code != TREE_BINFO); LTO_NO_PREVAIL (TREE_TYPE (t)); if (CODE_CONTAINS_STRUCT (code, TS_COMMON)) LTO_NO_PREVAIL (TREE_CHAIN (t)); *** lto_fixup_prevailing_decls (tree t) *** 2659,2664 --- 2660,2672 for (i = TREE_OPERAND_LENGTH (t) - 1; i = 0; --i) LTO_SET_PREVAIL (TREE_OPERAND (t, i)); } + else if (TREE_CODE (t) == CONSTRUCTOR) + { + int i; + tree val; + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (t), i, val) + LTO_SET_PREVAIL (val); + } else { switch (code) Richard. 2014-02-04 Richard Biener rguent...@suse.de PR lto/59723 * lto-streamer-out.c (lto_output_tree_ref): Do not write trees from lto_output_tree_ref. * lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref similar to LTO_imported_decl_ref. lto/ * lto.c (mentions_vars_p) Handle NAMELIST_DECL. Index: gcc/lto-streamer-out.c === *** gcc/lto-streamer-out.c(revision 207455) --- gcc/lto-streamer-out.c(working copy) *** lto_output_tree_ref (struct output_block *** 255,273 break; case NAMELIST_DECL: ! { ! unsigned i; ! tree value, tmp; ! ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! stream_write_tree (ob, DECL_NAME (expr), true); ! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr); ! gcc_assert (tmp != NULL_TREE); ! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)-length()); ! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value) ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, value); ! break; ! } case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref); --- 261,269 break; case NAMELIST_DECL: ! streamer_write_record_start (ob, LTO_namelist_decl_ref); ! lto_output_var_decl_index (ob-decl_state, ob-main_stream, expr); ! break; case NAMESPACE_DECL: streamer_write_record_start (ob, LTO_namespace_decl_ref); Index: gcc/lto/lto.c === *** gcc/lto/lto.c (revision 207455) --- gcc/lto/lto.c (working copy) *** mentions_vars_p (tree t) *** 926,931 --- 926,932 case RESULT_DECL: case IMPORTED_DECL: case NAMESPACE_DECL: + case NAMELIST_DECL: return mentions_vars_p_decl_common (t); case VAR_DECL: Index: gcc/lto-streamer-in.c === *** gcc/lto-streamer-in.c (revision 207455) --- gcc/lto-streamer-in.c (working copy) *** lto_input_tree_ref (struct lto_input_blo *** 244,275 case LTO_imported_decl_ref: case LTO_label_decl_ref: case LTO_translation_unit_decl_ref: ix_u = streamer_read_uhwi (ib); result = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); break; - case LTO_namelist_decl_ref: - { - tree tmp; - vecconstructor_elt, va_gc *nml_decls = NULL; - unsigned i, n; - - result = make_node (NAMELIST_DECL); - TREE_TYPE (result) = void_type_node; - DECL_NAME (result) =
Re: PR59723: fix LTO + fortran namelist ICEs
With the petches (an a minor fix in gcc/lto/lto.c) the remaining failures are FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 scan-assembler (DW_AT_name: label|label[^\n]*[^\n]*DW_AT_name) FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 -g3 scan-assembler (DW_AT_name: label|label[^\n]*[^\n]*DW_AT_name) FAIL: gfortran.dg/bind_c_array_params_2.f90 -O scan-assembler-times myBindC 1 FAIL: gfortran.dg/bind_c_vars.f90 -O0 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O1 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O2 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-loops (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -Os (test for excess errors) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_51.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/pr48636-2.f90 -O scan-ipa-dump cp Creating a specialized node of bar/[0-9]*\\. FAIL: gfortran.dg/pr52835.f90 -O scan-tree-dump optimized bar FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump cp Creating a specialized node of init FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump-times cp Aggregate replacements 2 The ICEs are /opt/gcc/work/gcc/testsuite/gfortran.dg/namelist_14.f90:96:0: internal compiler error: in force_decl_die, at dwarf2out.c:20211 The excess error for gfortran.dg/namelist_51.f90 is warning: invalid DWARF generated by the compiler: DIE 0x01f0 has multiple AT_calling_convention attributes in '/var/folders/8q/sh_swgz96r7f5vnn08f7fxr0gn/T//ccub2YMQ.ltrans0.ltrans.o'. Note that if the test is compiled with -g only, I get an ICE lto1: internal compiler error: in add_AT_specification, at dwarf2out.c:4096 I have looked to the scan errors for bind_c_array_params_2.f90: myBindC appears twice call_myBindC .ascii _main\0\0\0\0\0\0\0\0\0\0\0\0\274\0\0\0_myBindC\0\0\2\0\0\0\0\0\0\0\0\0\356\0\0\0__gfortran_set_options ... and for pr48636-2.f90 where Creating a specialized node of bar is replaced with Creating a specialized node of __foo_MOD_bar. Such failures are quite easy to fix, but I wonder if they worth the effort. Dominique
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Dominique Dhumieres wrote: With the petches (an a minor fix in gcc/lto/lto.c) the remaining failures are FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 scan-assembler (DW_AT_name: label|label[^\n]*[^\n]*DW_AT_name) FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 -g3 scan-assembler (DW_AT_name: label|label[^\n]*[^\n]*DW_AT_name) FAIL: gfortran.dg/bind_c_array_params_2.f90 -O scan-assembler-times myBindC 1 FAIL: gfortran.dg/bind_c_vars.f90 -O0 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O1 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O2 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-loops (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -Os (test for excess errors) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_51.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/pr48636-2.f90 -O scan-ipa-dump cp Creating a specialized node of bar/[0-9]*\\. FAIL: gfortran.dg/pr52835.f90 -O scan-tree-dump optimized bar FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump cp Creating a specialized node of init FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump-times cp Aggregate replacements 2 The ICEs are /opt/gcc/work/gcc/testsuite/gfortran.dg/namelist_14.f90:96:0: internal compiler error: in force_decl_die, at dwarf2out.c:20211 The excess error for gfortran.dg/namelist_51.f90 is warning: invalid DWARF generated by the compiler: DIE 0x01f0 has multiple AT_calling_convention attributes in '/var/folders/8q/sh_swgz96r7f5vnn08f7fxr0gn/T//ccub2YMQ.ltrans0.ltrans.o'. Note that if the test is compiled with -g only, I get an ICE lto1: internal compiler error: in add_AT_specification, at dwarf2out.c:4096 I have looked to the scan errors for bind_c_array_params_2.f90: myBindC appears twice call_myBindC .ascii _main\0\0\0\0\0\0\0\0\0\0\0\0\274\0\0\0_myBindC\0\0\2\0\0\0\0\0\0\0\0\0\356\0\0\0__gfortran_set_options ... and for pr48636-2.f90 where Creating a specialized node of bar is replaced with Creating a specialized node of __foo_MOD_bar. Such failures are quite easy to fix, but I wonder if they worth the effort. The dwarf2out.c ICEs are fixed with the tree_is_indexable hunk. Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC 2k6 -flto -g built and gfortran.dg tested with -flto, applied to trunk. Richard. 2014-02-04 Richard Biener rguent...@suse.de PR lto/59723 * lto-streamer-out.c (tree_is_indexable): Force NAMELIST_DECLs in function context local. (lto_output_tree_ref): Do not write trees from lto_output_tree_ref. * lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref similar to LTO_imported_decl_ref. lto/ * lto.c (mentions_vars_p): Handle NAMELIST_DECL. (lto_fixup_prevailing_decls): Handle fixing up CONSTRUCTOR values. Index: gcc/lto-streamer-in.c === *** gcc/lto-streamer-in.c (revision 207455) --- gcc/lto-streamer-in.c (working copy) *** lto_input_tree_ref (struct lto_input_blo *** 244,275 case LTO_imported_decl_ref: case LTO_label_decl_ref: case LTO_translation_unit_decl_ref: ix_u = streamer_read_uhwi (ib); result = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); break; - case LTO_namelist_decl_ref: - { - tree tmp; - vecconstructor_elt, va_gc *nml_decls = NULL; - unsigned i, n; - - result = make_node (NAMELIST_DECL); - TREE_TYPE (result) = void_type_node; - DECL_NAME (result) = stream_read_tree (ib, data_in); - n = streamer_read_uhwi (ib); - for (i = 0; i n; i++) - { - ix_u = streamer_read_uhwi (ib); - tmp = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); - gcc_assert (tmp != NULL_TREE); - CONSTRUCTOR_APPEND_ELT (nml_decls, NULL_TREE, tmp); - } - NAMELIST_DECL_ASSOCIATED_DECL (result) = build_constructor (NULL_TREE, - nml_decls); - break; - } - default:
Re: PR59723: fix LTO + fortran namelist ICEs
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.
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, Jan 28, 2014 at 10:40:51AM +0100, Richard Biener wrote: 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? I think usually from BLOCK_VARS of some BLOCK. Jakub
PR59723: fix LTO + fortran namelist ICEs
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? Aldy commit a916bfe9fec9b62971cea769ae58d4b3467e08bd Author: Aldy Hernandez al...@redhat.com Date: Fri Jan 17 08:01:44 2014 -0800 * lto-streamer-in.c (lto_input_tree_ref): Use streamer_read_string instead of stream_read_tree. Read in constructos with stream_read_tree. * lto-streamer-out.c (lto_output_tree_ref): Do not recurse on DFS walk. Output constructors correctly. diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c index df32a6b..9eec472 100644 --- a/gcc/lto-streamer-in.c +++ b/gcc/lto-streamer-in.c @@ -256,12 +256,12 @@ lto_input_tree_ref (struct lto_input_block *ib, struct data_in *data_in, result = make_node (NAMELIST_DECL); TREE_TYPE (result) = void_type_node; - DECL_NAME (result) = stream_read_tree (ib, data_in); + DECL_NAME (result) = get_identifier (streamer_read_string (data_in, + ib)); n = streamer_read_uhwi (ib); for (i = 0; i n; i++) { - ix_u = streamer_read_uhwi (ib); - tmp = lto_file_decl_data_get_var_decl (data_in-file_data, ix_u); + tmp = stream_read_tree (ib, data_in); gcc_assert (tmp != NULL_TREE); CONSTRUCTOR_APPEND_ELT (nml_decls, NULL_TREE, tmp); } diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c index 5d6aed5..dc48dd4 100644 --- a/gcc/lto-streamer-out.c +++ b/gcc/lto-streamer-out.c @@ -260,12 +260,13 @@ lto_output_tree_ref (struct output_block *ob, tree expr) tree value, tmp; streamer_write_record_start (ob, LTO_namelist_decl_ref); - stream_write_tree (ob, DECL_NAME (expr), true); + streamer_write_string (ob, ob-main_stream, + IDENTIFIER_POINTER (DECL_NAME (expr)), true); tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr); gcc_assert (tmp != NULL_TREE); streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)-length()); FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value) - lto_output_var_decl_index (ob-decl_state, ob-main_stream, value); + stream_write_tree (ob, value, true); break; }
Re: PR59723: fix LTO + fortran namelist ICEs
Aldy Hernandez al...@redhat.com 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? I'll return from vacation next week and will have a closer look then. Richard. Aldy