Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce an Alternate Toolchain option with gcc and binutils #3564

Closed
wants to merge 7 commits into from
Closed

Introduce an Alternate Toolchain option with gcc and binutils #3564

wants to merge 7 commits into from

Conversation

klauskiwi
Copy link

@klauskiwi klauskiwi commented Apr 15, 2020

  • Introduces an "Alternate Toolchain" concept, where an alternate version of gcc and binutils can be defined (separate from buildroot's Toolchain)
  • Make use of it on Hostboot, Hostboot-P8 and OCC-P8, which are packages that are known to not work with more recent GCC versions
  • Move configurations to use the default buildroot GCC version (7.x at the time of this writing)

@klauskiwi klauskiwi requested review from shenki and oohal April 15, 2020 21:33
@klauskiwi
Copy link
Author

@oohal @shenki care to take a look?

Copy link
Contributor

@stewartsmith stewartsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended for just POWER8 or also POWER9?

This patch itself does it for the POWER9 hostboot, not the POWER8 one.

It's also a pretty terrible ugly hack to build YET ANOTHER toolchain (what are we up to now? PORE, PPE, 405, and the dual BE/LE ppc64). So, we're already at five, and this is a sixth. (or 4 then 5 if you want to argue the dual BE/LE is actually one).

What does this add to the build time? Can it be brought into the buildroot toolchain building target so I can stop needlessly building 101 toolchains every time I want to build a new op-build?

openpower/package/hostboot/Config.in Outdated Show resolved Hide resolved
Copy link
Contributor

@stewartsmith stewartsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I very strongly disagree with the first fragment of the commit message "In the ideal buildroot world".

It should read "In an ideal world, Hostboot would be a maintained piece of software." Because that is fundamentally what this is working around - that nobody is maintaining the versions of hostboot that anybody uses.

@klauskiwi
Copy link
Author

klauskiwi commented Apr 15, 2020

Is this intended for just POWER8 or also POWER9?
This patch itself does it for the POWER9 hostboot, not the POWER8 one.

I enabled the alternate gcc for hostboot-p8, occ-p8 and hostboot which were the packages I couldn't build using gcc7. I created this draft PR to get comments and use the CI to exercise more configurations that I may not have tested locally.

It's also a pretty terrible ugly hack to build YET ANOTHER toolchain (what are we up to now? PORE, PPE, 405, and the dual BE/LE ppc64). So, we're already at five, and this is a sixth. (or 4 then 5 if you want to argue the dual BE/LE is actually one).

I agree

What does this add to the build time? Can it be brought into the buildroot toolchain building target so I can stop needlessly building 101 toolchains every time I want to build a new op-build?

It will probably add some time. I'll investigate a bit more if adding this as part of the SDK is possible, but I don't think that's trivial. i.e., I don't think ppe or pore are doing that

@klauskiwi
Copy link
Author

klauskiwi commented Apr 15, 2020

I very strongly disagree with the first fragment of the commit message "In the ideal buildroot world".

It should read "In an ideal world, Hostboot would be a maintained piece of software." Because that is fundamentally what this is working around - that nobody is maintaining the versions of hostboot that anybody uses.

There's actually another reason for this: This is necessary for long-term downstream releases where we need to keep hostboot and the such exactly the same, while rebasing the skiroot environment with a newer release (i.e., because the kernel or some other component is out of service)

@dcrowell77
Copy link
Contributor

It should read "In an ideal world, Hostboot would be a maintained piece of software." Because that is fundamentally what this is working around - that nobody is maintaining the versions of hostboot that anybody uses.

We are happy to take a set of pull requests for the master-p8 branch from the community to make it work on both the current and the newer GCC levels if that is something that the community thinks is important.

@shenki
Copy link
Member

shenki commented Apr 16, 2020

It should read "In an ideal world, Hostboot would be a maintained piece of software." Because that is fundamentally what this is working around - that nobody is maintaining the versions of hostboot that anybody uses.

We are happy to take a set of pull requests for the master-p8 branch from the community to make it work on both the current and the newer GCC levels if that is something that the community thinks is important.

I like this suggestion.

Here's a hostboot pull request: open-power/hostboot#193
Here's one for OCC: open-power/occ#28

With these changes I was able to build master for P8 with GCC 8 when I tested in January.

@klauskiwi
Copy link
Author

Please note that modernizing Hostboot et al to use more modern compiler is definitely critical, but not the main objective of carrying around an "alternate toolchain".

We need to have the ability to rebase "only" the Skiroot environment without invalidating tests done to Hostboot, Skiboot and such. The move to the default gcc7 compiler was a way to exercise this, not exactly the end goal.

Klaus Heinrich Kiwi added 7 commits April 16, 2020 12:46
Fix the ci/build-all-defconfigs.sh to consider SDK versions with
GCC 6, 7 or 8.

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
In the ideal buildroot world, there is one definition of a Toolchain
that will be used to build the entire set of packages used in a
distribution.

However, in the OpenPower world, this rule is not completely true, due
to:

 1) The need to build for two different target architectures at once
    (ppc64le and ppe42) - this is already addressed by the ppe42-gcc
    and ppe42-binutils packages

 2) The fact that some of the legacy packages are not actively
    maintained, but still needed (such as Hostboot-P8), while other
    packages are actively updated and could benefit using newer
    compilers (Linux Kernel, Skiboot etc).

Considering that (1) is already solved, and that (2) is desirable, this
patch reuses the mechanism used by (1) but to create an "Alternate"
Toolchain (initially composed of gcc and binutils) that could be used by
packages that are not willing or able to move towards newer compilers.

Once in use by a subset of packages, this also may allow for downstream
branches to keep using an older toolchain, while buildroot is re-based,
without necessarily moving the snapshot where those packages were
tested. This could be useful, for example, for upgrading Skiroot
environment completely without touching Hostboot, OCC, HCODE or even
Skiboot for that matter.

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
Enable Hostboot-P8 to optionally use an Alternate Toolchain

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
Enable OCC-P8 to optionally use an Alternate Toolchain

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
Enable Hostboot to optionally use an Alternate Toolchain

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
Enable OCC to optionally use an Alternate Toolchain

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
In order to exercise the Alternate Toolchain mechanism, unset
BR2_GCC_VERSION_6_X in order to use buildroot's default GCC 7, while
also enabling the Alternate GCC for all OCC and Hostboot packages.

Signed-off-by: Klaus Heinrich Kiwi <[email protected]>
@klauskiwi
Copy link
Author

Re-cycled into a new PR #4063

@klauskiwi klauskiwi closed this Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants