Re: [PING][PATCH] Mark symbols in offload tables with force_output in read_offload_tables

[Tom de Vries <Tom_deVries at mentor dot com>]

On 26/01/16 14:01, Ilya Verbin wrote:
> On Tue, Jan 26, 2016 at 13:21:57 +0100, Tom de Vries wrote:
> > On 25/01/16 14:27, Ilya Verbin wrote:
> > > On Tue, Jan 05, 2016 at 15:56:15 +0100, Tom de Vries wrote:
> > > > > diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> > > > > index 62e5454..cdaee41 100644
> > > > > --- a/gcc/lto-cgraph.c
> > > > > +++ b/gcc/lto-cgraph.c
> > > > > @@ -1911,6 +1911,11 @@ input_offload_tables (void)
> > > > >   	      tree fn_decl
> > > > >   		= lto_file_decl_data_get_fn_decl (file_data, decl_index);
> > > > >   	      vec_safe_push (offload_funcs, fn_decl);
> > > > > +
> > > > > +	      /* Prevent IPA from removing fn_decl as unreachable, since there
> > > > > +		 may be no refs from the parent function to child_fn in offload
> > > > > +		 LTO mode.  */
> > > > > +	      cgraph_node::get (fn_decl)->mark_force_output ();
> > > > >   	    }
> > > > >   	  else if (tag == LTO_symtab_variable)
> > > > >   	    {
> > > > > @@ -1918,6 +1923,10 @@ input_offload_tables (void)
> > > > >   	      tree var_decl
> > > > >   		= lto_file_decl_data_get_var_decl (file_data, decl_index);
> > > > >   	      vec_safe_push (offload_vars, var_decl);
> > > > > +
> > > > > +	      /* Prevent IPA from removing var_decl as unused, since there
> > > > > +		 may be no refs to var_decl in offload LTO mode.  */
> > > > > +	      varpool_node::get (var_decl)->force_output = 1;
> > > > >   	    }
> > >
> > > This doesn't work when there is more than one LTO partition, because only first
> > > partition contains full offload table to maintain correct order, but cgraph and
> > > varpool nodes aren't necessarily created for the first partition.  To reproduce:
> > >
> > > $ make check-target-libgomp RUNTESTFLAGS="c.exp=for-* --target_board=unix/-flto"
> > > FAIL: libgomp.c/for-3.c (internal compiler error)
> > > FAIL: libgomp.c/for-5.c (internal compiler error)
> > > FAIL: libgomp.c/for-6.c (internal compiler error)
> > > $ make check-target-libgomp RUNTESTFLAGS="c++.exp=for-* --target_board=unix/-flto"
> > > FAIL: libgomp.c++/for-11.C (internal compiler error)
> > > FAIL: libgomp.c++/for-13.C (internal compiler error)
> > > FAIL: libgomp.c++/for-14.C (internal compiler error)
> >
> > This works for me.
> >
> > OK for trunk?
> >
> > Thanks,
> > - Tom
>
> > Check that cgraph/varpool_node exists before use in input_offload_tables
> >
> > 2016-01-26  Tom de Vries  <tom@codesourcery.com>
> >
> > 	* lto-cgraph.c (input_offload_tables): Check that cgraph/varpool_node
> > 	exists before use.
>
> In this case they will be not marked as force_output in other partitions (except
> the first one).

AFAIU, that's not the case.

If we're splitting up lto compilation over partitions, it means we're 
first calling lto1 in WPA mode. We'll read in all offload tables, and 
mark all symbols with force_output, and when writing out the partitions, 
we'll write the offload symbols out with force_output set.

This updated patch only does the force_output marking for offload 
symbols in WPA or LTO. It's not necessary in LTRANS mode.

Bootstrapped and reg-tested on x86_64.

Build for nvidia accelerator and reg-tested libgomp with various lto 
settings.

OK for trunk, stage4?

Thanks,
- Tom

Don't mark offload symbols with force_output in ltrans

2016-02-08  Tom de Vries  <tom@codesourcery.com>

	PR lto/69655
	* lto-cgraph.c (input_offload_tables): Add and handle bool parameter
	do_force_output.
	* lto-streamer.h (input_offload_tables): Add and handle bool parameter.

	* lto.c (read_cgraph_and_symbols): Call input_offload_tables with
	argument.

---
 gcc/lto-cgraph.c   | 8 +++++---
 gcc/lto-streamer.h | 2 +-
 gcc/lto/lto.c      | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 0634779..95c446d 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -1885,7 +1885,7 @@ input_symtab (void)
    target code, and store them into OFFLOAD_FUNCS and OFFLOAD_VARS.  */
 
 void
-input_offload_tables (void)
+input_offload_tables (bool do_force_output)
 {
   struct lto_file_decl_data **file_data_vec = lto_get_file_decl_data ();
   struct lto_file_decl_data *file_data;
@@ -1915,7 +1915,8 @@ input_offload_tables (void)
 	      /* Prevent IPA from removing fn_decl as unreachable, since there
 		 may be no refs from the parent function to child_fn in offload
 		 LTO mode.  */
-	      cgraph_node::get (fn_decl)->mark_force_output ();
+	      if (do_force_output)
+		cgraph_node::get (fn_decl)->mark_force_output ();
 	    }
 	  else if (tag == LTO_symtab_variable)
 	    {
@@ -1926,7 +1927,8 @@ input_offload_tables (void)
 
 	      /* Prevent IPA from removing var_decl as unused, since there
 		 may be no refs to var_decl in offload LTO mode.  */
-	      varpool_node::get (var_decl)->force_output = 1;
+	      if (do_force_output)
+		varpool_node::get (var_decl)->force_output = 1;
 	    }
 	  else
 	    fatal_error (input_location,
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 0cb200e..f391161 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -915,7 +915,7 @@ bool lto_symtab_encoder_encode_initializer_p (lto_symtab_encoder_t,
 void output_symtab (void);
 void input_symtab (void);
 void output_offload_tables (void);
-void input_offload_tables (void);
+void input_offload_tables (bool);
 bool referenced_from_other_partition_p (struct ipa_ref_list *,
 				        lto_symtab_encoder_t);
 bool reachable_from_other_partition_p (struct cgraph_node *,
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index ecec1bc..2736c5c 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -2855,7 +2855,7 @@ read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
   /* Read the symtab.  */
   input_symtab ();
 
-  input_offload_tables ();
+  input_offload_tables (!flag_ltrans);
 
   /* Store resolutions into the symbol table.  */