[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield planned changes to this revision.
JonChesterfield added a comment.

Yeah, we probably can't leave the installed program with a search path only 
used for testing. I'm planning to revisit this to use 
libomptarget_amdgcn_bc_path instead of LIBRARY_PATH for testing, then drop the 
LIBRARY_PATH override.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

It seems that this path is baked in to clang executable even after make 
install. I'm not convinced this is the right direction.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

At present libomptarget_amdgcn_bc_path and nvptx need to be a path to a file. 
If we relax that to accept a path to a directory in which the corresponding 
file is found, then we can use that argument in place of LIBRARY_PATH from the 
test scripts. That will let the tests run, gives developers absolute control 
over which deviceRTL library is linked in, stops clang from picking up the 
wrong file from an unrelated toolchain on LIBRARY_PATH. I consider that 
strictly better than the above patch. Thoughts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I'm not so sure about that. With the other stuff in flight, it would let us 
build openmp apps from the clang build directory. That's useful for quicker 
build/debug cycles, as well as for lit tests that can be run by copying into a 
shell. At zero runtime cost when the library is found elsewhere, and trivial 
extra code in the driver.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Unlike others, such as `/usr/local/lib` or `/usr/local/cuda`, it is not a 
standard place. It doesn't make too much sense to hardcode it in the driver.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

More compelling than the testing case is that openmp is presently unusable 
without setting ld_library_path. This is part of fixing that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 requested changes to this revision.
tianshilei1992 added a comment.
This revision now requires changes to proceed.

From my perspective, this is not a good direction. No need to bother Clang 
driver for one case of testing. `LIBRARY_PATH` needs to be set correctly by lit 
instead of hardcoded the logic in the driver.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I think the lit test suite is intended to set environment variables such that 
this is not necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101935/new/

https://reviews.llvm.org/D101935

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101935: [clang] Search runtimes build tree for openmp runtime

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield created this revision.
JonChesterfield added reviewers: jdoerfert, pdhaliwal, ronlieb, grokos, 
tianshilei1992.
Herald added subscribers: guansong, yaxunl.
JonChesterfield requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

[clang] Search runtimes build tree for openmp runtime

D96248  added support for finding the openmp 
devicertl bitcode in the clang
lib directory where it is installed, or in the LIBRARY_PATH environment.

When testing openmp from a build using ENABLE_RUNTIMES, the corresponding
bitcode may be found under runtimes/runtimes-bins. This patch adds that to
the list of places clang looks, at lower priority then the others.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101935

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1686,6 +1686,12 @@
   llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
   LibraryPaths.emplace_back(DefaultLibPath.c_str());
 
+  // Add path to runtimes binary folder, used by ENABLE_RUNTIMES build
+  SmallString<256> RuntimesBinPath = llvm::sys::path::parent_path(D.Dir);
+  llvm::sys::path::append(RuntimesBinPath,
+  "runtimes/runtimes-bins/openmp/libomptarget");
+  LibraryPaths.emplace_back(RuntimesBinPath.c_str());
+
   OptSpecifier LibomptargetBCPathOpt =
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgcn_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1686,6 +1686,12 @@
   llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
   LibraryPaths.emplace_back(DefaultLibPath.c_str());
 
+  // Add path to runtimes binary folder, used by ENABLE_RUNTIMES build
+  SmallString<256> RuntimesBinPath = llvm::sys::path::parent_path(D.Dir);
+  llvm::sys::path::append(RuntimesBinPath,
+  "runtimes/runtimes-bins/openmp/libomptarget");
+  LibraryPaths.emplace_back(RuntimesBinPath.c_str());
+
   OptSpecifier LibomptargetBCPathOpt =
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgcn_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits