Patchwork [build,lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

login
register
mail settings
Submitter Richard Guenther
Date March 11, 2011, 12:30 p.m.
Message ID <alpine.LNX.2.00.1103111325570.25982@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/86415/
State New
Headers show

Comments

Richard Guenther - March 11, 2011, 12:30 p.m.
On Thu, 10 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> > Can we to fix 46944 change the dejagnu require-linker-plugin
> >> > to check if a linker plugin is used by default and not add
> >> > -fuse-linker-plugin?
> >> 
> >> That might be involved since it requires parsing gcc -Wl,-debug output.
> >> I suppose the solution outlined above is simpler and thus more robust.
> >
> > It might be as simple as
> >
> > int res;
> > int main() { int x; asm ("mov res, %0" : x(g)); return x; }
> >
> > which should fail to link with the plugin only (but yes, requies
> > target dependent assembly ...).
> 
> ... which I'd consider far too complicated/hard to maintain to consider.
> 
> > Or, use -save-temps and check for the existence of the resolution
> > file for int main() {}.  It should be named t.res for gcc -o t t.c -flto.
> 
> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
> even so the file is removed immediately.

The following seems to work for me

 
 # Return 1 if the target supports executing 750CL paired-single 
instructions, 0

it leaves an argument file in /tmp but otherwise nothing.

Eventually scanning -v output for '-plugin' is better.

> And even if we decide to fix PR lto/46944 like this, we're still left
> with the problem of users invoking gcc with -fuse-linker-plugin and
> getting either strange errors or no effect instead of a clear
> diagnostic.

Sure.  But just disabling linker-plugin support for almost everyone
doesn't sound like a good solution either.

I'm ok with fixing 46944 somehow at this point, as well as making
-fuse-linker-plugin the default only for known good linker versions.

Richard.
Rainer Orth - March 11, 2011, 2:37 p.m.
Richard Guenther <rguenther@suse.de> writes:

>> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
>> even so the file is removed immediately.
>
> The following seems to work for me
>
> Index: gcc/testsuite/lib/target-supports.exp
> ===================================================================
> --- gcc/testsuite/lib/target-supports.exp       (revision 170868)
> +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> @@ -1011,9 +1011,20 @@ proc check_effective_target_static_libgf
>  }
>  
>  proc check_linker_plugin_available { } {
> -  return [check_no_compiler_messages_nocache linker_plugin executable {
> +  set result [eval check_compile { linker_plugin object {
>       int main() { return 0; }
> -  } "-flto -fuse-linker-plugin"]
> +  } "-flto" } ]
> +  set lines [lindex $result 0]
> +  set output [lindex $result 1]
> +  set result [gcc_target_compile $output linker_plugin[pid] executable { 
> additional_flags=-flto additional_flags=-flto-partition=none 
> additional_flags=-save-temps } ]
> +  remote_file build delete $output
> +  remote_file build delete linker_plugin[pid]
> +  remote_file build delete linker_plugin[pid].s
> +  if [file exists linker_plugin[pid].res] {
> +    remote_file build delete linker_plugin[pid].res
> +    return 1
> +  }
> +  return 0
>  }

Still doesn't work for me if compiling and linking manually with GNU ld
2.21 on Solaris 11/x86: no .res file is being created, although
liblto_plugin.so is used.

> Eventually scanning -v output for '-plugin' is better.

This can only work if we make sure that -plugin is only passed to
linkers that properly handle it.

>> And even if we decide to fix PR lto/46944 like this, we're still left
>> with the problem of users invoking gcc with -fuse-linker-plugin and
>> getting either strange errors or no effect instead of a clear
>> diagnostic.
>
> Sure.  But just disabling linker-plugin support for almost everyone
> doesn't sound like a good solution either.

Why do you say so?  The tri-state solution I've outlined only disables it
completely for non-GNU linkers.  The plugin is used automatically for
gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.

