Openpower toolchain rework #3625
Conversation
There was a problem hiding this comment.
A few nits and questions here and there.
I did the naive thing:
$ . op-build-env
$ op-build O=$(pwd)/output-toolchain-rework blackbird_defconfig
$ op-build O=$(pwd)/output-toolchain-rework toolchain
But this only built what would have been built previously, I didn't get the applicable PPE42 toolchain built. So it feels there's something missing in the dependencies
| # consider them to make the sdk unique | ||
| buildroot/utils/config --file $SDK_BUILD_DIR/.config --package \ | ||
| --enable P8_PORE_TOOLCHAIN --enable HOST_P8_PORE_BINUTILS \ | ||
| --enable PPE42_TOOLCHAIN --enable HOST_PPE42_GCC --enable HOST_PPE42_BINUTILS |
There was a problem hiding this comment.
I'm trying to work out why this is needed and not caught by the defconfig that the SDK is being built for.
Is this a remnant of my old hack where "assume SDK for platform X is okay for the rest of them" that was part of the Jenkins CI setup before I left IBM?
There was a problem hiding this comment.
I'm trying to work out why this is needed and not caught by the defconfig that the SDK is being built for.
In a witherspoon_defconfig, ppe42-toolchain would already be selected. In a garrison_defconfig, p8-pore-toolchain. In a zz_defconfig, none of them would be selected.
The idea was to try forcibly select them to make the SDK as reusable as possible, so we have a single sdk package capable of building both witherspoon as well as garrisons (but not zz - see below)
Is this a remnant of my old hack where "assume SDK for platform X is okay for the rest of them" that was part of the Jenkins CI setup before I left IBM?
It's actually quite the opposite.
If you use pristine _defconfig, we will build a SDK that will be the representation of the toolchain that was used by that particular _defconfig:
- witherspoon_defconfig builds a gcc 6.5.0 with C++ support (for hostboot)
- zz_defconfig builds a gcc 7.4.0 without C++ support (i.e., hostboot is not required)
This was causing hazards such as zz being built with gcc 7.4.0 when built back-to-back, and using gcc 6.5.0 when using the (witherspoon-based) sdk. This would be theoretically fine, but for CI consistency purposes, I decided to rework that with #3586 .
openpower/package/hcode/Config.in
Outdated
| @@ -1,6 +1,7 @@ | |||
| config BR2_PACKAGE_HCODE | |||
| bool "hcode" | |||
| default y if (BR2_OPENPOWER_PLATFORM && BR2_OPENPOWER_POWER9) | |||
| depends on BR2_PACKAGE_HAS_PPE42_TOOLCHAIN | |||
There was a problem hiding this comment.
tab indentation when rest of file is spaces
There was a problem hiding this comment.
yeah quite honestly I noted that but decided to ignore at the time considering our Makefiles and shell scripts are all over the place regarding tab vs space.. But I don't want to make it worse, so I'll fix these - thanks
openpower/package/occ-p8/Config.in
Outdated
| @@ -1,6 +1,7 @@ | |||
| config BR2_PACKAGE_OCC_P8 | |||
| bool "OCC for P8" | |||
| default y if (BR2_OPENPOWER_PLATFORM && BR2_OPENPOWER_POWER8) | |||
| depends on BR2_PACKAGE_HAS_P8_PORE_TOOLCHAIN | |||
There was a problem hiding this comment.
tab indentation when rest of file is spaces
| @@ -1,6 +1,11 @@ | |||
| config BR2_PACKAGE_HOST_P8_PORE_BINUTILS | |||
| bool "p8-pore-binutils" | |||
| default y if (BR2_OPENPOWER_POWER8) | |||
| select BR2_PACKAGE_HAS_P8_PORE_TOOLCHAIN | |||
There was a problem hiding this comment.
looks like more tabs vs spaces
| @@ -1,6 +1,6 @@ | |||
| config BR2_PACKAGE_HOST_PPE42_BINUTILS | |||
| bool "ppe42-binutils" | |||
| default y if (BR2_OPENPOWER_PLATFORM && BR2_OPENPOWER_POWER9) | |||
| default y if BR2_PACKAGE_HOST_PPE42_GCC | |||
There was a problem hiding this comment.
Should this change be separate? Why are we changing the default selection?
There was a problem hiding this comment.
- I moved the
default y if (BR2_OPENPOWER_POWER9)to the ppe42-toolchain virtual package definition (https://github.com/open-power/op-build/pull/3625/files#diff-dc9e0d3bb515095fc9adb28b93f03622R4) BR2_OPENPOWER_PLATFORMwas already protecting$BR2_EXTERNAL_OP_BUILD_PATH/package/Config.inwhich was includingopenpower/package/ppe42-binutils/Config.into begin with, so it was a noop. But note that I'm now includingopenpower/package/ppe42-binutils/Config.inas part of the OpenPower Toolchain menu which is NOT protected byBR2_OPENPOWER_PLATFORM- reason is to hopefully make it easier to disable all target packages (by unsettingBR2_OPENPOWER_PLATFORM) while still allowing the openpower toolchain packages to be built.default y if BR2_PACKAGE_HOST_PPE42_GCCcould be a the reminiscence of a hack that I may need to do some more testing on. Originally, SBE, HCODE and OCC were depending on host-ppe42-gcc in their .mk files, and ppe42-gcc depends on ppe42-binutils. When converting to use virtual-packages, somewhere along the way buildroot started complaining about having dependencies on .mk files that were not selected by the Config.in file. I'll do some more testing on this
| select BR2_PACKAGE_HAS_PPE42_TOOLCHAIN | ||
| select BR2_PACKAGE_PPE42_BINUTILS |
There was a problem hiding this comment.
This may be related with the hack mentioned above, but I feel like keeping this one because ppe42-gcc.mk depends on host-ppe42-binutils, but it's config file doesn't explicitly select it (it was always selected due to the common criteria before)
| @@ -1,7 +1,7 @@ | |||
| config BR2_PACKAGE_HOST_PPE42_GCC | |||
| bool "ppe42-gcc" | |||
| default y if (BR2_OPENPOWER_PLATFORM && BR2_OPENPOWER_POWER9) | |||
There was a problem hiding this comment.
Same answer as the comment for openpower/package/ppe42-binutils/Config.in
toolchain is a buildroot virtual-package. If you call I was trying to avoid forcibly making the ppe42 (or p8-pore) dependencies for host-gcc-final or playing with dependencies for buildroot's toolchain virtual packages (reason: feels like an inner buildroot thing not supposed to be used by external trees), so I created a parallel toolchain package chain for openpower. host-ppe42-toolchain is a virtual package provided by host-ppe42-gcc which depends on host-ppe42-binutils. So in order to just build both toolchains (buildroot's native as well as the openpower specific) you'll have to: (you can also build the ppe42-toolchain or p8-pore-toolchain standalone as they are not depending on buildroot's toolchain) Similar to how buildroot does the sdk logic, host-ppe42-toolchain can also be provided by an external package, host-ppe42-toolchain-external - and that's what this change is really about. |
Create a "toolchain" category, plus "p8-pore-toolchain" and "ppe42-toolchain" virtual-packages, with the goal of lowering the coupling between these toolchain and target packages. Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Create the infrastructure necessary to bundle the p8-pore-binutils as part of the sdk build, allowing it to be re-inserted as part of an external sdk. Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Create the infrastructure necessary to bundle ppe42-gcc and ppe42-binutils as part of the sdk build, allowing it to be re-inserted as part of an external sdk. Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
|
rebased with the latest and re-triggered test |
Rework op-build to allow low-coupling of OpenPower toolchain packages (namely p8-pore-binutils, ppe42-gcc and ppe42-binutils) so that we can build them as part of the sdk, and re-insert them.
Hopefully this could speed-up builds, specially for CI, since these components rarely changes.