Skip to content

Avoid concurrency issues in LibompGetArchitecture #151313

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

Closed
wants to merge 2 commits into from

Conversation

Flamefire
Copy link
Contributor

Especially during make check-all -j <n> this CMake code might be run in parallel. So one process could remove the file another process has just written and tries to compile.

So do not delete the file.

Additionally use just try_compile as running is never intended nor possible anyway.

Note: Another solution could be to just extract the embedded file to a plain file in the same folder and use that as the source.

@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Jul 30, 2025
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

Especially during `make check-all -j <n>` this CMake code might be run in parallel.
So one process could remove the file another process has just written and tries to compile.

So do not delete the file.
Additionally use just `try_compile` as running is never intended nor possible anyway.
@jprotze
Copy link
Collaborator

jprotze commented Aug 5, 2025

Especially during make check-all -j <n> this CMake code might be run in parallel. So one process could remove the file another process has just written and tries to compile.

The code is executed at configure, not at build time. There is no concurrency for configure. What is the issue this patch tries to address?

@Flamefire
Copy link
Contributor Author

I've seen this being run during the make check-all invocation after trying to figure out why LIBOMP_ARCH ended up being empty failing the check at

libomp_check_variable(LIBOMP_ARCH 32e x86_64 32 i386 arm ppc ppc64 ppc64le aarch64 aarch64_32 aarch64_a64fx mic mips mips64 riscv64 loongarch64 ve s390x sparc sparcv9 wasm32)

CMake Error at /build/llvm-project-19.1.7.src/openmp/runtime/cmake/LibompUtils.cmake:26 (message):
  LIBOMP: libomp_check_variable(): LIBOMP_ARCH = is unknown
Call Stack (most recent call first):
  /build/llvm-project-19.1.7.src/openmp/runtime/cmake/LibompUtils.cmake:142 (libomp_error_say)
  /build/llvm-project-19.1.7.src/openmp/runtime/CMakeLists.txt:96 (libomp_check_variable)

Some digging through logs showed that the output was empty after an error occurred that said it couldn't find the file.

Our workaround was to use make -j1 check-all which increases the build/test time but is reliable.

It is hard to reproduce now consistently and to tell where exactly it tries to do that concurrently.

The only indication I have is this output before it gets to the libomp_get_architecture:

Performing configure step for 'runtimes'

This looks like ExternalProject (likely https://github.com/llvm/llvm-project/blob/6897ca460e6e28bcf76ae941438dd1313426e0bb/llvm/runtimes/CMakeLists.txt#L263C28-L263C36) which gets added as a test dependency at

set_property(GLOBAL APPEND PROPERTY LLVM_ALL_ADDITIONAL_TEST_TARGETS runtimes ${extra_deps})

I'm not sure this change would fully resolve this as it likely reuses the same binary directory which will make it fail in similar ways just while writing the temporary compile results.
As LIBOMP* variables are passed through to runtime builds maybe it is enough to set LIBOMP_ARCH. Or pass LLVM_TARGET_ARCH and LLVM_HOST_TRIPLE to the runtimes configure. If those are set at that point at least.

I extracted the other changes to #152153 as they are independent of this issue.

@Flamefire
Copy link
Contributor Author

I found the issue: Until CMake 3.19 there was a "bug" with ExternalProject that then got fixed, guarded by policy CMP0114 which was only enabled for LLVM 20 with 49de154

So in LLVM 20+ using CMake 3.19+ avoids the issue.

@Flamefire Flamefire closed this Aug 7, 2025
@Flamefire Flamefire deleted the patch-3 branch August 7, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants