This adds testing LibC's GPU mode to the AMDGPU (experimental) buildbot.
The change requires the buildbot to also depend on the libc project and
pass-in a few more CMake flags.
Finally, it runs the additional check-libc target.
Details
Diff Detail
- Repository
- rZORG LLVM Github Zorg
- Build Status
Buildable 222669 Build 342010: arc lint + arc unit
Event Timeline
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1888 | Actually I never implemented that for libc, I probably should go ahead and do that. But I think for the buildbot it will help to be explicit, so if the GPU dies we don't silently start passing tests. theres a LIBC_GPU_TEST_ARCHITECTURE variable for that purpose. |
I wanted to do this today, my afternoon. This should give us some time to reasonably react if things don't go the way we want.
Could you elaborate on why did you specify LLVM_ENABLE_PROJECTS explicitly, and why it does not match what's in the depends_on_projects, please? The same question is for LLVM_ENABLE_RUNTIMES and enable_runtimes respectively.
With this patch the openmp-offload-amdgpu-runtime-experimental builder would not listen for compiler_rt changes, but would build that. Meaning false blame lists every time when a compiler_rt commit breaks a build. This does not look right. What am I missing?
Thanks for the comment and explanation. You're not missing anything, it was a mistake on my end. I'll fix it and update the patch.
Addressed feedback: removed extra CMake flags to enable runtimes and projects manually. Instead rely on enable_runtimes and depends_on_projects lists from the buildbot factory.
The builtbot before specified the CMake flag to enable the openmp runtime manually (despite listing it in enable_runtimes), so I removed that completely.
If this is the preferred way to do it, we should change the AMDGPU-openmp buildbot (non-experimental) and also remove the manual CMake flag there.
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1877 | Unless you need to handle a special case, you do not need to specify enable_runtimes. Just make sure you list all the dependencies in depends_on_projects (which seems you are), no matter if they are "projects" or "runtimes", and let the default machinery do the trick. |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1877 | Will the default machinery put it in the right place? The libc project can go in both as far as I'm aware. |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1877 | I added these as enable_runtimes as the OpenMPBuilder will set the enable_runtimes=['openmp'] when no argument is given. Should we then also change the OpenMPBuilder or is this part of the default machinery you mentioned? |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1877 | As far as I can see OpenMPBuilder does the right thing. It passes through both depends_on_projects and enable_runtimes to LLVMBuildFactory and then uses the factory properties to set cmake params. Please see https://github.com/llvm/llvm-zorg/blob/91e9b7954ae867c3294b896e44fdc7ad498840b5/zorg/buildbot/builders/OpenMPBuilder.py#L75 and below for more details.
I do not think it would. Where do you see that in the code? |
Please let me know if this patch needs more revision after the changes I implemented, and thanks for all the help.
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
1877 | My bad, you are totally right. I must have confused the enable_projects parts in the OpenMPBuilder. |
It seems that the automation does not correctly enable libc or openmp as runtimes when only given as depend_on_projects.
The CMake invocation that actually happened was
cmake -G Ninja ../llvm.src/llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON '-DLLVM_LIT_ARGS=-vv --show-unsupported --show-xfail -j 32' -DCMAKE_INSTALL_PREFIX=/home/ompworker/bbot/openmp-offload-amdgpu-runtime-experimental/llvm.inst -DCMAKE_BUILD_TYPE=Release -DCLANG_DEFAULT_LINKER=lld '-DLLVM_TARGETS_TO_BUILD=X86;AMDGPU' -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DLIBOMPTARGET_FOUND_AMDGPU_GPU=ON -DLLVM_LIBC_FULL_BUILD=ON -DLIBC_GPU_BUILD=ON -DLIBC_GPU_ARCHITECTURE=gfx906 -DLIBC_GPU_TEST_ARCHITECTURE=gfx906 '-DLLVM_ENABLE_PROJECTS=llvm;libc;clang;lld;compiler-rt;openmp'
So, it only enables those as projects. This leads to a compiler error as we actually need the Clang to build the runtimes.
Is this an error in the automation machinery that we should fix, or shall we go back to enabling those as enable_runtimes?
OpenMPBuilder didn't do enable_runtimes="auto". This is fixed in https://github.com/llvm/llvm-zorg/commit/b86d771caf03cb7fe94d0d187cad25d9519ea584.
Could you commit this patch again, please? And do not revert even if something wouldn't work as expected. Since we are all looking at this thing, it would be a bit easier with less moving parts.
Unless you need to handle a special case, you do not need to specify enable_runtimes. Just make sure you list all the dependencies in depends_on_projects (which seems you are), no matter if they are "projects" or "runtimes", and let the default machinery do the trick.