This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make cdtor names stable for LTO (PR lto/91307).


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 ;)

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));


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]