I don't see the `almost everyone' here, on the contrary: the situation
is identical to what we have now, with the exception that we don't try
to pass -plugin to linkers we don't know they can somehow (even with
restrictions) handle it.  That's what PR lto/46944 is primarily about.

	Rainer
Richard Guenther - March 11, 2011, 3:10 p.m.
On Fri, 11 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
> >> even so the file is removed immediately.
> >
> > The following seems to work for me
> >
> > Index: gcc/testsuite/lib/target-supports.exp
> > ===================================================================
> > --- gcc/testsuite/lib/target-supports.exp       (revision 170868)
> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> > @@ -1011,9 +1011,20 @@ proc check_effective_target_static_libgf
> >  }
> >  
> >  proc check_linker_plugin_available { } {
> > -  return [check_no_compiler_messages_nocache linker_plugin executable {
> > +  set result [eval check_compile { linker_plugin object {
> >       int main() { return 0; }
> > -  } "-flto -fuse-linker-plugin"]
> > +  } "-flto" } ]
> > +  set lines [lindex $result 0]
> > +  set output [lindex $result 1]
> > +  set result [gcc_target_compile $output linker_plugin[pid] executable { 
> > additional_flags=-flto additional_flags=-flto-partition=none 
> > additional_flags=-save-temps } ]
> > +  remote_file build delete $output
> > +  remote_file build delete linker_plugin[pid]
> > +  remote_file build delete linker_plugin[pid].s
> > +  if [file exists linker_plugin[pid].res] {
> > +    remote_file build delete linker_plugin[pid].res
> > +    return 1
> > +  }
> > +  return 0
> >  }
> 
> Still doesn't work for me if compiling and linking manually with GNU ld
> 2.21 on Solaris 11/x86: no .res file is being created, although
> liblto_plugin.so is used.

"Work" as in, it detects if -fuse-linker-plugin is enabled by default.
Which it isn't for you?

> > Eventually scanning -v output for '-plugin' is better.
> 
> This can only work if we make sure that -plugin is only passed to
> linkers that properly handle it.

Sure, but your version check patch part would ensure that, if not
overridden by -fuse-linker-plugin.

> >> And even if we decide to fix PR lto/46944 like this, we're still left
> >> with the problem of users invoking gcc with -fuse-linker-plugin and
> >> getting either strange errors or no effect instead of a clear
> >> diagnostic.
> >
> > Sure.  But just disabling linker-plugin support for almost everyone
> > doesn't sound like a good solution either.
> 
> Why do you say so?  The tri-state solution I've outlined only disables it
> completely for non-GNU linkers.  The plugin is used automatically for
> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
> 
> I don't see the `almost everyone' here, on the contrary: the situation
> is identical to what we have now, with the exception that we don't try
> to pass -plugin to linkers we don't know they can somehow (even with
> restrictions) handle it.  That's what PR lto/46944 is primarily about.

Can you update your patch with the tri-state solution?

Thanks,
Richard.
Rainer Orth - March 11, 2011, 3:18 p.m.
Richard Guenther <rguenther@suse.de> writes:

>> Still doesn't work for me if compiling and linking manually with GNU ld
>> 2.21 on Solaris 11/x86: no .res file is being created, although
>> liblto_plugin.so is used.
>
> "Work" as in, it detects if -fuse-linker-plugin is enabled by default.
> Which it isn't for you?

I'm using gld 2.21, and -flto automatically uses the linker plugin, as
seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
of that file, but nothing more.

>> > Eventually scanning -v output for '-plugin' is better.
>> 
>> This can only work if we make sure that -plugin is only passed to
>> linkers that properly handle it.
>
> Sure, but your version check patch part would ensure that, if not
> overridden by -fuse-linker-plugin.

No, -fuse-linker-plugin wouldn't override here: that option is only
accepted if we know that the linker has *some* -plugin support.

>> Why do you say so?  The tri-state solution I've outlined only disables it
>> completely for non-GNU linkers.  The plugin is used automatically for
>> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
>> 
>> I don't see the `almost everyone' here, on the contrary: the situation
>> is identical to what we have now, with the exception that we don't try
>> to pass -plugin to linkers we don't know they can somehow (even with
>> restrictions) handle it.  That's what PR lto/46944 is primarily about.
>
> Can you update your patch with the tri-state solution?

