[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added inline comments. Comment at: clang/include/clang/Driver/Options.td:948 def libomptarget_nvptx_bc_path_EQ : Joined<["--"], "libomptarget-nvptx-bc-path=">, Group, HelpText<"Path to libomptarget-nvptx bitcode library">; def dD : Flag<["-"], "dD">, Group,

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

jdoerfert reopened this revision. jdoerfert added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:948 def libomptarget_nvptx_bc_path_EQ : Joined<["--"], "libomptarget-nvptx-bc-path=">, Group, HelpText<"Path

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG79401b43ce4e: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode (authored by Pushpinder Singh , committe

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

tianshilei1992 accepted this revision. tianshilei1992 added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 _

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. Parameter to err_drv_omp_offload_target_missingbcruntime is sensible. @tianshilei1992? With this we can set the path for nvptx and amdgcn independently. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ htt

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322638. pdhaliwal marked an inline comment as done. pdhaliwal added a comment. I haave removed libomptarget-device-bc-path and have added amdgcn one. For diagnostic, instead of having one per architecture, I have used the same and added second parameter to

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. Using one option for both targets seems great - if both have put the devicertl in the same folder. Which I suppose they might not have. Maybe keep it separate for one, one for nvptx and one for amdgcn, and hope for a common 'device' later. Repository: rG LLV

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

tianshilei1992 requested changes to this revision. tianshilei1992 added a comment. This revision now requires changes to proceed. In D96248#2551503 , @JonChesterfield wrote: > @tianshilei1992 @jdoerfert can we agree on 'libomptarget-device-bc-path' > bei

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. Some background discussion about the diagnostic. This change means people using nvptx, where the library cannot be found, will now be advised to use libomptarget-device-bc-path instead of libomptarget-nvptx-bc-path. If they use libomptarget-nvptx-bc-path anyway,

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

tianshilei1992 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:193 CC1Args.push_back("-emit-llvm-bc"); + + std::string BitcodeSuffix = "amdgcn-" + GpuArch.str(); JonChesterfield wrote: > tianshilei1992 wrote: > > JonChesterfiel

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:193 CC1Args.push_back("-emit-llvm-bc"); + + std::string BitcodeSuffix = "amdgcn-" + GpuArch.str(); tianshilei1992 wrote: > JonChesterfield wrote: > > Need `if (Dri

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322298. pdhaliwal added a comment. - Added check for nogpulib - Fixed diagnostic message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 Files: clang/include/clang/

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

tianshilei1992 added a comment. Generally LGTM. In D96248#2549339 , @JonChesterfield wrote: > The existing search logic looks in clang's lib and LIBRARY_PATH, I think we > should probably look in the runtime directory as well for running from the > bui

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:193 CC1Args.push_back("-emit-llvm-bc"); + + std::string BitcodeSuffix = "amdgcn-" + GpuArch.str(); Need `if (DriverArgs.hasArg(options::OPT_nogpulib)) return;` her

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. Looks like an existing problem, but for building openmp via runtimes, with this patch applied this doesn't look in the right place and errors: 'error: No library 'libomptarget-amdgcn-gfx906.bc' found in the default clang lib directory or in LIBRARY_PATH. Please

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. LGTM. Let's wait for someone using nvptx to sanity check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 ___ cfe-commits mailing

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322090. pdhaliwal added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 Files: clang/include/clang/Driver/Options.td clang/l

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

JonChesterfield added a comment. I like this. Using the same logic, in the same function call, to find this library on either gpu is the right thing to do. Looks like a non functional change on nvptx, though phab doesn't make that obvious. Comment at: clang/include/clang/Driv

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322052. pdhaliwal added a comment. Accidently missed some changes, - Fix openmp-offload.c test failure - Fix amdgpu-openmp-toolchain.c test failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ ht

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb, saiislam, ABataev. Herald added subscribers: dang, kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. Herald added a reviewer: jansvoboda11. pdhaliwal requested review of t