Add nightly workflow to test x86_64 backend against llvm-test-suite - #1536
Add nightly workflow to test x86_64 backend against llvm-test-suite#1536Bhoumik Patidar (BhoumikPatidar) wants to merge 1 commit into
Conversation
01c6ecd to
e871025
Compare
| uses: egor-tensin/setup-clang@23bc15cd4207e45f1566447deb48f4ff3ef932cb #v2.3 | ||
| with: | ||
| version: "20" | ||
| platform: x64 |
There was a problem hiding this comment.
clang 22 to mimic the nightly builder.
| sudo apt install -y libc++-dev libc++abi-dev \ | ||
| libclang-rt-20-dev cmake ninja-build llvm-20 llvm-20-tools | ||
| pip3 install lit | ||
| echo "PATH=/usr/lib/llvm-20/bin:$PATH" >> $GITHUB_ENV |
There was a problem hiding this comment.
follow up : Can we make the nightly builder use a common action to install things, so that whatever LLVM/clang is used is at one place ?
There was a problem hiding this comment.
okay, added to tasks
| with: | ||
| name: llvm-test-suite-results | ||
| path: test-output.txt | ||
| retention-days: 2 |
There was a problem hiding this comment.
retetion days 1?
| run: | | ||
| if grep -q '^FAIL: ' test-output.txt; then | ||
| grep '^FAIL: ' test-output.txt | ||
| exit 1 |
There was a problem hiding this comment.
Do we need this ?
There was a problem hiding this comment.
Not needed, removed it.
| -DCMAKE_C_COMPILER=$(which clang) \ | ||
| -DCMAKE_CXX_COMPILER=$(which clang++) \ | ||
| -DCMAKE_C_FLAGS="-fuse-ld=$ELD_BIN" \ | ||
| -DCMAKE_CXX_FLAGS="-fuse-ld=$ELD_BIN -stdlib=libc++" \ |
There was a problem hiding this comment.
I think you need to use --ld-path. Not sure if -fuse-ld respects eld.
There was a problem hiding this comment.
-fuse-ld does respect eld but emits warning warning: '-fuse-ld=' taking a path is deprecated; use '--ld-path=' instead.
I will switch to --ld-path.
| lit -j$(nproc) \ | ||
| build-llvm-test-suite/SingleSource \ | ||
| build-llvm-test-suite/MultiSource \ | ||
| build-llvm-test-suite/MicroBenchmarks \ |
There was a problem hiding this comment.
Add a comment on why other llvm test suites are not enabled. Add everything and comment it out.
| -DCMAKE_C_FLAGS="-fuse-ld=$ELD_BIN" \ | ||
| -DCMAKE_CXX_FLAGS="-fuse-ld=$ELD_BIN -stdlib=libc++" \ | ||
| -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=$ELD_BIN -stdlib=libc++" \ | ||
| -DTEST_SUITE_SUBDIRS="SingleSource;MultiSource;MicroBenchmarks" \ |
There was a problem hiding this comment.
Do you need this as you are saying lit to run specific folders ?
There was a problem hiding this comment.
currently its needed because we are restricting the folders because of the BitCode failures.
Without this CMake compiles/builds all default that we are not currently executing.
After the BitCode fix, we dont restrict folders so this wont be needed then.
| @@ -0,0 +1,116 @@ | |||
| name: Nightly LLVM Test Suite (x86_64) | |||
There was a problem hiding this comment.
Follow up : enable for other targets as well.
There was a problem hiding this comment.
okay, added to tasks
|
|
||
| on: | ||
| schedule: | ||
| - cron: '0 3 * * *' # 9:00 PM CST |
There was a problem hiding this comment.
This needs to run after the nightly completes, now it overlaps I believe.
There was a problem hiding this comment.
changed to 5:00 AM CST
There was a problem hiding this comment.
If the goal is just to always run it after the nightly, would it be worth using workflow_run (with types as completed) rather than scheduling it?
There was a problem hiding this comment.
Yes, using workflow_run instead of scheduling should be better. This can also prevent cases where the nightly fails and the artifact expires and leads to these workflows failing. Parth (@parth-07) Shankar Easwaran (@quic-seaswara) what do you suggest?
There was a problem hiding this comment.
FWIW, since originaly suggesting this, we've had the zizmor checks turned on which will complain about workflow_run being insecure (we're seeing these errors in CPULLVM right now). So, maybe workflow_run wasn't such a good suggestion in hindsight 😅
There was a problem hiding this comment.
I think workflow_run is a good suggestion for jobs that depend upon nightly artifact for the cases Bhoumik Patidar (@BhoumikPatidar) pointed out
I am using it here : #1611
who added zizmor? is it really needed?
There was a problem hiding this comment.
My understanding from what happened with cpullvm is it was turned on at an organization level (github.com/qualcomm).
Again only based on what we saw in cpullvm, most of the things reported are/were legitimate issues and the fixes follow recommended practices so it's probably good from that perspective.
For workflow_run specifically, I didn't look into why it is/was considered insecure, so can't comment on that part.
| if: github.repository == 'qualcomm/eld' || github.event_name == 'workflow_dispatch' | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| ELD_SOURCE_DIR: ${{ github.workspace }}/llvm-project/llvm/tools/eld |
There was a problem hiding this comment.
We can improve this to run on a PR as well, which would be great.
1da9262 to
0e2a36e
Compare
Steven Ramirez Rosa (Steven6798)
left a comment
Steven Ramirez Rosa (Steven6798)
left a comment
There was a problem hiding this comment.
Great addition to ELD
0e2a36e to
59a019e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Checkout eld | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| path: llvm-project/llvm/tools/eld |
Links and runs SingleSouce(1891 tests), MultiSource(201 tests) and MicroBenchmarks(22 tests) subdirectories from llvm/llvm-test-suite for x86_64. ld.eld is fetched via FetchNightlyToolset. No failures expected. Signed-off-by: Bhoumik Patidar <bpatidar@qti.qualcomm.com>
59a019e to
545cbd4
Compare
7 participants
Links and runs
SingleSouce(1891 tests),MultiSource(201 tests) andMicroBenchmarks(22 tests) subdirectories fromllvm/llvm-test-suiteforx86_64.ld.eldis fetched viaFetchNightlyToolset.No failures expected.