PATCH: 2 stage BFD linker for LTO plugin

classic Classic list List threaded Threaded
36 messages Options
12
Reply | Threaded
Open this post in threaded view
|

PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:

> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>> On 04/12/2010 01:24, H.J. Lu wrote:
>>
>>> I checked in a patch to implement stage 2 linking. Everything
>>> seems to work, including "gcc -static ... -lm".
>>
>>  Any chance you could send a complete diff?
>>
>
> I will submit a complete diff after I fix a few corner cases.
> In the meantime, you can clone my git tree and do a "git diff".
>
Hi,

This patch implements 2 stage BFD linker for LTO plugin.
It works with current LTO API on all cases I tested.

Known issue:  --whole-archive will call plugin on archives with IR
in stage 2 linking. But ld never calls plugin to get back object files.
I will try to avoid it in a follow up patch.

OK for trunk?

Thanks.

--
H.J.
---
bfd/

2010-12-03  H.J. Lu  <[hidden email]>

        PR driver/42690
        * bfd.c (BFD_PLUGIN): New.
        (BFD_FLAGS_SAVED): Add BFD_PLUGIN.
        (BFD_FLAGS_FOR_BFD_USE_MASK): Likewise.

        * bfd-in2.h: Regenerated.

ld/

2010-12-03  H.J. Lu  <[hidden email]>

        PR driver/42690
        * ldfile.c (ldfile_try_open_bfd): Turn on BFD_PLUGIN and set
        claimed to false on non-object files and unclaimed object files.
        Set stage1.

        * ldlang.c (cmdline_list): New.
        (cmdline_next_claimed_output): Likewise.
        (cmdline_list_init): Likewise.
        (cmdline_get_stage2_input_files): Likewise.
        (debug_cmdline_list): Likewise.
        (cmdline_list_append): Likewise.
        (cmdline_set_next_claimed_output): Likewise.
        (cmdline_list_insert_claimed_output): Likewise.
        (new_afile): Set stage1 to FALSE;
        (lang_init): Call cmdline_list_init.
        (lang_process): Call plugin_active_plugins_p to check plugin
        support.  Check cmdline_next_claimed_output before opening
        stage 2 input.  Call debug_cmdline_list if trace_file_tries
        is set.  Call cmdline_get_stage2_input_files to get stage 2
        input files.

        * ldlang.h (lang_input_statement_struct): Add stage1.
        (cmdline_enum_type): New.
        (cmdline_header_type): Likewise.
        (cmdline_input_statement_type): Likewise.
        (cmdline_claimed_output_type): Likewise.
        (cmdline_union_type): Likewise.
        (cmdline_list_type): Likewise.
        (cmdline_list_append): Likewise.
        (cmdline_list_insert_claimed_output): Likewise.
        (cmdline_set_next_claimed_output): Likewise.

        * lexsup.c (parse_args): Call cmdline_list_append if needed.

        * plugin.c (plugin_opt_plugin_arg): Ignore -pass-through=.
        (add_input_file): Replace lang_add_input_file with
        cmdline_list_insert_claimed_output.
        (add_input_library): Likewise.

ld/testsuite/

2010-12-03  H.J. Lu  <[hidden email]>

        PR driver/42690
        * ld-plugin/func1i.c: New.
        * ld-plugin/func2.c: Likewise.
        * ld-plugin/func2h.c: Likewise.
        * ld-plugin/func3p.c: Likewise.

        * ld-plugin/plugin.exp: Add object files for symbols claimed
        or created by testplugin.
        * ld-plugin/plugin-7.d: Updated.
        * ld-plugin/plugin-8.d: Likewise.
        * ld-plugin/plugin-9.d: Likewise.

