[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
This revision was automatically updated to reflect the committed changes. Closed by commit rC343230: [OpenMP] Improve search for libomptarget-nvptx (authored by Hahnfeld, committed by ). Repository: rC Clang https://reviews.llvm.org/D51686 Files: include/clang/Driver/Options.td lib/Driver/ToolChains/Cuda.cpp test/Driver/openmp-offload-gpu.c Index: test/Driver/openmp-offload-gpu.c === --- test/Driver/openmp-offload-gpu.c +++ test/Driver/openmp-offload-gpu.c @@ -30,6 +30,22 @@ /// ### +/// Check that -lomptarget-nvptx is passed to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-NVLINK %s +/// Check that the value of --libomptarget-nvptx-path is forwarded to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: --libomptarget-nvptx-path=/path/to/libomptarget/ \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-NVLINK,CHK-LIBOMPTARGET-NVPTX-PATH %s + +// CHK-NVLINK: nvlink +// CHK-LIBOMPTARGET-NVPTX-PATH-SAME: "-L/path/to/libomptarget/" +// CHK-NVLINK-SAME: "-lomptarget-nvptx" + +/// ### + /// Check cubin file generation and usage by nvlink // RUN: %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \ // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -save-temps %s 2>&1 \ @@ -151,6 +167,11 @@ // RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ // RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-BCLIB %s +/// The user can override default detection using --libomptarget-nvptx-path=. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda --libomptarget-nvptx-path=%S/Inputs/libomptarget \ +// RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ +// RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-BCLIB %s // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc // CHK-BCLIB-NOT: {{error:|warning:}} Index: lib/Driver/ToolChains/Cuda.cpp === --- lib/Driver/ToolChains/Cuda.cpp +++ lib/Driver/ToolChains/Cuda.cpp @@ -511,6 +511,11 @@ CmdArgs.push_back("-arch"); CmdArgs.push_back(Args.MakeArgString(GPUArch)); + // Assume that the directory specified with --libomptarget_nvptx_path + // contains the static library libomptarget-nvptx.a. + if (const Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) +CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue())); + // Add paths specified in LIBRARY_PATH environment variable as -L options. addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH"); @@ -647,12 +652,9 @@ if (DeviceOffloadingKind == Action::OFK_OpenMP) { SmallVector LibraryPaths; -// Add path to lib and/or lib64 folders. -SmallString<256> DefaultLibPath = - llvm::sys::path::parent_path(getDriver().Dir); -llvm::sys::path::append(DefaultLibPath, -Twine("lib") + CLANG_LIBDIR_SUFFIX); -LibraryPaths.emplace_back(DefaultLibPath.c_str()); + +if (const Arg *A = DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) + LibraryPaths.push_back(A->getValue()); // Add user defined library paths from LIBRARY_PATH. llvm::Optional LibPath = @@ -665,6 +667,12 @@ LibraryPaths.emplace_back(Path.trim()); } +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = +llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX); +LibraryPaths.emplace_back(DefaultLibPath.c_str()); + std::string LibOmpTargetName = "libomptarget-nvptx-" + GpuArch.str() + ".bc"; bool FoundBCLibrary = false; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -596,6 +596,8 @@ HelpText<"HIP device library">; def fhip_dump_offload_linker_script : Flag<["-"], "fhip-dump-offload-linker-script">, Group, Flags<[NoArgumentUnused, HelpHidden]>; +def libomptarget_nvptx_path_EQ : Joined<["--"], "libomptarget-nvptx-path=">, Group, + HelpText<"Path to libomptarget-nvptx libraries">; def dA : Flag<["-"], "dA">, Group; def dD : Flag<["-"], "dD">, Group, Flags<[CC1Option]>, HelpText<"Print macro definitions in -E mode in addition to normal
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:665 +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = Hahnfeld wrote: > ABataev wrote: > > You're changing the order of the lookup for the paths. Is this intended? If > > so, you need the test for this. > Yes, that's explained in the summary. > > I'll try to see if there is a way to test this, even if we can't expect > libomptarget-nvptx to be built together with Clang. At the moment I think > only `LIBRARY_PATH` is tested, I'm adding some for the new > `--libomptarget-nvptx-path`. After some thoughts it seems impossible to reliably test this code path because the user can set `CLANG_LIBDIR_SUFFIX`. https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld updated this revision to Diff 167250. Hahnfeld marked 2 inline comments as done. Hahnfeld added a comment. Add `const` per review comments. https://reviews.llvm.org/D51686 Files: include/clang/Driver/Options.td lib/Driver/ToolChains/Cuda.cpp test/Driver/openmp-offload-gpu.c Index: test/Driver/openmp-offload-gpu.c === --- test/Driver/openmp-offload-gpu.c +++ test/Driver/openmp-offload-gpu.c @@ -30,6 +30,22 @@ /// ### +/// Check that -lomptarget-nvptx is passed to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-NVLINK %s +/// Check that the value of --libomptarget-nvptx-path is forwarded to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: --libomptarget-nvptx-path=/path/to/libomptarget/ \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-NVLINK,CHK-LIBOMPTARGET-NVPTX-PATH %s + +// CHK-NVLINK: nvlink +// CHK-LIBOMPTARGET-NVPTX-PATH-SAME: "-L/path/to/libomptarget/" +// CHK-NVLINK-SAME: "-lomptarget-nvptx" + +/// ### + /// Check cubin file generation and usage by nvlink // RUN: %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \ // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -save-temps %s 2>&1 \ @@ -151,6 +167,11 @@ // RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ // RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-BCLIB %s +/// The user can override default detection using --libomptarget-nvptx-path=. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda --libomptarget-nvptx-path=%S/Inputs/libomptarget \ +// RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ +// RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-BCLIB %s // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc // CHK-BCLIB-NOT: {{error:|warning:}} Index: lib/Driver/ToolChains/Cuda.cpp === --- lib/Driver/ToolChains/Cuda.cpp +++ lib/Driver/ToolChains/Cuda.cpp @@ -511,6 +511,11 @@ CmdArgs.push_back("-arch"); CmdArgs.push_back(Args.MakeArgString(GPUArch)); + // Assume that the directory specified with --libomptarget_nvptx_path + // contains the static library libomptarget-nvptx.a. + if (const Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) +CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue())); + // Add paths specified in LIBRARY_PATH environment variable as -L options. addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH"); @@ -647,12 +652,9 @@ if (DeviceOffloadingKind == Action::OFK_OpenMP) { SmallVector LibraryPaths; -// Add path to lib and/or lib64 folders. -SmallString<256> DefaultLibPath = - llvm::sys::path::parent_path(getDriver().Dir); -llvm::sys::path::append(DefaultLibPath, -Twine("lib") + CLANG_LIBDIR_SUFFIX); -LibraryPaths.emplace_back(DefaultLibPath.c_str()); + +if (const Arg *A = DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) + LibraryPaths.push_back(A->getValue()); // Add user defined library paths from LIBRARY_PATH. llvm::Optional LibPath = @@ -665,6 +667,12 @@ LibraryPaths.emplace_back(Path.trim()); } +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = +llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX); +LibraryPaths.emplace_back(DefaultLibPath.c_str()); + std::string LibOmpTargetName = "libomptarget-nvptx-" + GpuArch.str() + ".bc"; bool FoundBCLibrary = false; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -596,6 +596,8 @@ HelpText<"HIP device library">; def fhip_dump_offload_linker_script : Flag<["-"], "fhip-dump-offload-linker-script">, Group, Flags<[NoArgumentUnused, HelpHidden]>; +def libomptarget_nvptx_path_EQ : Joined<["--"], "libomptarget-nvptx-path=">, Group, + HelpText<"Path to libomptarget-nvptx libraries">; def dA : Flag<["-"], "dA">, Group; def dD : Flag<["-"], "dD">, Group, Flags<[CC1Option]>, HelpText<"Print macro definitions in -E mode in addition to normal output">; Index: test/Driver/openmp-offload-gpu.c
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:665 +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = ABataev wrote: > You're changing the order of the lookup for the paths. Is this intended? If > so, you need the test for this. Yes, that's explained in the summary. I'll try to see if there is a way to test this, even if we can't expect libomptarget-nvptx to be built together with Clang. At the moment I think only `LIBRARY_PATH` is tested, I'm adding some for the new `--libomptarget-nvptx-path`. Repository: rC Clang https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
ABataev added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:515 + if (Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) +CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue())); + `const Arg *A` Comment at: lib/Driver/ToolChains/Cuda.cpp:651 + +if (Arg *A = DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) + LibraryPaths.push_back(A->getValue()); `const Arg *` Comment at: lib/Driver/ToolChains/Cuda.cpp:665 +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = You're changing the order of the lookup for the paths. Is this intended? If so, you need the test for this. Repository: rC Clang https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D51686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld created this revision. Hahnfeld added reviewers: gtbercea, ABataev. Herald added subscribers: cfe-commits, guansong. When looking for the bclib Clang considered the default library path first while it preferred directories in LIBRARY_PATH when constructing the invocation of nvlink. The latter actually makes more sense because during development it allows using a non-default runtime library. So change the search for the bclib to start looking in directories given by LIBRARY_PATH. Additionally add a new option --libomptarget-nvptx-path= which will be searched first. This will be handy for testing purposes. Repository: rC Clang https://reviews.llvm.org/D51686 Files: include/clang/Driver/Options.td lib/Driver/ToolChains/Cuda.cpp test/Driver/openmp-offload-gpu.c Index: test/Driver/openmp-offload-gpu.c === --- test/Driver/openmp-offload-gpu.c +++ test/Driver/openmp-offload-gpu.c @@ -30,6 +30,22 @@ /// ### +/// Check that -lomptarget-nvptx is passed to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-NVLINK %s +/// Check that the value of --libomptarget-nvptx-path is forwarded to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: --libomptarget-nvptx-path=/path/to/libomptarget/ \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-NVLINK,CHK-LIBOMPTARGET-NVPTX-PATH %s + +// CHK-NVLINK: nvlink +// CHK-LIBOMPTARGET-NVPTX-PATH-SAME: "-L/path/to/libomptarget/" +// CHK-NVLINK-SAME: "-lomptarget-nvptx" + +/// ### + /// Check cubin file generation and usage by nvlink // RUN: %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \ // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -save-temps %s 2>&1 \ @@ -151,6 +167,11 @@ // RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ // RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-BCLIB %s +/// The user can override default detection using --libomptarget-nvptx-path=. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda --libomptarget-nvptx-path=%S/Inputs/libomptarget \ +// RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ +// RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-BCLIB %s // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc // CHK-BCLIB-NOT: {{error:|warning:}} Index: lib/Driver/ToolChains/Cuda.cpp === --- lib/Driver/ToolChains/Cuda.cpp +++ lib/Driver/ToolChains/Cuda.cpp @@ -509,6 +509,11 @@ CmdArgs.push_back("-arch"); CmdArgs.push_back(Args.MakeArgString(GPUArch)); + // Assume that the directory specified with --libomptarget_nvptx_path + // contains the static library libomptarget-nvptx.a. + if (Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) +CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue())); + // Add paths specified in LIBRARY_PATH environment variable as -L options. addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH"); @@ -642,12 +647,9 @@ if (DeviceOffloadingKind == Action::OFK_OpenMP) { SmallVector LibraryPaths; -// Add path to lib and/or lib64 folders. -SmallString<256> DefaultLibPath = - llvm::sys::path::parent_path(getDriver().Dir); -llvm::sys::path::append(DefaultLibPath, -Twine("lib") + CLANG_LIBDIR_SUFFIX); -LibraryPaths.emplace_back(DefaultLibPath.c_str()); + +if (Arg *A = DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) + LibraryPaths.push_back(A->getValue()); // Add user defined library paths from LIBRARY_PATH. llvm::Optional LibPath = @@ -660,6 +662,12 @@ LibraryPaths.emplace_back(Path.trim()); } +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = +llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX); +LibraryPaths.emplace_back(DefaultLibPath.c_str()); + std::string LibOmpTargetName = "libomptarget-nvptx-" + GpuArch.str() + ".bc"; bool FoundBCLibrary = false; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -596,6 +596,8 @@ HelpText<"HIP device library">; def