Re: PR59723: fix LTO + fortran namelist ICEs

2014-02-04 Thread Richard Biener
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

2014-02-04 Thread Dominique Dhumieres
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

2014-02-04 Thread Richard Biener
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

2014-02-04 Thread Richard Biener
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

2014-02-04 Thread Dominique Dhumieres
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

2014-02-04 Thread Richard Biener
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

2014-01-28 Thread Richard Biener
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

2014-01-28 Thread Jakub Jelinek
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

2014-01-21 Thread Aldy Hernandez

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

2014-01-21 Thread Richard Biener
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