mixed LTO support for 'ld -r'

Hi Nicolas,

Post by Nicolas Pitre
Hello toolchain gurus,
In the course of Linaro's kernel tinification project, the ability to
compile the Linux kernel using LTO is a frequent requirement. However
the kernel makes heavy usage of 'ld -r' with .o files resulting from LTO
build of .c files as well as .o files resulting from pure assembly code.
This mix of LTO and non-LTO object files is not supported by upstream
binutils unless a patch from H.J. Lu is applied. That patch has been
available since 2013 and was last refreshed in his 2.25.51.0.4 branch
https://git.linaro.org/toolchain/binutils-gdb.git/commit/6da5456971
I've attached a very simple test case demonstrating the problem. With
the binutils-lto-mixed.patch applied, this test case compiles to a
working executable. Otherwise compilation fails at the 'ld -r' step.
- What, if anything, has prevented this patch from being merged in the
master branch upstream?

I am trying to find the original thread, if any, where H.J. Lu sent the
initial patch to figure out why exactly it has not been already merged
upstream.

However based on past iterations with GNU projects it can be due some
reasons:

1. Limited review from code maintainers which do not cover full patch;
2. Lack of code review follow up from developers based on initial
comments;
3. Some missing functionality or inherent design flaw. Sometimes it
is something that only appear in a distinct configuration or
platform.

Post by Nicolas Pitre
- In the mean time, could we include this patch in the Linaro binutils
package and releases?

And I enumerate the possible reasons to mainly discuss if it would be
worthwhile to include it on linaro binutils branch. It is a large patch
(it even adds a new elf special section), it has not been extensively
tested in same way as the releases branches (does any distro include
this patchset?), and based on ChangeLog it seems only H.J. Lu has been
working on this (which in turn only he fully understand it).

I am not against integrate this patch in our release, however I would
strongly prefer to have it merged first to avoid future update/rebase
conflicts. If it is not a current option or if the time-frame won't
allow I would like to understand what is preventing it to be included in
master to check if the any issues with this patchset.

Maybe we can contact H.J. Lu to check what is preventing him to merge
this branch and work to help him on this path.

Post by Nicolas Pitre
Having this available in our toolchain releases would greatly simplify
the LTO related work on the kernel. It was included in all binutils
releases from H.J. Lu since 2013 and therefore has obtained significant
exposure already.
Thanks.
Nicolas
_______________________________________________
linaro-toolchain mailing list
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Jim Wilson

2015-12-21 23:50:40 UTC

On Mon, Dec 21, 2015 at 4:55 AM, Adhemerval Zanella

Post by Nicolas Pitre
This mix of LTO and non-LTO object files is not supported by upstream
binutils unless a patch from H.J. Lu is applied. That patch has been
available since 2013 and was last refreshed in his 2.25.51.0.4 branch

November 2013 is when we switched from svn to git, which is why the
git branch was created then.

I tracked the bulk of the patch back to April 2011, though some new
LTO related testsuite changes date back to January 2011. The initial
patch submission for the bulk of the patch appears to be
https://sourceware.org/ml/binutils/2011-04/msg00275.html
It is a large patch, and HJ had to update it twice in the next 24
hours to fix problems with it. The size would have discouraged an
immediate review. And the fact that it was updated twice in 24 hours
after posting would have discouraged reviewers even more. People were
perhaps waiting for the final version of the patch before trying to
review it, and then accidentally forgot about it along the way. I
don't see any discussion of the patch at the time. And I haven't seen
any attempt to resubmit it, though I could have missed something.

I see that the issue was discussed earlier in December 2010. HJ made
a proposal for a fix, and there was feedback at that time.
https://gcc.gnu.org/ml/gcc/2010-12/msg00229.html
it looks like there were 3 separate related threads which may have
confused the issue a bit.
https://sourceware.org/ml/binutils/2010-12/msg00012.html
https://sourceware.org/ml/binutils/2010-12/msg00182.html
https://sourceware.org/ml/binutils/2010-12/msg00231.html

Anyways, the size of the patch suggests using caution and waiting for
upstream review. Though I did find a reference that suggests Fedora
is using it
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20130513/1022584.html
which suggests that it may be well tested. This was done by Nick
Clifton, who is one of the binutils maintainers, so maybe we just need
someone to ask about the status of the patch on the binutils mailing
list to remind people that it still needs to be reviewed for the
upstream FSF binutils tree.

Jim

Nicolas Pitre

2015-12-22 16:22:05 UTC

