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: [LTO merge][6/15][RFA] Driver


On Mon, 28 Sep 2009, Diego Novillo wrote:

> Joseph, if you need help tracking down the rationale for a
> certain change, let me know.  Some of this is before I got more
> involved with the branch, though.

Note that I can't review the collect2 changes.

> +    %{use-linker-plugin: \
> +    -plugin %(linker_plugin_file) \
> +    -plugin-opt=%(lto_wrapper) \
> +    -plugin-opt=%(lto_gcc) \
> +    %{static|static-libgcc:-plugin-opt=-libgcc=%(lto_libgcc)}	\

Why does the plugin need a special option for libgcc - but apparently not 
for any of the other libraries distributed with GCC?

> +
> +      if (switch_matches (use_linker_plugin,
> +			  use_linker_plugin + strlen (use_linker_plugin), 0))
> +	{

Should there be a specific error at this point if LTO is not enabled?

> +	  linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +						 "liblto_plugin.so", X_OK,
> +						 false);
> +	  if (!linker_plugin_file_spec)
> +	    fatal ("-use-linker-plugin, but liblto_plugin.so not found.");

Errors should not end with ".".

> +	  lto_libgcc_spec = find_a_file (&startfile_prefixes, "libgcc.a",
> +					 R_OK, true);
> +	  if (!lto_libgcc_spec)
> +	    fatal ("could not find libgcc.a.");

Likewise.

> +  /* Emit LTO marker if LTO info has been previously emitted.  This is
> +     used by collect2 to determine whether an object file contains IL.
> +     We used to emit an undefined reference here, but this produces
> +     link errors if an object file with IL is stored into a shared
> +     library without invoking lto1.  */
> +  if (flag_generate_lto)
> +    fprintf (asm_out_file,"\t.comm\tgnu_lto_v1,1,1\n");

I have two concerns about this:

* Various back ends have their own hooks involved in emitting common 
variables.  I don't know if the above syntax is or is not valid for all 
ELF targets, but it would seem better to create a DECL and let it be 
output in the usual way rather than hardcoding assembler synta here.

* The name gnu_lto_v1 is in the user's namespace and so users can 
legitimately create their own variables with that name, which should not 
trigger any special LTO handling.  Either a name in the implementation 
namespace (e.g. __gnu_lto_v1) should be used, or, better, a name that 
cannot be generated from C source at all (e.g. one with a "." such as 
__gnu_lto.v1).  Remember that arbitrary NUL-terminated strings are valid 
as symbol names in ELF.

-- 
Joseph S. Myers
joseph@codesourcery.com


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