This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Make cdtor names stable for LTO (PR lto/91307).
On Fri, Aug 16, 2019 at 10:53 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Fri, Aug 16, 2019 at 10:47 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Thu, Aug 15, 2019 at 4:25 PM Jan Hubicka <hubicka@ucw.cz> wrote:
> > >
> > > > On Thu, Aug 1, 2019 at 3:10 PM Martin Liška <mliska@suse.cz> wrote:
> > > > >
> > > > > Hi.
> > > > >
> > > > > In LTO WPA mode we don't have to append temp file name
> > > > > to the global cdtor function names.
> > > >
> > > > Is that true? You can link with -r -flinker-output=rel and use
> > > > multiple WPA phases whose results you then finally link.
> > > >
> > > > So I don't think it's that easy. You might be able to look at
> > > > all_translation_units, pick the one with a sensible name
> > > > (hmm, not sure if we actually have a name there) and the lowest
> > > > UID and use that? Thus, make the set of involved source files
> > > > known and pick one. Ah,
> > > >
> > > > struct GTY(()) tree_translation_unit_decl {
> > > > struct tree_decl_common common;
> > > > /* Source language of this translation unit. Used for DWARF output. */
> > > > const char * GTY((skip(""))) language;
> > > > /* TODO: Non-optimization used to build this translation unit. */
> > > > /* TODO: Root of a partial DWARF tree for global types and decls. */
> > > > };
> > > >
> > > > so you might be able to get at a filename via the decls location,
> > > > I'm not sure. Do we have any LTO records per "input file" where we
> > > > can stream main_input_filename to?
> > >
> > > This is all bit sloppy. If you incrmentally link into .o file and then
> > > use LTO again to add more code, you will very likely run into conflict
> > > with .lto_priv clones as well. Especially now when we made them more
> > > stable.
> > >
> > > I wondered if Linker should not provide us also with list of symbols
> > > that are used in the unit, so we can safely produce more local ones?
> >
> > OTOH those are all local symbols so the only effect is that the
> > user may see duplicates when debugging.
> >
> > So I wonder why we even try to invent these fancy names for
> > targets where we have targetm.have_ctors_dtors and thus those
> > end up being local... My theory is that either gdb has some
> > fancy way to break on global initializers from file X (doesn't work
> > with LTO merging anyways) or it's just the users will be able to
> > pick up the correct one with tab-completion more easily!?
> >
> > So I believe choosing any name is fine for targetm_have_ctors_dtors
> > like with the following change, local to the IPA logic:
> >
> > Index: gcc/ipa.c
> > ===================================================================
> > --- gcc/ipa.c (revision 274536)
> > +++ gcc/ipa.c (working copy)
> > @@ -836,13 +836,15 @@ cgraph_build_static_cdtor_1 (char which,
> > /* The priority is encoded in the constructor or destructor name.
> > collect2 will sort the names and arrange that they are called at
> > program startup. */
> > - if (final)
> > - sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
> > + if (!targetm.have_ctors_dtors && final)
> > + {
> > + sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
> > + name = get_file_function_name (which_buf);
> > + }
> > else
> > - /* Proudce sane name but one not recognizable by collect2, just for the
> > - case we fail to inline the function. */
> > + /* Proudce sane name but one not recognizable by collect2, just for the
> > + case we fail to inline the function. */
> > sprintf (which_buf, "sub_%c_%.5d_%d", which, priority, counter++);
> > - name = get_file_function_name (which_buf);
> >
> > decl = build_decl (input_location, FUNCTION_DECL, name,
> > build_function_type_list (void_type_node, NULL_TREE));
> >
> > Is that change OK with you? If so I'll test & commit it.
>
> Better the following which actually works ;)
Bootstrapped/tested on x86_64-unknown-linux-gnu, OK?
Thanks,
Richard.
> Richard.
>
> 2019-08-16 Richard Biener <rguenther@suse.de>
>
> PR lto/91307
> * ipa.c (cgraph_build_static_cdtor_1): Use names not recognizable
> by collect2 when targetm.have_ctors_dtors which avoids dragging
> in temporary filenames from LTO input objects.
>
> Index: gcc/ipa.c
> ===================================================================
> --- gcc/ipa.c (revision 274536)
> +++ gcc/ipa.c (working copy)
> @@ -836,13 +836,18 @@ cgraph_build_static_cdtor_1 (char which,
> /* The priority is encoded in the constructor or destructor name.
> collect2 will sort the names and arrange that they are called at
> program startup. */
> - if (final)
> - sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
> + if (!targetm.have_ctors_dtors && final)
> + {
> + sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
> + name = get_file_function_name (which_buf);
> + }
> else
> - /* Proudce sane name but one not recognizable by collect2, just for the
> - case we fail to inline the function. */
> - sprintf (which_buf, "sub_%c_%.5d_%d", which, priority, counter++);
> - name = get_file_function_name (which_buf);
> + {
> + /* Proudce sane name but one not recognizable by collect2, just for the
> + case we fail to inline the function. */
> + sprintf (which_buf, "_sub_%c_%.5d_%d", which, priority, counter++);
> + name = get_identifier (which_buf);
> + }
>
> decl = build_decl (input_location, FUNCTION_DECL, name,
> build_function_type_list (void_type_node, NULL_TREE));