Sure if the solution is deemed acceptable.  There isn't much point in
following that route if you see problems up front.

	Rainer
Richard Guenther - March 11, 2011, 3:31 p.m.
On Fri, 11 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> Still doesn't work for me if compiling and linking manually with GNU ld
> >> 2.21 on Solaris 11/x86: no .res file is being created, although
> >> liblto_plugin.so is used.
> >
> > "Work" as in, it detects if -fuse-linker-plugin is enabled by default.
> > Which it isn't for you?
> 
> I'm using gld 2.21, and -flto automatically uses the linker plugin, as
> seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
> to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
> of that file, but nothing more.

Interesting - it works for me with both GNU ld and gold from binutils 
2.21.

> >> > Eventually scanning -v output for '-plugin' is better.
> >> 
> >> This can only work if we make sure that -plugin is only passed to
> >> linkers that properly handle it.
> >
> > Sure, but your version check patch part would ensure that, if not
> > overridden by -fuse-linker-plugin.
> 
> No, -fuse-linker-plugin wouldn't override here: that option is only
> accepted if we know that the linker has *some* -plugin support.

Yeah, of course - that's desirable.

> >> Why do you say so?  The tri-state solution I've outlined only disables it
> >> completely for non-GNU linkers.  The plugin is used automatically for
> >> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
> >> 
> >> I don't see the `almost everyone' here, on the contrary: the situation
> >> is identical to what we have now, with the exception that we don't try
> >> to pass -plugin to linkers we don't know they can somehow (even with
> >> restrictions) handle it.  That's what PR lto/46944 is primarily about.
> >
> > Can you update your patch with the tri-state solution?
> 
> Sure if the solution is deemed acceptable.  There isn't much point in
> following that route if you see problems up front.

If that solution avoids 3) then yes, I'm fine with going that route.
Both 1) and 2) are very desirable anyway.

Thanks,
Richard.
Rainer Orth - March 11, 2011, 3:35 p.m.
Richard Guenther <rguenther@suse.de> writes:

>> I'm using gld 2.21, and -flto automatically uses the linker plugin, as
>> seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
>> to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
>> of that file, but nothing more.
>
> Interesting - it works for me with both GNU ld and gold from binutils 
> 2.21.

Strange indeed.  Maybe related to using xgcc -B./ from a build tree?
gold still doesn't fully work for me, perhaps it does for this example.
I'll give that a try too.

>> > Can you update your patch with the tri-state solution?
>> 
>> Sure if the solution is deemed acceptable.  There isn't much point in
>> following that route if you see problems up front.
>
> If that solution avoids 3) then yes, I'm fine with going that route.
> Both 1) and 2) are very desirable anyway.

Ok, I'll update the patch over the weekend.

Thanks.
	Rainer

Patch

Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp       (revision 170868)
+++ gcc/testsuite/lib/target-supports.exp       (working copy)
@@ -1011,9 +1011,20 @@  proc check_effective_target_static_libgf
 }
 
 proc check_linker_plugin_available { } {
-  return [check_no_compiler_messages_nocache linker_plugin executable {
+  set result [eval check_compile { linker_plugin object {
      int main() { return 0; }
-  } "-flto -fuse-linker-plugin"]
+  } "-flto" } ]
+  set lines [lindex $result 0]
+  set output [lindex $result 1]
+  set result [gcc_target_compile $output linker_plugin[pid] executable { 
additional_flags=-flto additional_flags=-flto-partition=none 
additional_flags=-save-temps } ]
+  remote_file build delete $output
+  remote_file build delete linker_plugin[pid]
+  remote_file build delete linker_plugin[pid].s
+  if [file exists linker_plugin[pid].res] {
+    remote_file build delete linker_plugin[pid].res
+    return 1
+  }
+  return 0
 }