[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

thakis added a comment. Bot's happy again. Thanks for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128914/new/ https://reviews.llvm.org/D128914 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3644022 , @thakis wrote: > This breaks check-clang on mac: http://45.33.8.238/macm1/39907/step_7.txt > > Please take a look and revert for now if it takes a while to fix. Let me know if rGfe6a391357fc

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3644022 , @thakis wrote: > This breaks check-clang on mac: http://45.33.8.238/macm1/39907/step_7.txt > > Please take a look and revert for now if it takes a while to fix. I changed some of the argument formats in a pre

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

thakis added a comment. This breaks check-clang on mac: http://45.33.8.238/macm1/39907/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128914/new/ https://reviews.llvm.org/

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3643802 , @tra wrote: > For what it's worth, NCCL is the only > nontrivial library that needs RDC compilation that I'm aware of. > It's also self-contained for RDC purposes we only n

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

tra added a comment. In D128914#3643495 , @jhuber6 wrote: > Yes, it's actually pretty difficult to find a CUDA application using > `fgpu-rdc`. It seems much more common to just stick everything that's needed > in the file.I've considered finding a CUDA

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3643451 , @yaxunl wrote: > If you only unregister fatbin once for the whole program, then it should be > safe -fgpu-rdc. I am not sure if that is the case. it should be here, the generated handle is private to the reg

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

yaxunl added a comment. In D128914#3643270 , @jhuber6 wrote: >> There is only one fatbin for -fgpu-rdc mode but the fatbin unregister >> function is called multiple times in each TU. HIP runtime expects each >> fatbin is unregistered only once. The old

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce091eb3b91f: [HIP] Add support for handling HIP in the linker wrapper (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D1

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3642869 , @yaxunl wrote: > In D128914#3642567 , @jhuber6 wrote: > >> In D128914#3642558 , >> @JonChesterfield wrote: >> >>> Code looks

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

yaxunl added a comment. In D128914#3642567 , @jhuber6 wrote: > In D128914#3642558 , > @JonChesterfield wrote: > >> Code looks good to me. It's hard to be sure whether it works without running >> a bunch of hip t

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. In D128914#3642558 , @JonChesterfield wrote: > Code looks good to me. It's hard to be sure whether it works without running > a bunch of hip test cases through it, have you already done so? If it doesn't > work out of the box i

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Code looks good to me. It's hard to be sure whether it works without running a bunch of hip test cases through it, have you already done so? If it doesn't work out of the box

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128914/new/ https://reviews.llvm.org/D128914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 updated this revision to Diff 442356. jhuber6 added a comment. Addressing some comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128914/new/ https://reviews.llvm.org/D128914 Files: clang/test/Driver/linker-wrapper-image.c clang/t

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 added a comment. Thanks for the comments. Comment at: clang/test/Driver/linker-wrapper.c:109 // RUN: clang-offload-packager -o %t-lib.out \ // RUN: --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \ tra wrote:

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

tra added a comment. Syntax/style looks OK to me with a few nits. Comment at: clang/test/Driver/linker-wrapper.c:109 // RUN: clang-offload-packager -o %t-lib.out \ // RUN: --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \ ---

[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, yaxunl, tra. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. This patch adds the necessary changes required to