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.

Sign up for GitHub

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

Jump to bottom

ci: Test and build with more clang versions #1208

Open
javierhonduco opened this issue Jan 10, 2023 · 5 comments · May be fixed by #2332
Open

ci: Test and build with more clang versions #1208

javierhonduco opened this issue Jan 10, 2023 · 5 comments · May be fixed by #2332
Labels
area/build-pipeline Something to do with CI and build pipeline help wanted Extra attention is needed

Comments

@javierhonduco
Copy link
Contributor

We use Clang (and related LLVM toolchain) version 11 released the 12 October 2020 in CI for builds and tests.

Locally, some of us run more modern versions of Clang and LLVM. My system reports:

$ clang --version
clang version 14.0.5 (Fedora 14.0.5-2.fc36)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

It would be great to have a matrix of Clang / LLVM versions that we want to support. I think we should have:

  • The minimum version of the LLVM we want to support (e.g. LLVM 11)
  • The "stable" version of LLVM that most of us run (e.g. LLVM 14);
  • Latest LLVM, either a release candidate or some recent version from their main branch, to ensure that there are no issues with the latest changes upstream and our BPF code

By doing this we will ensure that everything works as expected with new LLVM releases. It will also help increase our confidence when bumping the LLVM version in CI, taking advantage of the latest improvements in our BPF compilation toolchain.

Given that this would increase the build time, perhaps this could be a periodic action that we run on a weekly basis.
@javierhonduco javierhonduco added the help wanted Extra attention is needed label Jan 10, 2023
@kakkoyun
Copy link
Member

@javierhonduco Just one note, this is mainly related to the dev environment. We should add tests to run on the local environment. We provide the builds, and our CI pipelines use the LLVM 11. So, running against only the minimum required version in CI should be sufficient.

@javierhonduco
Copy link
Contributor Author

I agree that the releases we provide should be working fine as we test and build with LLVM and Clang 11. The idea of having CI test on other LLVM versions is twofold:

  1. Testing with the LLVM + Clang that most of us use to develop locally;
  2. Help us catch potential issues before we decide to upgrade LLVM versions;

Imagine that our BPF code compiled with some LLVM version (14 or latest, for example) generates some bytecode that verifiers reject. This has happened in the past and might happen again. By testing this periodically we can report this upstream (or start looking for a workaround if this is a blocker) and get this sorted before we decide to upgrade.

All this is in consideration that in the next few months, we should upgrade LLVM as we run an old version and might be leaving optimisations on the table, but this can wait, of course, and I think that first testing with multiple LLVM versions would greatly help us.

@javierhonduco javierhonduco mentioned this issue Jan 10, 2023
Closed
@kakkoyun kakkoyun added the area/build-pipeline Something to do with CI and build pipeline label Feb 9, 2023
@javierhonduco
Copy link
Contributor Author

Bumping to LLVM 14 fixes a verifier issue in 5.4 #1356

@Namanl2001
Copy link
Contributor

I agree with the idea above of building with various Clang versions weekly to get timely alerts. 16 is the latest LLVM version, should we add a weekly job for this version?

@javierhonduco
Copy link
Contributor Author

@Namanl2001 that would be awesome! It will help us catch potential regressions in code generation affecting our code being accepted or not by the verifier (among other things)

@Namanl2001 Namanl2001 linked a pull request Dec 3, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/build-pipeline Something to do with CI and build pipeline help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

weekly build script to test on latest clang version Namanl2001/parca-agent
3 participants
@kakkoyun @javierhonduco @Namanl2001