This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING][PATCH] Mark symbols in offload tables with force_output in read_offload_tables
- From: Ilya Verbin <iverbin at gmail dot com>
- To: Tom de Vries <Tom_deVries at mentor dot com>
- Cc: Richard Biener <rguenther at suse dot de>, Thomas Schwinge <thomas at codesourcery dot com>, Jakub Jelinek <jakub at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Mon, 8 Feb 2016 18:01:38 +0300
- Subject: Re: [PING][PATCH] Mark symbols in offload tables with force_output in read_offload_tables
- Authentication-results: sourceware.org; auth=none
- References: <565F8C69 dot 1070906 at mentor dot com> <alpine dot LSU dot 2 dot 11 dot 1512030958510 dot 4884 at t29 dot fhfr dot qr> <566022D0 dot 2030906 at mentor dot com> <56697AC5 dot 7060902 at mentor dot com> <56718B13 dot 4040601 at mentor dot com> <568BD98F dot 7020908 at mentor dot com> <20160125132703 dot GA43532 at msticlxl57 dot ims dot intel dot com> <56A764E5 dot 2050405 at mentor dot com> <20160126130154 dot GA51036 at msticlxl57 dot ims dot intel dot com> <56B8960B dot 2090203 at mentor dot com>
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