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

Enable link time optimization (LTO) for s390x #2758

Closed
danbev opened this issue Sep 10, 2021 · 11 comments
Closed

Enable link time optimization (LTO) for s390x #2758

danbev opened this issue Sep 10, 2021 · 11 comments
Assignees
Labels

Comments

@danbev
Copy link
Contributor

danbev commented Sep 10, 2021

There is currently a daily job that enables link time optimization (LT0) and we are wondering if it would be possible to create new CI jobs, or modify existing ones, to enable LTO for s390x and i686:

$ ./configure --enable-lto

The motivation for this is that at Red Hat/IBM we have run into issues with these platforms and it would be great if these could be discovered early.

@richardlau
Copy link
Member

We don't build/test on i686 since Node.js 10 (see #885 for the whole discussion). 32-bit Linux binaries are built unofficially over in https://unofficial-builds.nodejs.org/ but those are not regularly checked and don't have tests run. We only have a few Linux -x86 platforms left in the CI (for node-addon-api and libuv, no Node.js builds run on them anymore) on older OS'es (Ubuntu 1404 and 1604) that I'd like to get rid of.

s390 would be reasonably easy to add without having to add a new CI job -- an extra call to https://ci.nodejs.org/job/node-test-commit-linuxone/ (with CONFIG_FLAGS build parameter set) from https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/.

@danbev danbev changed the title Enable link time optimization (LTO) for s390x and i686 Enable link time optimization (LTO) for s390x Sep 10, 2021
@richardlau
Copy link
Member

Ah the other problem might be is that we don't have later versions of gcc on the CI hosts. For s390x we have devtoolsets 6 and 8.

@mhdawson
Copy link
Member

There is a dependency on the compiler in order to be able to enable LTO ?

@richardlau
Copy link
Member

@mhdawson It was reported broken on gcc 9: nodejs/node#38570. Rereading it might be okay on gcc 8 -- I've started some CI runs with --enable-lto on s390x to check.

@richardlau
Copy link
Member

https://ci.nodejs.org/job/node-test-commit-linuxone/28796/ passed with --enable-lto so compiler isn't an issue.

@mhdawson
Copy link
Member

@richardlau so that's good, I wonder if it would make sense to add it as another target under the make PR testing job? linuxone is fast enough that it would likely not cause any issues.

@richardlau
Copy link
Member

I would very much prefer not having to add a brand new job into the CI for testing LTO -- I'd rather call the existing job(s) (e.g. node-test-commit-linuxone) a second time with the CONFIG_FLAGS build parameter set to --enable-lto.

If we add to the PR testing job (as opposed to daily master) then we need to take into account all the release lines (as we use the same job to test backport PRs). Test builds with --enable-lto show that Node.js 12 and 14 are currently broken in that configuration:
Node.js 12: https://ci.nodejs.org/job/node-test-commit-linuxone/28835/
Node.js 14: https://ci.nodejs.org/job/node-test-commit-linuxone/28836/
Node.js 16 (passes): https://ci.nodejs.org/job/node-test-commit-linuxone/28838/

@mhdawson
Copy link
Member

mhdawson commented Sep 14, 2021

I'm thinking we must have a way to filter out jobs for particular node versions. Maybe using the version selector script we can filter it out by having a new platform, something like s390x-lto ?

That might also give us the way to run the same job with --enable-lto ?

@richardlau
Copy link
Member

I've added an s390x LTO build into node-test-commit -- a second invocation of node-test-commit-linuxone with CONFIG_FLAGS set to --enable-lto run on condition ${NODE_MAJOR_VERSION} >= 16

Audit

Test builds

12:35:59 Skipping node-test-commit-linuxone (LTO). Condition was evaluated to false.

@richardlau richardlau added ci-change PSA of configuration changes platform:linuxONE labels Sep 15, 2021
@richardlau richardlau self-assigned this Sep 15, 2021
@mhdawson
Copy link
Member

Looks great. @richardlau thanks !

@richardlau
Copy link
Member

I've had to change the LTO build to be a second "platform" matrix configuration so that "resume build" will properly resume LTO builds -- it looks like the multijob plugin has some issues with multiple invocations of the same job with different parameters when resuming.

Audit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants