[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D43197#1027139, @gtbercea wrote:

> In https://reviews.llvm.org/D43197#1011256, @Hahnfeld wrote:
>
> > Looking more closely at the patch, this doesn't seem to look into the `lib` / `lib64` next to the compiler. I'm not sure if `LIBRARY_PATH` is set for every installation, so I think we should add this one to catch the obvious case. This probably needs some attention for the tests because they'll find the just-built libraries...
>
>
> The contract with the user us that the .bc lib needs to be in LIBRARY_PATH, this is what we require today. Inlining of the runtime is not essential for correctness.


And I don't think this is clever. The compiler should always try to automatically find the files it needs and looking in the lib directory next to the compiler installation isn't hard in that case and a guess that will meet 99% of deployments.



================
Comment at: lib/Driver/ToolChains/Cuda.cpp:536-542
+      StringRef CompilerPath = env;
+      while (!CompilerPath.empty()) {
+        std::pair<StringRef, StringRef> Split =
+            CompilerPath.split(llvm::sys::EnvPathSeparator);
+        LibraryPaths.push_back(Split.first);
+        CompilerPath = Split.second;
+      }
----------------
gtbercea wrote:
> Hahnfeld wrote:
> > `tools::addDirectoryList` uses `StringRef::find`, I'm not sure if `StringRef::split` creates real copies of the string...
> What is your suggestion?
IMO you should use whatever existing code does, in that case `StringRef::find`.


Repository:
  rC Clang

https://reviews.llvm.org/D43197