Post by Jim Wilson
I tracked the bulk of the patch back to April 2011, though some new
LTO related testsuite changes date back to January 2011. The initial
patch submission for the bulk of the patch appears to be
https://sourceware.org/ml/binutils/2011-04/msg00275.html
It is a large patch, and HJ had to update it twice in the next 24
hours to fix problems with it. The size would have discouraged an
immediate review. And the fact that it was updated twice in 24 hours
after posting would have discouraged reviewers even more.

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Post by Jim Wilson
People were perhaps waiting for the final version of the patch before
trying to review it, and then accidentally forgot about it along the
way. I don't see any discussion of the patch at the time. And I
haven't seen any attempt to resubmit it, though I could have missed
something.
I see that the issue was discussed earlier in December 2010. HJ made
a proposal for a fix, and there was feedback at that time.
https://gcc.gnu.org/ml/gcc/2010-12/msg00229.html
it looks like there were 3 separate related threads which may have
confused the issue a bit.
https://sourceware.org/ml/binutils/2010-12/msg00012.html
https://sourceware.org/ml/binutils/2010-12/msg00182.html
https://sourceware.org/ml/binutils/2010-12/msg00231.html
Anyways, the size of the patch suggests using caution and waiting for
upstream review. Though I did find a reference that suggests Fedora
is using it
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20130513/1022584.html
which suggests that it may be well tested. This was done by Nick
Clifton, who is one of the binutils maintainers, so maybe we just need
someone to ask about the status of the patch on the binutils mailing
list to remind people that it still needs to be reviewed for the
upstream FSF binutils tree.

Could you (i.e. someone in the toolchain team) take care of this?

Nicolas

Adhemerval Zanella

2015-12-23 17:37:37 UTC

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Post by Nicolas Pitre
Nicolas

Nicolas Pitre

2015-12-23 17:41:04 UTC

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Great, thank you.

Nicolas

Pinski, Andrew

2015-12-23 20:17:17 UTC

Note I rather see split stack support than ld -r LTO support done. I think most enterprise folks would too.

Thanks,
Andrew

-----Original Message-----
From: linaro-toolchain [mailto:linaro-toolchain-***@lists.linaro.org] On Behalf Of Nicolas Pitre
Sent: Wednesday, December 23, 2015 9:41 AM
To: Adhemerval Zanella <***@linaro.org>
Cc: Jim Wilson <***@linaro.org>; Linaro Toolchain Mailman List <linaro-***@lists.linaro.org>
Subject: Re: mixed LTO support for 'ld -r'

Post by Jim Wilson
I tracked the bulk of the patch back to April 2011, though some new
LTO related testsuite changes date back to January 2011. The
initial patch submission for the bulk of the patch appears to be
https://sourceware.org/ml/binutils/2011-04/msg00275.html
It is a large patch, and HJ had to update it twice in the next 24
hours to fix problems with it. The size would have discouraged an
immediate review. And the fact that it was updated twice in 24
hours after posting would have discouraged reviewers even more.

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Post by Jim Wilson
People were perhaps waiting for the final version of the patch
before trying to review it, and then accidentally forgot about it
along the way. I don't see any discussion of the patch at the
time. And I haven't seen any attempt to resubmit it, though I
could have missed something.
I see that the issue was discussed earlier in December 2010. HJ
made a proposal for a fix, and there was feedback at that time.
https://gcc.gnu.org/ml/gcc/2010-12/msg00229.html
it looks like there were 3 separate related threads which may have
confused the issue a bit.
https://sourceware.org/ml/binutils/2010-12/msg00012.html
https://sourceware.org/ml/binutils/2010-12/msg00182.html
https://sourceware.org/ml/binutils/2010-12/msg00231.html
Anyways, the size of the patch suggests using caution and waiting
for upstream review. Though I did find a reference that suggests
Fedora is using it
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-2
0130513/1022584.html which suggests that it may be well tested.
This was done by Nick Clifton, who is one of the binutils
maintainers, so maybe we just need someone to ask about the status
of the patch on the binutils mailing list to remind people that it
still needs to be reviewed for the upstream FSF binutils tree.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Great, thank you.

Nicolas

Adhemerval Zanella

2016-01-04 13:33:13 UTC

These are orthogonal requests, let's track them independently. Also,
although I noticed s390 work to add split-stack support, my understanding
is it is mainly aimed for go runtime support and current go moved
*away* from split-stack [1]. Which are the current usercases aimed for
split-stack support currently?

[1] http://tip.golang.org/doc/go1.4#runtime

Post by Pinski, Andrew
Note I rather see split stack support than ld -r LTO support done. I think most enterprise folks would too.
Thanks,
Andrew
-----Original Message-----
Sent: Wednesday, December 23, 2015 9:41 AM
Subject: Re: mixed LTO support for 'ld -r'

Post by Jim Wilson
I tracked the bulk of the patch back to April 2011, though some new
LTO related testsuite changes date back to January 2011. The
initial patch submission for the bulk of the patch appears to be
https://sourceware.org/ml/binutils/2011-04/msg00275.html
It is a large patch, and HJ had to update it twice in the next 24
hours to fix problems with it. The size would have discouraged an
immediate review. And the fact that it was updated twice in 24
hours after posting would have discouraged reviewers even more.

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Post by Jim Wilson
People were perhaps waiting for the final version of the patch
before trying to review it, and then accidentally forgot about it
along the way. I don't see any discussion of the patch at the
time. And I haven't seen any attempt to resubmit it, though I
could have missed something.
I see that the issue was discussed earlier in December 2010. HJ
made a proposal for a fix, and there was feedback at that time.
https://gcc.gnu.org/ml/gcc/2010-12/msg00229.html
it looks like there were 3 separate related threads which may have
confused the issue a bit.
https://sourceware.org/ml/binutils/2010-12/msg00012.html
https://sourceware.org/ml/binutils/2010-12/msg00182.html
https://sourceware.org/ml/binutils/2010-12/msg00231.html
Anyways, the size of the patch suggests using caution and waiting
for upstream review. Though I did find a reference that suggests
Fedora is using it
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-2
0130513/1022584.html which suggests that it may be well tested.
This was done by Nick Clifton, who is one of the binutils
maintainers, so maybe we just need someone to ask about the status
of the patch on the binutils mailing list to remind people that it
still needs to be reviewed for the upstream FSF binutils tree.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Great, thank you.
Nicolas

Pinski, Andrew

2016-01-04 17:20:34 UTC

GCC GO has not moved away from split stack.

I think GCC GO should be supported.

There are a few other items on the parity list before ld -r support is needed. Including inside the kernel.

-----Original Message-----
From: Adhemerval Zanella [mailto:***@linaro.org]
Sent: Monday, January 4, 2016 5:33 AM
To: Pinski, Andrew <***@caviumnetworks.com>; Nicolas Pitre <***@linaro.org>
Cc: Jim Wilson <***@linaro.org>; Linaro Toolchain Mailman List <linaro-***@lists.linaro.org>
Subject: Re: mixed LTO support for 'ld -r'

These are orthogonal requests, let's track them independently. Also, although I noticed s390 work to add split-stack support, my understanding is it is mainly aimed for go runtime support and current go moved
*away* from split-stack [1]. Which are the current usercases aimed for split-stack support currently?

[1] http://tip.golang.org/doc/go1.4#runtime

Post by Pinski, Andrew
Note I rather see split stack support than ld -r LTO support done. I think most enterprise folks would too.
Thanks,
Andrew
-----Original Message-----
From: linaro-toolchain
Nicolas Pitre
Sent: Wednesday, December 23, 2015 9:41 AM
Subject: Re: mixed LTO support for 'ld -r'

Post by Jim Wilson
I tracked the bulk of the patch back to April 2011, though some new
LTO related testsuite changes date back to January 2011. The
initial patch submission for the bulk of the patch appears to be
https://sourceware.org/ml/binutils/2011-04/msg00275.html
It is a large patch, and HJ had to update it twice in the next 24
hours to fix problems with it. The size would have discouraged an
immediate review. And the fact that it was updated twice in 24
hours after posting would have discouraged reviewers even more.

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Post by Jim Wilson
People were perhaps waiting for the final version of the patch
before trying to review it, and then accidentally forgot about it
along the way. I don't see any discussion of the patch at the
time. And I haven't seen any attempt to resubmit it, though I
could have missed something.
I see that the issue was discussed earlier in December 2010. HJ
made a proposal for a fix, and there was feedback at that time.
https://gcc.gnu.org/ml/gcc/2010-12/msg00229.html
it looks like there were 3 separate related threads which may have
confused the issue a bit.
https://sourceware.org/ml/binutils/2010-12/msg00012.html
https://sourceware.org/ml/binutils/2010-12/msg00182.html
https://sourceware.org/ml/binutils/2010-12/msg00231.html
Anyways, the size of the patch suggests using caution and waiting
for upstream review. Though I did find a reference that suggests
Fedora is using it
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-2
0130513/1022584.html which suggests that it may be well tested.
This was done by Nick Clifton, who is one of the binutils
maintainers, so maybe we just need someone to ask about the status
of the patch on the binutils mailing list to remind people that it
still needs to be reviewed for the upstream FSF binutils tree.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Great, thank you.
Nicolas

Adhemerval Zanella

2016-01-04 17:03:31 UTC

Multiple revisions in a few days isn't uncommon. But 5 years have
passed at this point.

Could you (i.e. someone in the toolchain team) take care of this?

I will sort this out when I get back from holidays.

Great, thank you.
Nicolas

Nicolas, I am about to start a new thread about ""ld -r" on mixed
IR/non-IR objects", asking current status from H.J. Lu, what is
preventing upstream merge, concerns and objections.

First I would to know which the priority of this feature for your
work on kernel tinification. I am asking it because based on the
threads about previous tries to push this upstream I foresee it
will take some time (either by review or design discussion).

It will also help us discuss the viability to add the patchset in
your branch and the pro and cons of having it out-of-tree.

Nicolas Pitre

2016-01-04 18:13:18 UTC