binutils-2stage-1.patch (34K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Fri, Dec 3, 2010 at 10:07 PM, H.J. Lu <[hidden email]> wrote:

> On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:
>> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>>> On 04/12/2010 01:24, H.J. Lu wrote:
>>>
>>>> I checked in a patch to implement stage 2 linking. Everything
>>>> seems to work, including "gcc -static ... -lm".
>>>
>>>  Any chance you could send a complete diff?
>>>
>>
>> I will submit a complete diff after I fix a few corner cases.
>> In the meantime, you can clone my git tree and do a "git diff".
>>
>
> Hi,
>
> This patch implements 2 stage BFD linker for LTO plugin.
> It works with current LTO API on all cases I tested.
>
> Known issue:  --whole-archive will call plugin on archives with IR
> in stage 2 linking. But ld never calls plugin to get back object files.
> I will try to avoid it in a follow up patch.
>

This turns out not a problem. In stage 2 linking, for --whole-archive
we call plugin to get symbols in the IR element of an archive and
it will be ignored for stage 2 linking.  It is OK since we already get
the trans object files back for stage 2 linking.

BTW, the new linker passed bootstrap-lto with all default languages.
I am planning to include this patch in the next Linux binutils.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Sat, Dec 4, 2010 at 9:34 AM, H.J. Lu <[hidden email]> wrote:

> On Fri, Dec 3, 2010 at 10:07 PM, H.J. Lu <[hidden email]> wrote:
>> On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:
>>> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>>>> On 04/12/2010 01:24, H.J. Lu wrote:
>>>>
>>>>> I checked in a patch to implement stage 2 linking. Everything
>>>>> seems to work, including "gcc -static ... -lm".
>>>>
>>>>  Any chance you could send a complete diff?
>>>>
>>>
>>> I will submit a complete diff after I fix a few corner cases.
>>> In the meantime, you can clone my git tree and do a "git diff".
>>>
>>
>> Hi,
>>
>> This patch implements 2 stage BFD linker for LTO plugin.
>> It works with current LTO API on all cases I tested.
>>
>> Known issue:  --whole-archive will call plugin on archives with IR
>> in stage 2 linking. But ld never calls plugin to get back object files.
>> I will try to avoid it in a follow up patch.
>>
>
> This turns out not a problem. In stage 2 linking, for --whole-archive
> we call plugin to get symbols in the IR element of an archive and
> it will be ignored for stage 2 linking.  It is OK since we already get
> the trans object files back for stage 2 linking.
>
> BTW, the new linker passed bootstrap-lto with all default languages.
> I am planning to include this patch in the next Linux binutils.
>
I missed the IR object in an archive:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34

This updated patch fixed it.  OK for trunk?

Thanks.

--
H.J.
---

binutils-2stage-2.patch (38K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Sat, Dec 4, 2010 at 4:43 PM, H.J. Lu <[hidden email]> wrote:

> On Sat, Dec 4, 2010 at 9:34 AM, H.J. Lu <[hidden email]> wrote:
>> On Fri, Dec 3, 2010 at 10:07 PM, H.J. Lu <[hidden email]> wrote:
>>> On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:
>>>> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>>>>> On 04/12/2010 01:24, H.J. Lu wrote:
>>>>>
>>>>>> I checked in a patch to implement stage 2 linking. Everything
>>>>>> seems to work, including "gcc -static ... -lm".
>>>>>
>>>>>  Any chance you could send a complete diff?
>>>>>
>>>>
>>>> I will submit a complete diff after I fix a few corner cases.
>>>> In the meantime, you can clone my git tree and do a "git diff".
>>>>
>>>
>>> Hi,
>>>
>>> This patch implements 2 stage BFD linker for LTO plugin.
>>> It works with current LTO API on all cases I tested.
>>>
>>> Known issue:  --whole-archive will call plugin on archives with IR
>>> in stage 2 linking. But ld never calls plugin to get back object files.
>>> I will try to avoid it in a follow up patch.
>>>
>>
>> This turns out not a problem. In stage 2 linking, for --whole-archive
>> we call plugin to get symbols in the IR element of an archive and
>> it will be ignored for stage 2 linking.  It is OK since we already get
>> the trans object files back for stage 2 linking.
>>
>> BTW, the new linker passed bootstrap-lto with all default languages.
>> I am planning to include this patch in the next Linux binutils.
>>
>
> I missed the IR object in an archive:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34
>
> This updated patch fixed it.  OK for trunk?
>
We shouldn't clear SEC_EXCLUDE if BFD_PLUGIN is set:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c38

This updated patch fixed it.  OK for trunk?

Thanks.


--
H.J.

binutils-2stage-4.patch (41K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Sun, Dec 5, 2010 at 10:22 AM, H.J. Lu <[hidden email]> wrote:

> On Sat, Dec 4, 2010 at 4:43 PM, H.J. Lu <[hidden email]> wrote:
>> On Sat, Dec 4, 2010 at 9:34 AM, H.J. Lu <[hidden email]> wrote:
>>> On Fri, Dec 3, 2010 at 10:07 PM, H.J. Lu <[hidden email]> wrote:
>>>> On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:
>>>>> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>>>>>> On 04/12/2010 01:24, H.J. Lu wrote:
>>>>>>
>>>>>>> I checked in a patch to implement stage 2 linking. Everything
>>>>>>> seems to work, including "gcc -static ... -lm".
>>>>>>
>>>>>>  Any chance you could send a complete diff?
>>>>>>
>>>>>
>>>>> I will submit a complete diff after I fix a few corner cases.
>>>>> In the meantime, you can clone my git tree and do a "git diff".
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> This patch implements 2 stage BFD linker for LTO plugin.
>>>> It works with current LTO API on all cases I tested.
>>>>
>>>> Known issue:  --whole-archive will call plugin on archives with IR
>>>> in stage 2 linking. But ld never calls plugin to get back object files.
>>>> I will try to avoid it in a follow up patch.
>>>>
>>>
>>> This turns out not a problem. In stage 2 linking, for --whole-archive
>>> we call plugin to get symbols in the IR element of an archive and
>>> it will be ignored for stage 2 linking.  It is OK since we already get
>>> the trans object files back for stage 2 linking.
>>>
>>> BTW, the new linker passed bootstrap-lto with all default languages.
>>> I am planning to include this patch in the next Linux binutils.
>>>
>>
>> I missed the IR object in an archive:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34
>>
>> This updated patch fixed it.  OK for trunk?
>>
>
> We shouldn't clear SEC_EXCLUDE if BFD_PLUGIN is set:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c38
>
> This updated patch fixed it.  OK for trunk?
>

It turns out that my patch also fixes:

http://sourceware.org/bugzilla/show_bug.cgi?id=12277


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Sun, Dec 5, 2010 at 4:11 PM, H.J. Lu <[hidden email]> wrote:

> On Sun, Dec 5, 2010 at 10:22 AM, H.J. Lu <[hidden email]> wrote:
>> On Sat, Dec 4, 2010 at 4:43 PM, H.J. Lu <[hidden email]> wrote:
>>> On Sat, Dec 4, 2010 at 9:34 AM, H.J. Lu <[hidden email]> wrote:
>>>> On Fri, Dec 3, 2010 at 10:07 PM, H.J. Lu <[hidden email]> wrote:
>>>>> On Fri, Dec 3, 2010 at 6:34 PM, H.J. Lu <[hidden email]> wrote:
>>>>>> On Fri, Dec 3, 2010 at 6:23 PM, Dave Korn <[hidden email]> wrote:
>>>>>>> On 04/12/2010 01:24, H.J. Lu wrote:
>>>>>>>
>>>>>>>> I checked in a patch to implement stage 2 linking. Everything
>>>>>>>> seems to work, including "gcc -static ... -lm".
>>>>>>>
>>>>>>>  Any chance you could send a complete diff?
>>>>>>>
>>>>>>
>>>>>> I will submit a complete diff after I fix a few corner cases.
>>>>>> In the meantime, you can clone my git tree and do a "git diff".
>>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> This patch implements 2 stage BFD linker for LTO plugin.
>>>>> It works with current LTO API on all cases I tested.
>>>>>
>>>>> Known issue:  --whole-archive will call plugin on archives with IR
>>>>> in stage 2 linking. But ld never calls plugin to get back object files.
>>>>> I will try to avoid it in a follow up patch.
>>>>>
>>>>
>>>> This turns out not a problem. In stage 2 linking, for --whole-archive
>>>> we call plugin to get symbols in the IR element of an archive and
>>>> it will be ignored for stage 2 linking.  It is OK since we already get
>>>> the trans object files back for stage 2 linking.
>>>>
>>>> BTW, the new linker passed bootstrap-lto with all default languages.
>>>> I am planning to include this patch in the next Linux binutils.
>>>>
>>>
>>> I missed the IR object in an archive:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34
>>>
>>> This updated patch fixed it.  OK for trunk?
>>>
>>
>> We shouldn't clear SEC_EXCLUDE if BFD_PLUGIN is set:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c38
>>
>> This updated patch fixed it.  OK for trunk?
>>
>
> It turns out that my patch also fixes:
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=12277
>
Updated patch, adjusted for plugin ELF symbol visibility bug fix.
OK for trunk?

Thanks.


--
H.J.

binutils-2stage-5.patch (42K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Dave Korn-9
On 06/12/2010 02:20, H.J. Lu wrote:

>>>>> BTW, the new linker passed bootstrap-lto with all default languages.
>>>>> I am planning to include this patch in the next Linux binutils.
>>>>>
>>>> I missed the IR object in an archive:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34
>>>>
>>>> This updated patch fixed it.  OK for trunk?
>>>>
>>> We shouldn't clear SEC_EXCLUDE if BFD_PLUGIN is set:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c38
>>>
>>> This updated patch fixed it.  OK for trunk?
>>>
>> It turns out that my patch also fixes:
>>
>> http://sourceware.org/bugzilla/show_bug.cgi?id=12277
>>
>
> Updated patch, adjusted for plugin ELF symbol visibility bug fix.
> OK for trunk?

  Well, I reckon this patch is great (but don't have the approval rights).
It's passed an lto-bootstrap of gcc on i686-pc-cygwin and the tests are well
underway without anything abnormal showing up.

> +  /* Free the old already linked table and create a new one.  */
> +  bfd_section_already_linked_table_free ();
> +  if (!bfd_section_already_linked_table_init ())
> +    einfo (_("%P%F: Failed to create hash table\n"));
> +
> +  /* Free the old hash table and create a new one.  */
> +  bfd_link_hash_table_free (link_info.output_bfd,
> +    link_info.hash);
> +  link_info.hash
> +    = bfd_link_hash_table_create (link_info.output_bfd);
> +  if (link_info.hash == NULL)
> +    einfo (_("%P%F: can not create hash table: %E\n"));

  If I had known that there was really this little stored state to be unwound
and regenerated, I would have wanted to do it this way in the first place.

> +typedef struct cmdlin_header_struct

  Typo there.

  Tristan, sorry, you must be sick of hearing from me by now, but I notice the
branch was still labile a couple of hours ago... it would be really good if we
could get HJ's patch approved and backported before you spin the release.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Mon, Dec 6, 2010 at 9:23 AM, Dave Korn <[hidden email]> wrote:

> On 06/12/2010 02:20, H.J. Lu wrote:
>
>>>>>> BTW, the new linker passed bootstrap-lto with all default languages.
>>>>>> I am planning to include this patch in the next Linux binutils.
>>>>>>
>>>>> I missed the IR object in an archive:
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c34
>>>>>
>>>>> This updated patch fixed it.  OK for trunk?
>>>>>
>>>> We shouldn't clear SEC_EXCLUDE if BFD_PLUGIN is set:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690#c38
>>>>
>>>> This updated patch fixed it.  OK for trunk?
>>>>
>>> It turns out that my patch also fixes:
>>>
>>> http://sourceware.org/bugzilla/show_bug.cgi?id=12277
>>>
>>
>> Updated patch, adjusted for plugin ELF symbol visibility bug fix.
>> OK for trunk?
>
>  Well, I reckon this patch is great (but don't have the approval rights).
> It's passed an lto-bootstrap of gcc on i686-pc-cygwin and the tests are well
> underway without anything abnormal showing up.
>
>> +       /* Free the old already linked table and create a new one.  */
>> +       bfd_section_already_linked_table_free ();
>> +       if (!bfd_section_already_linked_table_init ())
>> +         einfo (_("%P%F: Failed to create hash table\n"));
>> +
>> +       /* Free the old hash table and create a new one.  */
>> +       bfd_link_hash_table_free (link_info.output_bfd,
>> +                                 link_info.hash);
>> +       link_info.hash
>> +         = bfd_link_hash_table_create (link_info.output_bfd);
>> +       if (link_info.hash == NULL)
>> +         einfo (_("%P%F: can not create hash table: %E\n"));
>
>  If I had known that there was really this little stored state to be unwound
> and regenerated, I would have wanted to do it this way in the first place.
>
>> +typedef struct cmdlin_header_struct
>
>  Typo there.

Fixed.

>  Tristan, sorry, you must be sick of hearing from me by now, but I notice the
> branch was still labile a couple of hours ago... it would be really good if we
> could get HJ's patch approved and backported before you spin the release.

I also fixed a bunch other plugin bugs on trrunk.  If we want a BFD linker with
working plugin support in binutils 2.21, we should just copy all pluging changes
on my hjl/lto branch at

http://git.kernel.org/?p=devel/binutils/hjl/x86.git;a=summary

into 2.21.

Thanks.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
In reply to this post by Dave Korn-9
On Mon, Dec 6, 2010 at 9:23 AM, Dave Korn <[hidden email]> wrote:
>
>  Well, I reckon this patch is great (but don't have the approval rights).
> It's passed an lto-bootstrap of gcc on i686-pc-cygwin and the tests are well
> underway without anything abnormal showing up.
>

Can we close

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690

as invalid and point the linker bug to

http://www.sourceware.org/bugzilla/show_bug.cgi?id=12248

Thanks.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Dave Korn-9
On 06/12/2010 17:44, H.J. Lu wrote:

> On Mon, Dec 6, 2010 at 9:23 AM, Dave Korn <[hidden email]> wrote:
>>  Well, I reckon this patch is great (but don't have the approval rights).
>> It's passed an lto-bootstrap of gcc on i686-pc-cygwin and the tests are well
>> underway without anything abnormal showing up.
>>
>
> Can we close
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690
>
> as invalid

  It's not invalid; GOLD linking with -static (explicit or implicit) would
still have problems without the pass through.

> and point the linker bug to
>
> http://www.sourceware.org/bugzilla/show_bug.cgi?id=12248

  I added a reference to it to the gcc PR and closed it.

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Mon, Dec 6, 2010 at 10:19 AM, Dave Korn <[hidden email]> wrote:

> On 06/12/2010 17:44, H.J. Lu wrote:
>> On Mon, Dec 6, 2010 at 9:23 AM, Dave Korn <[hidden email]> wrote:
>>>  Well, I reckon this patch is great (but don't have the approval rights).
>>> It's passed an lto-bootstrap of gcc on i686-pc-cygwin and the tests are well
>>> underway without anything abnormal showing up.
>>>
>>
>> Can we close
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42690
>>
>> as invalid
>
>  It's not invalid; GOLD linking with -static (explicit or implicit) would
> still have problems without the pass through.
>

Personally, I think 2 stage linking is one way to fix this issue.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Alan Modra-3
On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
> Personally, I think 2 stage linking is one way to fix this issue.

Ian has stated that he thinks this is a really bad idea.  I haven't
approved the patch because I value Ian's opinion, and can see why he
thinks it is the wrong way to go.  On the other hand, BFD is full of
bad ideas..  I'm not strongly opposed to your patch myself.

HJ, you showed that link times for gcc did not regress too much with
your 2 stage lto link patch.  It would be more interesting to see the
results for a large C++ project, mozilla for example.

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Mon, Dec 6, 2010 at 3:29 PM, Alan Modra <[hidden email]> wrote:

> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>> Personally, I think 2 stage linking is one way to fix this issue.
>
> Ian has stated that he thinks this is a really bad idea.  I haven't
> approved the patch because I value Ian's opinion, and can see why he
> thinks it is the wrong way to go.  On the other hand, BFD is full of
> bad ideas..  I'm not strongly opposed to your patch myself.
>
> HJ, you showed that link times for gcc did not regress too much with
> your 2 stage lto link patch.  It would be more interesting to see the
> results for a large C++ project, mozilla for example.
>

I don't have such programs at hand. Will timings from gccgo, which is
written in C++, help?


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Ian Lance Taylor-3
In reply to this post by Alan Modra-3
Alan Modra <[hidden email]> writes:

> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>> Personally, I think 2 stage linking is one way to fix this issue.
>
> Ian has stated that he thinks this is a really bad idea.  I haven't
> approved the patch because I value Ian's opinion, and can see why he
> thinks it is the wrong way to go.  On the other hand, BFD is full of
> bad ideas..  I'm not strongly opposed to your patch myself.

Why don't we spend a short amount of time fixing these relatively minor
issues in lto-plugin without going all the way to two-stage linking?
Both Cary and I made several suggestions for how to do this.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Ian Lance Taylor-3
In reply to this post by H.J. Lu-30
"H.J. Lu" <[hidden email]> writes:

> I don't have such programs at hand. Will timings from gccgo, which is
> written in C++, help?

gccgo by itself is not really a large C++ program.  It's only about
50,000 lines of C++.

Building gcc with --enable-build-with-cxx would get you a large C++
program, but of course it would not be very typical of C++ programs.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
In reply to this post by Ian Lance Taylor-3
On Mon, Dec 6, 2010 at 3:54 PM, Ian Lance Taylor <[hidden email]> wrote:

> Alan Modra <[hidden email]> writes:
>
>> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>>> Personally, I think 2 stage linking is one way to fix this issue.
>>
>> Ian has stated that he thinks this is a really bad idea.  I haven't
>> approved the patch because I value Ian's opinion, and can see why he
>> thinks it is the wrong way to go.  On the other hand, BFD is full of
>> bad ideas..  I'm not strongly opposed to your patch myself.
>
> Why don't we spend a short amount of time fixing these relatively minor
> issues in lto-plugin without going all the way to two-stage linking?

The issues may be "relatively minor".  But proper fix without
two-stage linking may be quite tricky.

> Both Cary and I made several suggestions for how to do this.

Fixing all known/unknown issues may not be easy without
two-stage linking.  With two-stage linking, everything just works.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
In reply to this post by Ian Lance Taylor-3
On Mon, Dec 6, 2010 at 3:55 PM, Ian Lance Taylor <[hidden email]> wrote:

> "H.J. Lu" <[hidden email]> writes:
>
>> I don't have such programs at hand. Will timings from gccgo, which is
>> written in C++, help?
>
> gccgo by itself is not really a large C++ program.  It's only about
> 50,000 lines of C++.
>
> Building gcc with --enable-build-with-cxx would get you a large C++
> program, but of course it would not be very typical of C++ programs.
>

I can give it a try if it will help make a decision.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

Ian Lance Taylor-3
In reply to this post by H.J. Lu-30
"H.J. Lu" <[hidden email]> writes:

> On Mon, Dec 6, 2010 at 3:54 PM, Ian Lance Taylor <[hidden email]> wrote:
>> Alan Modra <[hidden email]> writes:
>>
>>> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>>>> Personally, I think 2 stage linking is one way to fix this issue.
>>>
>>> Ian has stated that he thinks this is a really bad idea.  I haven't
>>> approved the patch because I value Ian's opinion, and can see why he
>>> thinks it is the wrong way to go.  On the other hand, BFD is full of
>>> bad ideas..  I'm not strongly opposed to your patch myself.
>>
>> Why don't we spend a short amount of time fixing these relatively minor
>> issues in lto-plugin without going all the way to two-stage linking?
>
> The issues may be "relatively minor".  But proper fix without
> two-stage linking may be quite tricky.

I see no particular reason why that should be the case.  The issues are
conceptually simple.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Mon, Dec 6, 2010 at 4:08 PM, Ian Lance Taylor <[hidden email]> wrote:

> "H.J. Lu" <[hidden email]> writes:
>
>> On Mon, Dec 6, 2010 at 3:54 PM, Ian Lance Taylor <[hidden email]> wrote:
>>> Alan Modra <[hidden email]> writes:
>>>
>>>> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>>>>> Personally, I think 2 stage linking is one way to fix this issue.
>>>>
>>>> Ian has stated that he thinks this is a really bad idea.  I haven't
>>>> approved the patch because I value Ian's opinion, and can see why he
>>>> thinks it is the wrong way to go.  On the other hand, BFD is full of
>>>> bad ideas..  I'm not strongly opposed to your patch myself.
>>>
>>> Why don't we spend a short amount of time fixing these relatively minor
>>> issues in lto-plugin without going all the way to two-stage linking?
>>
>> The issues may be "relatively minor".  But proper fix without
>> two-stage linking may be quite tricky.
>
> I see no particular reason why that should be the case.  The issues are
> conceptually simple.

I'd like to a gold implementation which works on all known cases.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: 2 stage BFD linker for LTO plugin

H.J. Lu-30
On Mon, Dec 6, 2010 at 5:02 PM, H.J. Lu <[hidden email]> wrote:

> On Mon, Dec 6, 2010 at 4:08 PM, Ian Lance Taylor <[hidden email]> wrote:
>> "H.J. Lu" <[hidden email]> writes:
>>
>>> On Mon, Dec 6, 2010 at 3:54 PM, Ian Lance Taylor <[hidden email]> wrote:
>>>> Alan Modra <[hidden email]> writes:
>>>>
>>>>> On Mon, Dec 06, 2010 at 09:57:14AM -0800, H.J. Lu wrote:
>>>>>> Personally, I think 2 stage linking is one way to fix this issue.
>>>>>
>>>>> Ian has stated that he thinks this is a really bad idea.  I haven't
>>>>> approved the patch because I value Ian's opinion, and can see why he
>>>>> thinks it is the wrong way to go.  On the other hand, BFD is full of
>>>>> bad ideas..  I'm not strongly opposed to your patch myself.
>>>>
>>>> Why don't we spend a short amount of time fixing these relatively minor
>>>> issues in lto-plugin without going all the way to two-stage linking?
>>>
>>> The issues may be "relatively minor".  But proper fix without
>>> two-stage linking may be quite tricky.
>>
>> I see no particular reason why that should be the case.  The issues are
>> conceptually simple.
>
> I'd like to a gold implementation which works on all known cases.
>

BTW, gold LTO plugin miscompiled 416.gamess in SPEC CPU 2006:

http://www.sourceware.org/bugzilla/show_bug.cgi?id=12244

BFD linker plugin is OK.

--
H.J.
12