| Message ID | 20141207235458.GA43729@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 8 Dec 2014, Ilya Verbin wrote: > Hi, > > On 28 Nov 09:36, Richard Biener wrote: > > On Fri, 28 Nov 2014, Ilya Verbin wrote: > > > I found a bug here, have_{lto,offload} must be set if at least one file contains > > > lto/offload sections, but currently they are overwritten by the last file. > > > Fix is bootstrapped and regtested on x86_64-linux. OK for trunk? > > Ok. > > Unfortunately, this fix was not general enough. > There might be cases when mixed object files get into lto-wrapper, ie some of > them contain only LTO sections, some contain only offload sections, and some > contain both. But when lto-wrapper will pass all these files to recompilation, > the compiler might crash (it depends on the order of input files), since in > read_cgraph_and_symbols it expects that *all* input files contain IR section of > given type. > This patch splits input objects from argv into lto_argv and offload_argv, so > that all files in arrays contain corresponding IR. > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload > IR without LTO, to claimed_files, since this may corrupt a resolution file. > > Tested on various combinations of files with/without -flto and with/without > offload, using trunk ld and gold, also tested on ld without plugin support. > Bootstrap and make check passed on x86_64-linux and i686-linux. Ok for trunk? Did you check that bootstrap-lto still works? Ok if so. Thanks, Richard. > But I am unable to run lto-bootstrap on trunk, it says: > /x/build-x86_64-linux/./prev-gcc/gcc-ar -B/x/build-x86_64-linux/./prev-gcc/ cru libdecnumber.a decNumber.o decContext.o decimal32.o decimal64.o decimal128.o bid2dpd_dpd2bid.o host-ieee32.o host-ieee64.o host-ieee128.o > /usr/bin/ar terminated with signal 11 [Segmentation fault], core dumped > make[4]: *** [libdecnumber.a] Error 1 > > > gcc/ > * lto-wrapper.c (compile_offload_image): Start processing in_argv > from 0 instead of 1. > (run_gcc): Put offload objects into offload_argv, put LTO objects and > possible preceding arguments into lto_argv. > Pass offload_argv to compile_images_for_offload_targets instead of argv. > Use lto_argv for LTO recompilation instead of argv. > lto-plugin/ > * lto-plugin.c (offload_files, num_offload_files): New static variables. > (free_1): Use arguments instead of global variables. > (free_2): Free offload_files. > (all_symbols_read_handler): Add names from offload_files to lto-wrapper > arguments. > (claim_file_handler): Do not add file to claimed_files if it contains > offload sections without LTO sections. Add it to offload_files instead. > > > diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c > index 0f69457..f75c0dc 100644 > --- a/gcc/lto-wrapper.c > +++ b/gcc/lto-wrapper.c > @@ -669,7 +669,7 @@ compile_offload_image (const char *target, const char *compiler_path, > obstack_ptr_grow (&argv_obstack, filename); > > /* Append names of input object files. */ > - for (unsigned i = 1; i < in_argc; i++) > + for (unsigned i = 0; i < in_argc; i++) > obstack_ptr_grow (&argv_obstack, in_argv[i]); > > /* Append options from offload_lto sections. */ > @@ -883,6 +883,8 @@ run_gcc (unsigned argc, char *argv[]) > int new_head_argc; > bool have_lto = false; > bool have_offload = false; > + unsigned lto_argc = 0, offload_argc = 0; > + char **lto_argv, **offload_argv; > > /* Get the driver and options. */ > collect_gcc = getenv ("COLLECT_GCC"); > @@ -896,6 +898,11 @@ run_gcc (unsigned argc, char *argv[]) > &decoded_options, > &decoded_options_count); > > + /* Allocate arrays for input object files with LTO or offload IL, > + and for possible preceding arguments. */ > + lto_argv = XNEWVEC (char *, argc); > + offload_argv = XNEWVEC (char *, argc); > + > /* Look at saved options in the IL files. */ > for (i = 1; i < argc; ++i) > { > @@ -918,17 +925,27 @@ run_gcc (unsigned argc, char *argv[]) > } > fd = open (argv[i], O_RDONLY); > if (fd == -1) > - continue; > + { > + lto_argv[lto_argc++] = argv[i]; > + continue; > + } > + > + if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX, > + &fdecoded_options, &fdecoded_options_count, > + collect_gcc)) > + { > + have_lto = true; > + lto_argv[lto_argc++] = argv[i]; > + } > + > + if (find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX, > + &offload_fdecoded_options, > + &offload_fdecoded_options_count, collect_gcc)) > + { > + have_offload = true; > + offload_argv[offload_argc++] = argv[i]; > + } > > - have_lto > - |= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX, > - &fdecoded_options, &fdecoded_options_count, > - collect_gcc); > - have_offload > - |= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX, > - &offload_fdecoded_options, > - &offload_fdecoded_options_count, > - collect_gcc); > close (fd); > } > > @@ -1027,7 +1044,8 @@ run_gcc (unsigned argc, char *argv[]) > > if (have_offload) > { > - compile_images_for_offload_targets (argc, argv, offload_fdecoded_options, > + compile_images_for_offload_targets (offload_argc, offload_argv, > + offload_fdecoded_options, > offload_fdecoded_options_count, > decoded_options, > decoded_options_count); > @@ -1119,8 +1137,8 @@ run_gcc (unsigned argc, char *argv[]) > } > > /* Append the input objects and possible preceding arguments. */ > - for (i = 1; i < argc; ++i) > - obstack_ptr_grow (&argv_obstack, argv[i]); > + for (i = 0; i < lto_argc; ++i) > + obstack_ptr_grow (&argv_obstack, lto_argv[i]); > obstack_ptr_grow (&argv_obstack, NULL); > > new_argv = XOBFINISH (&argv_obstack, const char **); > @@ -1295,6 +1313,8 @@ cont: > if (offloadend) > printf ("%s\n", offloadend); > > + XDELETE (lto_argv); > + XDELETE (offload_argv); > obstack_free (&argv_obstack, NULL); > } > > diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c > index fb6555d..8d957402 100644 > --- a/lto-plugin/lto-plugin.c > +++ b/lto-plugin/lto-plugin.c > @@ -152,6 +152,9 @@ static ld_plugin_add_symbols add_symbols; > static struct plugin_file_info *claimed_files = NULL; > static unsigned int num_claimed_files = 0; > > +static struct plugin_file_info *offload_files = NULL; > +static unsigned int num_offload_files = 0; > + > static char **output_files = NULL; > static unsigned int num_output_files = 0; > > @@ -313,12 +316,12 @@ translate (char *data, char *end, struct plugin_symtab *out) > resolution. */ > > static void > -free_1 (void) > +free_1 (struct plugin_file_info *files, unsigned num_files) > { > unsigned int i; > - for (i = 0; i < num_claimed_files; i++) > + for (i = 0; i < num_files; i++) > { > - struct plugin_file_info *info = &claimed_files[i]; > + struct plugin_file_info *info = &files[i]; > struct plugin_symtab *symtab = &info->symtab; > unsigned int j; > for (j = 0; j < symtab->nsyms; j++) > @@ -346,6 +349,14 @@ free_2 (void) > free (info->name); > } > > + for (i = 0; i < num_offload_files; i++) > + { > + struct plugin_file_info *info = &offload_files[i]; > + struct plugin_symtab *symtab = &info->symtab; > + free (symtab->aux); > + free (info->name); > + } > + > for (i = 0; i < num_output_files; i++) > free (output_files[i]); > free (output_files); > @@ -354,6 +365,10 @@ free_2 (void) > claimed_files = NULL; > num_claimed_files = 0; > > + free (offload_files); > + offload_files = NULL; > + num_offload_files = 0; > + > free (arguments_file_name); > arguments_file_name = NULL; > } > @@ -608,10 +623,11 @@ static enum ld_plugin_status > all_symbols_read_handler (void) > { > unsigned i; > - unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 1; > + unsigned num_lto_args > + = num_claimed_files + num_offload_files + lto_wrapper_num_args + 1; > char **lto_argv; > const char **lto_arg_ptr; > - if (num_claimed_files == 0) > + if (num_claimed_files + num_offload_files == 0) > return LDPS_OK; > > if (nop) > @@ -626,7 +642,8 @@ all_symbols_read_handler (void) > > write_resolution (); > > - free_1 (); > + free_1 (claimed_files, num_claimed_files); > + free_1 (offload_files, num_offload_files); > > for (i = 0; i < lto_wrapper_num_args; i++) > *lto_arg_ptr++ = lto_wrapper_argv[i]; > @@ -638,6 +655,13 @@ all_symbols_read_handler (void) > *lto_arg_ptr++ = info->name; > } > > + for (i = 0; i < num_offload_files; i++) > + { > + struct plugin_file_info *info = &offload_files[i]; > + > + *lto_arg_ptr++ = info->name; > + } > + > *lto_arg_ptr++ = NULL; > exec_lto_wrapper (lto_argv); > > @@ -949,16 +973,29 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) > if (obj.found > 1) > resolve_conflicts (<o_file.symtab, <o_file.conflicts); > > - status = add_symbols (file->handle, lto_file.symtab.nsyms, > - lto_file.symtab.syms); > - check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); > + if (obj.found > 0) > + { > + status = add_symbols (file->handle, lto_file.symtab.nsyms, > + lto_file.symtab.syms); > + check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); > + > + num_claimed_files++; > + claimed_files = > + xrealloc (claimed_files, > + num_claimed_files * sizeof (struct plugin_file_info)); > + claimed_files[num_claimed_files - 1] = lto_file; > + } > + > + if (obj.found == 0 && obj.offload == 1) > + { > + num_offload_files++; > + offload_files = > + xrealloc (offload_files, > + num_offload_files * sizeof (struct plugin_file_info)); > + offload_files[num_offload_files - 1] = lto_file; > + } > > *claimed = 1; > - num_claimed_files++; > - claimed_files = > - xrealloc (claimed_files, > - num_claimed_files * sizeof (struct plugin_file_info)); > - claimed_files[num_claimed_files - 1] = lto_file; > > goto cleanup; > > > -- Ilya > >
On 09 Dec 14:59, Richard Biener wrote: > On Mon, 8 Dec 2014, Ilya Verbin wrote: > > Unfortunately, this fix was not general enough. > > There might be cases when mixed object files get into lto-wrapper, ie some of > > them contain only LTO sections, some contain only offload sections, and some > > contain both. But when lto-wrapper will pass all these files to recompilation, > > the compiler might crash (it depends on the order of input files), since in > > read_cgraph_and_symbols it expects that *all* input files contain IR section of > > given type. > > This patch splits input objects from argv into lto_argv and offload_argv, so > > that all files in arrays contain corresponding IR. > > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload > > IR without LTO, to claimed_files, since this may corrupt a resolution file. > > > > Tested on various combinations of files with/without -flto and with/without > > offload, using trunk ld and gold, also tested on ld without plugin support. > > Bootstrap and make check passed on x86_64-linux and i686-linux. Ok for trunk? > > Did you check that bootstrap-lto still works? Ok if so. Yes, bootstrap-lto passed. Committed revision 218543. Thanks, -- Ilya
On Wed, Dec 10, 2014 at 01:48:21 +0300, Ilya Verbin wrote: > On 09 Dec 14:59, Richard Biener wrote: > > On Mon, 8 Dec 2014, Ilya Verbin wrote: > > > Unfortunately, this fix was not general enough. > > > There might be cases when mixed object files get into lto-wrapper, ie some of > > > them contain only LTO sections, some contain only offload sections, and some > > > contain both. But when lto-wrapper will pass all these files to recompilation, > > > the compiler might crash (it depends on the order of input files), since in > > > read_cgraph_and_symbols it expects that *all* input files contain IR section of > > > given type. > > > This patch splits input objects from argv into lto_argv and offload_argv, so > > > that all files in arrays contain corresponding IR. > > > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload > > > IR without LTO, to claimed_files, since this may corrupt a resolution file. > > > > > > Tested on various combinations of files with/without -flto and with/without > > > offload, using trunk ld and gold, also tested on ld without plugin support. > > > Bootstrap and make check passed on x86_64-linux and i686-linux. Ok for trunk? > > > > Did you check that bootstrap-lto still works? Ok if so. > > Yes, bootstrap-lto passed. > Committed revision 218543. I don't know how I missed this a year ago, but mixing of LTO objects with offloading-without-LTO objects still doesn't work :( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68463 filed about that. Any thoughts how to fix this? Thanks, -- Ilya
On Fri, 20 Nov 2015, Ilya Verbin wrote: > On Wed, Dec 10, 2014 at 01:48:21 +0300, Ilya Verbin wrote: > > On 09 Dec 14:59, Richard Biener wrote: > > > On Mon, 8 Dec 2014, Ilya Verbin wrote: > > > > Unfortunately, this fix was not general enough. > > > > There might be cases when mixed object files get into lto-wrapper, ie some of > > > > them contain only LTO sections, some contain only offload sections, and some > > > > contain both. But when lto-wrapper will pass all these files to recompilation, > > > > the compiler might crash (it depends on the order of input files), since in > > > > read_cgraph_and_symbols it expects that *all* input files contain IR section of > > > > given type. > > > > This patch splits input objects from argv into lto_argv and offload_argv, so > > > > that all files in arrays contain corresponding IR. > > > > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload > > > > IR without LTO, to claimed_files, since this may corrupt a resolution file. > > > > > > > > Tested on various combinations of files with/without -flto and with/without > > > > offload, using trunk ld and gold, also tested on ld without plugin support. > > > > Bootstrap and make check passed on x86_64-linux and i686-linux. Ok for trunk? > > > > > > Did you check that bootstrap-lto still works? Ok if so. > > > > Yes, bootstrap-lto passed. > > Committed revision 218543. > > I don't know how I missed this a year ago, but mixing of LTO objects with > offloading-without-LTO objects still doesn't work :( > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68463 filed about that. > Any thoughts how to fix this? Don't claim files you don't handle. Richard.
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c index 0f69457..f75c0dc 100644 --- a/gcc/lto-wrapper.c +++ b/gcc/lto-wrapper.c @@ -669,7 +669,7 @@ compile_offload_image (const char *target, const char *compiler_path, obstack_ptr_grow (&argv_obstack, filename); /* Append names of input object files. */ - for (unsigned i = 1; i < in_argc; i++) + for (unsigned i = 0; i < in_argc; i++) obstack_ptr_grow (&argv_obstack, in_argv[i]); /* Append options from offload_lto sections. */ @@ -883,6 +883,8 @@ run_gcc (unsigned argc, char *argv[]) int new_head_argc; bool have_lto = false; bool have_offload = false; + unsigned lto_argc = 0, offload_argc = 0; + char **lto_argv, **offload_argv; /* Get the driver and options. */ collect_gcc = getenv ("COLLECT_GCC"); @@ -896,6 +898,11 @@ run_gcc (unsigned argc, char *argv[]) &decoded_options, &decoded_options_count); + /* Allocate arrays for input object files with LTO or offload IL, + and for possible preceding arguments. */ + lto_argv = XNEWVEC (char *, argc); + offload_argv = XNEWVEC (char *, argc); + /* Look at saved options in the IL files. */ for (i = 1; i < argc; ++i) { @@ -918,17 +925,27 @@ run_gcc (unsigned argc, char *argv[]) } fd = open (argv[i], O_RDONLY); if (fd == -1) - continue; + { + lto_argv[lto_argc++] = argv[i]; + continue; + } + + if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX, + &fdecoded_options, &fdecoded_options_count, + collect_gcc)) + { + have_lto = true; + lto_argv[lto_argc++] = argv[i]; + } + + if (find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX, + &offload_fdecoded_options, + &offload_fdecoded_options_count, collect_gcc)) + { + have_offload = true; + offload_argv[offload_argc++] = argv[i]; + } - have_lto - |= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX, - &fdecoded_options, &fdecoded_options_count, - collect_gcc); - have_offload - |= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX, - &offload_fdecoded_options, - &offload_fdecoded_options_count, - collect_gcc); close (fd); } @@ -1027,7 +1044,8 @@ run_gcc (unsigned argc, char *argv[]) if (have_offload) { - compile_images_for_offload_targets (argc, argv, offload_fdecoded_options, + compile_images_for_offload_targets (offload_argc, offload_argv, + offload_fdecoded_options, offload_fdecoded_options_count, decoded_options, decoded_options_count); @@ -1119,8 +1137,8 @@ run_gcc (unsigned argc, char *argv[]) } /* Append the input objects and possible preceding arguments. */ - for (i = 1; i < argc; ++i) - obstack_ptr_grow (&argv_obstack, argv[i]); + for (i = 0; i < lto_argc; ++i) + obstack_ptr_grow (&argv_obstack, lto_argv[i]); obstack_ptr_grow (&argv_obstack, NULL); new_argv = XOBFINISH (&argv_obstack, const char **); @@ -1295,6 +1313,8 @@ cont: if (offloadend) printf ("%s\n", offloadend); + XDELETE (lto_argv); + XDELETE (offload_argv); obstack_free (&argv_obstack, NULL); } diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c index fb6555d..8d957402 100644 --- a/lto-plugin/lto-plugin.c +++ b/lto-plugin/lto-plugin.c @@ -152,6 +152,9 @@ static ld_plugin_add_symbols add_symbols; static struct plugin_file_info *claimed_files = NULL; static unsigned int num_claimed_files = 0; +static struct plugin_file_info *offload_files = NULL; +static unsigned int num_offload_files = 0; + static char **output_files = NULL; static unsigned int num_output_files = 0; @@ -313,12 +316,12 @@ translate (char *data, char *end, struct plugin_symtab *out) resolution. */ static void -free_1 (void) +free_1 (struct plugin_file_info *files, unsigned num_files) { unsigned int i; - for (i = 0; i < num_claimed_files; i++) + for (i = 0; i < num_files; i++) { - struct plugin_file_info *info = &claimed_files[i]; + struct plugin_file_info *info = &files[i]; struct plugin_symtab *symtab = &info->symtab; unsigned int j; for (j = 0; j < symtab->nsyms; j++) @@ -346,6 +349,14 @@ free_2 (void) free (info->name); } + for (i = 0; i < num_offload_files; i++) + { + struct plugin_file_info *info = &offload_files[i]; + struct plugin_symtab *symtab = &info->symtab; + free (symtab->aux); + free (info->name); + } + for (i = 0; i < num_output_files; i++) free (output_files[i]); free (output_files); @@ -354,6 +365,10 @@ free_2 (void) claimed_files = NULL; num_claimed_files = 0; + free (offload_files); + offload_files = NULL; + num_offload_files = 0; + free (arguments_file_name); arguments_file_name = NULL; } @@ -608,10 +623,11 @@ static enum ld_plugin_status all_symbols_read_handler (void) { unsigned i; - unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 1; + unsigned num_lto_args + = num_claimed_files + num_offload_files + lto_wrapper_num_args + 1; char **lto_argv; const char **lto_arg_ptr; - if (num_claimed_files == 0) + if (num_claimed_files + num_offload_files == 0) return LDPS_OK; if (nop) @@ -626,7 +642,8 @@ all_symbols_read_handler (void) write_resolution (); - free_1 (); + free_1 (claimed_files, num_claimed_files); + free_1 (offload_files, num_offload_files); for (i = 0; i < lto_wrapper_num_args; i++) *lto_arg_ptr++ = lto_wrapper_argv[i]; @@ -638,6 +655,13 @@ all_symbols_read_handler (void) *lto_arg_ptr++ = info->name; } + for (i = 0; i < num_offload_files; i++) + { + struct plugin_file_info *info = &offload_files[i]; + + *lto_arg_ptr++ = info->name; + } + *lto_arg_ptr++ = NULL; exec_lto_wrapper (lto_argv); @@ -949,16 +973,29 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) if (obj.found > 1) resolve_conflicts (<o_file.symtab, <o_file.conflicts); - status = add_symbols (file->handle, lto_file.symtab.nsyms, - lto_file.symtab.syms); - check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); + if (obj.found > 0) + { + status = add_symbols (file->handle, lto_file.symtab.nsyms, + lto_file.symtab.syms); + check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); + + num_claimed_files++; + claimed_files = + xrealloc (claimed_files, + num_claimed_files * sizeof (struct plugin_file_info)); + claimed_files[num_claimed_files - 1] = lto_file; + } + + if (obj.found == 0 && obj.offload == 1) + { + num_offload_files++; + offload_files = + xrealloc (offload_files, + num_offload_files * sizeof (struct plugin_file_info)); + offload_files[num_offload_files - 1] = lto_file; + } *claimed = 1; - num_claimed_files++; - claimed_files = - xrealloc (claimed_files, - num_claimed_files * sizeof (struct plugin_file_info)); - claimed_files[num_claimed_files - 1] = lto_file; goto cleanup;