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 last September. It is accessible here:
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.
One question and one request:
- What, if anything, has prevented this patch from being merged in the master branch upstream?
- In the mean time, could we include this patch in the Linaro binutils package and releases?
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
Hi Nicolas,
On 18-12-2015 19:04, Nicolas Pitre wrote:
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 last September. It is accessible here:
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.
One question and one request:
- 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.
- 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.
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 linaro-toolchain@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-toolchain
On Mon, Dec 21, 2015 at 4:55 AM, Adhemerval Zanella adhemerval.zanella@linaro.org wrote:
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 last September. It is accessible here:
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/1... 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
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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/1... 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
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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/1... 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.
Nicolas
On Wed, 23 Dec 2015, Adhemerval Zanella wrote:
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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/1... 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
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-bounces@lists.linaro.org] On Behalf Of Nicolas Pitre Sent: Wednesday, December 23, 2015 9:41 AM To: Adhemerval Zanella adhemerval.zanella@linaro.org Cc: Jim Wilson jim.wilson@linaro.org; Linaro Toolchain Mailman List linaro-toolchain@lists.linaro.org Subject: Re: mixed LTO support for 'ld -r'
On Wed, 23 Dec 2015, Adhemerval Zanella wrote:
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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
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
On 23-12-2015 18:17, Pinski, Andrew wrote:
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-bounces@lists.linaro.org] On Behalf Of Nicolas Pitre Sent: Wednesday, December 23, 2015 9:41 AM To: Adhemerval Zanella adhemerval.zanella@linaro.org Cc: Jim Wilson jim.wilson@linaro.org; Linaro Toolchain Mailman List linaro-toolchain@lists.linaro.org Subject: Re: mixed LTO support for 'ld -r'
On Wed, 23 Dec 2015, Adhemerval Zanella wrote:
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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
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:adhemerval.zanella@linaro.org] Sent: Monday, January 4, 2016 5:33 AM To: Pinski, Andrew Andrew.Pinski@caviumnetworks.com; Nicolas Pitre nicolas.pitre@linaro.org Cc: Jim Wilson jim.wilson@linaro.org; Linaro Toolchain Mailman List linaro-toolchain@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
On 23-12-2015 18:17, Pinski, Andrew wrote:
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-bounces@lists.linaro.org] On Behalf Of Nicolas Pitre Sent: Wednesday, December 23, 2015 9:41 AM To: Adhemerval Zanella adhemerval.zanella@linaro.org Cc: Jim Wilson jim.wilson@linaro.org; Linaro Toolchain Mailman List linaro-toolchain@lists.linaro.org Subject: Re: mixed LTO support for 'ld -r'
On Wed, 23 Dec 2015, Adhemerval Zanella wrote:
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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
On 23-12-2015 15:41, Nicolas Pitre wrote:
On Wed, 23 Dec 2015, Adhemerval Zanella wrote:
Em 22 de dez de 2015, às 14:22, Nicolas Pitre nicolas.pitre@linaro.org escreveu:
On Mon, 21 Dec 2015, Jim Wilson wrote:
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.
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/1... 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
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.
On Mon, 4 Jan 2016, Adhemerval Zanella wrote:
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.
I currently build my own toolchain in order to have this feature. So it isn't an immediate priority to me personally. However it is a major hurdle for many people who otherwise would contribute to the advancement of LTO and tinification of the kernel who simply won't go as far as building their own toolchain. That's especially true for Linaro members who expect us to produce those builds for them.
It also looks like a chicken or egg problem. Given upstream kernel doesn't support LTO probably explains why ld -r for LTO is not in upstream binutils yet, and that in turn doesn't help LTO for the kernel moving forward.
So if the design is sane enough, having Linaro carry the patch might give the whole thing some momentum that the long upstream binutils review and release cycle on its own doesn't allow...unless this happens more quickly than we might expect.
And the risk is low for Linaro to carry the patch even if upstream people decide to redo the design entirely as usage should be transparent (there is no special command line argument involved, etc.) Maybe a complete recompile might be necessary after a toolchain upgrade, but That's what most people do after a toolchain upgrade anyway.
Nicolas
linaro-toolchain@lists.linaro.org
-
Adhemerval Zanella -
Jim Wilson -
Nicolas Pitre -
Pinski, Andrew