-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[OpenMP] Fix incorrect CUDA bc path after library change #157547
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
Conversation
@llvm/pr-subscribers-offload Author: Joseph Huber (jhuber6) ChangesFull diff: https://github.com/llvm/llvm-project/pull/157547.diff 1 Files Affected:
diff --git a/offload/test/lit.cfg b/offload/test/lit.cfg
index f3e8e9a66685e..a41bcb9c0d064 100644
--- a/offload/test/lit.cfg
+++ b/offload/test/lit.cfg
@@ -169,7 +169,7 @@ else: # Unices
if config.cuda_libdir:
config.test_flags += " -Wl,-rpath," + config.cuda_libdir
if config.libomptarget_current_target.startswith('nvptx'):
- config.test_flags_clang += " --libomptarget-nvptx-bc-path=" + config.llvm_library_intdir
+ config.test_flags_clang += " --libomptarget-nvptx-bc-path=" + config.llvm_library_intdir + "/nvptx64-nvidia-cuda"
if config.libomptarget_current_target.endswith('-LTO'):
config.test_flags += " -foffload-lto"
if config.libomptarget_current_target.endswith('-JIT-LTO') and evaluate_bool_env(
|
config.test_flags += " -Wl,-rpath," + config.cuda_libdir | ||
if config.libomptarget_current_target.startswith('nvptx'): | ||
config.test_flags_clang += " --libomptarget-nvptx-bc-path=" + config.llvm_library_intdir | ||
config.test_flags_clang += " --libomptarget-nvptx-bc-path=" + config.llvm_library_intdir + "/nvptx64-nvidia-cuda" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, config.llvm_library_intdir
is /home/jdenny/llvm/build/lib
. But the file that's needed is /home/jdenny/llvm/build/runtimes/runtimes-nvptx64-nvidia-cuda-bins/openmp/device/libomptarget-nvptx.bc
. So the above doesn't fix the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought at one point we set the intermediate directory for those bitcode files explicitly to config.llvm_library_intdir
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not getting installed there? It's supposed to be installed to build/lib/nvptx64-nvidia-cuda
but it's possible something's messed up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a quick fix, likely what we want is to inform the runtimes which targets are active or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ ls build/lib/nvptx64-nvidia-cuda/
crt1.o libc.a libm.a libompdevice.a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not installing before testing. But I thought you were pointing to a build directory anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to specify the output to the right place, should hopefully work now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A clean build (on my amd gpu system or nvidia gpu system) now fails with something like:
ninja: error: 'openmp/device/libomptarget-amdgpu.bc', needed by '/home/jdenny/llvm/build/lib/amdgcn-amd-amdhsa/libompdevice.a', missing and no known rule to make it
I have not pulled since this morning. I only applied your diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Thanks.
No description provided.