User Details
- User Since
- Apr 23 2020, 6:41 PM (178 w, 3 d)
Dec 22 2021
I am seeing a lot of failures on nvptx machine (sm_70, cuda11.4) with this patch,
libomptarget :: nvptx64-nvidia-cuda :: offloading/bug49021.cpp libomptarget :: nvptx64-nvidia-cuda :: offloading/bug49334.cpp libomptarget :: nvptx64-nvidia-cuda :: offloading/bug49779.cpp libomptarget :: nvptx64-nvidia-cuda :: offloading/bug51781.c libomptarget :: nvptx64-nvidia-cuda :: offloading/bug51982.c libomptarget :: nvptx64-nvidia-cuda :: unified_shared_memory/close_enter_exit.c libomptarget :: nvptx64-nvidia-cuda :: unified_shared_memory/close_modifier.c libomptarget :: nvptx64-nvidia-cuda :: unified_shared_memory/shared_update.c libomptarget :: nvptx64-nvidia-cuda-newRTL :: offloading/bug49021.cpp libomptarget :: nvptx64-nvidia-cuda-newRTL :: offloading/bug49334.cpp libomptarget :: nvptx64-nvidia-cuda-newRTL :: offloading/bug51781.c libomptarget :: nvptx64-nvidia-cuda-newRTL :: unified_shared_memory/close_enter_exit.c libomptarget :: nvptx64-nvidia-cuda-newRTL :: unified_shared_memory/close_modifier.c libomptarget :: nvptx64-nvidia-cuda-newRTL :: unified_shared_memory/shared_update.c
On amdgcn, these are the tests failing,
libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp libomptarget :: amdgcn-amd-amdhsa :: offloading/bug51781.c libomptarget :: amdgcn-amd-amdhsa :: offloading/bug51982.c libomptarget :: amdgcn-amd-amdhsa-newRTL :: offloading/bug49021.cpp libomptarget :: amdgcn-amd-amdhsa-newRTL :: offloading/bug51781.c
Dec 8 2021
Hi, this is again causing issues on amdgpu buildbot (https://lab.llvm.org/buildbot/#/builders/193). In the logs, I see that cmake was unable to find libelf dependency which is necessary for openmp plugins to be built and tested. In absence of libelf, the bot, currently, silently ignores and run non-amdgpu tests. I locally reverted this patch and found that tests are again working fine.
Dec 6 2021
LGTM
Dec 1 2021
Nov 17 2021
If its helpful, following is the cmake command I used (cmake version: 3.20)
cmake ../llvm -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD="X86;AMDGPU" -DLLVM_ENABLE_ASSERTIONS=On -DLLVM_ENABLE_RUNTIMES=openmp -DCLANG_DEFAULT_LINKER=lld -DLLVM_ENABLE_PROJECTS="lld;clang"
Hi, this patch is producing cmake errors in our amdgpu buildbot: https://lab.llvm.org/buildbot/#/builders/193. Initially I thought there was something wrong with our configuration, but git bisecting revealed that this commit is causing the issue. If I remove the following line, the build is fine.
project(Runtimes LANGUAGES NONE)
Oct 6 2021
Only removing optnone.
I don't have any concrete evidence but I have some doubt on presence of function pointers causing backend to behave improperly. Also, here removing optnone alone suffices to fix the issue.
I have created a patch (D111218) with fix for amdgcn. This is a temporary fix. I will still keep on looking into it until I find a real root cause.
Sep 30 2021
Abandoning this in favor of D110902 due to dependency issues.
I modified the declare_mapper_target to print the contents of array after target region and found the following output:
2 3 4 5 6 7 8 9 10 11 Sum = 65
Sep 29 2021
Apologies for late reply. Most of the tests now do not try to call malloc, so no page fault errors. But all of them are producing wrong results. For e.g. declare_mapper_target.cpp produces Sum = 132608 with the patch applied. Similarly for other tests as well. So don't know what's happening yet.
Sep 27 2021
- review comments
- rebase after rG9d0eb440ff40
- change return type of hsa_status_t
Sep 26 2021
Sep 22 2021
Rebase
I got this after changing __kmpc_impl_malloc to return 0xdeadbeef. So, this confirms that missing malloc implementation is the root cause.
Memory access fault by GPU node-4 (Agent handle: 0x1bc5000) on address 0xdeadb000. Reason: Page not present or supervisor privilege.
It looks like from IR diff that this patch is adding use of kmpc_alloc_shared method. These methods likely won't work on AMDGPU as device malloc is not available. Not sure what could be done apart from marking those tests as XFAIL on amdgcn. :(
Sep 20 2021
Looks good.
update logic for fine-grained/kernarg pools
Sep 19 2021
This is not dependent on D109875, instead it is the other case. Apologies for not mentioning here. Diff shown here is messed up.
Sep 17 2021
Check if both values are set properly.
Sep 16 2021
run clang-format
Split this patch into two,
- Current patch now only includes changes for simplifying the host pool lookup
- D109875 includes plain refactoring.
Sep 15 2021
Sep 14 2021
Even with declare variant separated using ifdef's, the error is still there. So I don't think we have workaround for this.
Sep 13 2021
Hey, Jon, sorry for late reply. I cannot reproduce this issue on nvptx so it seems to occur only on amdgcn. Will it be better if instead the name mangling issue is fixed? Or for the meantime, I could add #ifdef around as a temporary fix. Suggestions?
Sep 9 2021
Looks good! Thanks.
Sep 7 2021
Add amdgcn to the same arch list as nvptx
Sep 1 2021
LGTM
Aug 26 2021
Confirmed locally that this fixes the linking issue. It has fixed the linking issue on amdgcn as well. Thanks for working on this.
Aug 24 2021
Oh sorry, forgot about this. Landing it now.
Aug 23 2021
Aug 12 2021
Remove redundant test
Aug 10 2021
Rebase
Aug 4 2021
Aug 2 2021
@ye-luo and @JonChesterfield can you please test the latest version of this patch? It should work now.
Fixed compilation error for nvptx headers. Tested on both cuda and non-cuda systems.
Jul 30 2021
Addressed review comments.
It required some work to fix the failing lit test case. And many thanks to
@estewart for helping in that.
Jul 29 2021
Missed comment.
Rename method to getCommonDeviceLibNames
Jul 28 2021
Due to the current state of math headers, I was unable to test this patch without ockl. But last time when headers were working, I was actually required to link ockl for a symbol (I forgot the name). I will update once I am able to get the math headers work again.
Jul 27 2021
Extract the options from HIP/OpenMP to a common method in base class.
Jul 15 2021
I need to rebase this.
Jul 14 2021
Move linking logic to a common method.
Looks ok to me. Regression tests and runtime tests went fine. Tested a simple cuda and openmp kernel with sin function on sm_61, didn't see any issue.
Jul 1 2021
Should the name of file be changed as well?
Looks good! Thanks.
Jun 30 2021
Jun 28 2021
- Move constant to openmp_wrappers/cmath
- Using push/pop_macro to avoid redefinition
Looks good! Thanks
Typo
Addressed review comments.
Jun 25 2021
Fix format errors
Jun 23 2021
Addressed review comments.
Jun 22 2021
Fix clang-tidy errors and use scalar for host memory pool.