Skip to content

[openmp][test] The gtid.cpp test case fails when OpenMP has been built with -DLIBOMP_ENABLE_SHARED=False #113436

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
pawosm-arm opened this issue Oct 23, 2024 · 7 comments · Fixed by #113522
Assignees

Comments

@pawosm-arm
Copy link
Contributor

pawosm-arm commented Oct 23, 2024

With OpenMP built with the -DLIBOMP_ENABLE_SHARED=False CMake flag, the openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp test case is failing as the linker complains that the __kmp_hidden_helper_threads_num symbol is defined in both libomp.a and gtid.cpp. In gtid.cpp it is defined globally as such:

kmp_int32 __kmp_hidden_helper_threads_num;

There is one more test case which also defines kmp_int32 __kmp_hidden_helper_threads_num, it is the openmp/runtime/test/worksharing/for/kmp_sch_simd_guided.c test case and it defines it as such:

static int __kmp_hidden_helper_threads_num = 0;

Shouldn't it be static in the gtid.cpp test case too?

@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2024

@llvm/issue-subscribers-openmp

Author: Paul Osmialowski (pawosm-arm)

With OpenMP built with the `-DLIBOMP_ENABLE_SHARED=False` CMake flag, the `openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp` test case is failing as the linker complains that the `__kmp_hidden_helper_threads_num` symbol is defined in both `libomp.a` and `gtid.cpp`. In `gtid.cpp` it is defined globally as such:
kmp_int32 __kmp_hidden_helper_threads_num;

There is one more test case which also defines kmp_int32 __kmp_hidden_helper_threads_num, it is the openmp/runtime/test/worksharing/for/kmp_sch_simd_guided.c test case and it defines it as such:

static int __kmp_hidden_helper_threads_num = 0;

Shouldn't it be static in the gtid.cpp test case too?

@shiltian shiltian self-assigned this Oct 23, 2024
@shiltian
Copy link
Contributor

@TerryLWilmarth Do we really have to support building libomp as a static library?

@pawosm-arm
Copy link
Contributor Author

pawosm-arm commented Oct 23, 2024

I have a debugger test suite which builds test cases with and without -static, the only way to make it work is to have libomp.a.

@TerryLWilmarth
Copy link
Contributor

Hi Shilei,
Yeah, it's strongly discouraged, but there are still people who need it, so we continue to support it, but only on Linux these days. Here are a couple of cases:
https://discourse.llvm.org/t/building-a-static-llvm-openmp-library/39629
https://discourse.llvm.org/t/building-a-static-library-of-openmp/51606

@pawosm-arm
Copy link
Contributor Author

pawosm-arm commented Oct 23, 2024

A somewhat more general question, is this test case correct in defining this variable global without static?

@TerryLWilmarth
Copy link
Contributor

For this bug, the test should just rename that variable to something without the _kmp* prefix. The __kmp, __kmpc, kmp, pseudo-namespaces are meant for the Intel and LLVM OpenMP runtime.

@pawosm-arm
Copy link
Contributor Author

Thank you @shiltian

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

Successfully merging a pull request may close this issue.

5 participants