Message ID | 56B8960B.2090203@mentor.com |
---|---|
State | New |
Headers | show |
On Mon, Feb 08, 2016 at 14:20:11 +0100, Tom de Vries wrote: > 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. You're right, works for me. -- Ilya
[ was: [PING][PATCH] Mark symbols in offload tables with force_output in read_offload_tables ] On 08/02/16 14:20, Tom de Vries wrote: > 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? > Ping. Original submission here: https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00549.html . Thanks, - Tom > 0006-Don-t-mark-offload-symbols-with-force_output-in-ltrans.patch > > > 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. */ > >
On Mon, 15 Feb 2016, Tom de Vries wrote: > [ was: [PING][PATCH] Mark symbols in offload tables with force_output in > read_offload_tables ] > > On 08/02/16 14:20, Tom de Vries wrote: > > 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? > > > > Ping. Original submission here: > https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00549.html . Ok. Richhard. > Thanks, > - Tom > > > 0006-Don-t-mark-offload-symbols-with-force_output-in-ltrans.patch > > > > > > 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. */ > > > > > >
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. */