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.

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

Bug in Libomptarget when state initialization comes from outside the translation unit #54208

Closed
jhuber6 opened this issue Mar 4, 2022 · 7 comments
Assignees
Labels

Comments

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 4, 2022

Libomptarget uses some shared variables to track certain internal stated in the runtime. This causes problems when we have code that contains no OpenMP kernels. These variables are normally initialized upon kernel entry, but if there are no kernels we will see no initialization. Currently we load the runtime into each source file when not running in LTO mode, so these variables will be erroneously considered undefined or dead and removed, causing miscompiles.

@jhuber6 jhuber6 added the openmp label Mar 4, 2022
@jhuber6 jhuber6 added this to the LLVM 14.0.0 Release milestone Mar 4, 2022
@jhuber6 jhuber6 self-assigned this Mar 4, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2022

@llvm/issue-subscribers-openmp

@jhuber6 jhuber6 closed this as completed in e2dcc22 Mar 4, 2022
@jhuber6
Copy link
Contributor Author

jhuber6 commented Mar 4, 2022

/cherry-pick e2dcc22

@EugeneZelenko
Copy link
Contributor

For backport.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2022

/branch llvmbot/llvm-project/issue54208

llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Mar 4, 2022
Libomptarget uses some shared variables to track certain internal stated
in the runtime. This causes problems when we have code that contains no
OpenMP kernels. These variables are normally initialized upon kernel
entry, but if there are no kernels we will see no initialization.
Currently we load the runtime into each source file when not running in
LTO mode, so these variables will be erroneously considered undefined or
dead and removed, causing miscompiles. This patch temporarily works
around the most obvious case, but others still exhibit this problem. We
will need to fix this more soundly later.

Fixes llvm#54208.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D121007

(cherry picked from commit e2dcc22)
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2022

/pull-request llvmbot#119

@tstellar
Copy link
Collaborator

tstellar commented Mar 5, 2022

@jdoerfert What do you think about backporting this? e2dcc22

@jdoerfert
Copy link
Member

I'm torn. I initially thought we should but I'm not so sure anymore. I fear the performance impact on a lot of codes is not worth it for the few this might help (to actually run). For the next version we'll have to have a proper solution. Let's keep 14 as is.

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

No branches or pull requests

5 participants