[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 326022.
zero9178 added a comment.

Undid refactoring of getCompilerRTBasename. Previous diffs introduced a new internal function that built the basename. In that version getCompilerRTBasename simply called getCompilerRT and extracted the filename component.

This lead to issues as Toolchains such as Baremetal override getCompilerRTBasename to provide a different basename then default.

This diff now instead changes the default implementation of getCompilerRTBasename to also auto detect whether an architecture suffix should be used or not. Therefore callers of getCompilerRTBasename can rely on it picking the right filenames as well. getCompilerRT still calls into getCompilerRTBasename to get the filename, allowing Toolchains to override the virtual function.

There is now sadly a bit of repetition as both getCompilerRT and getCompilerRTBasename have to look through the library path but this should not cause any issues I believe.

Sorry for the inconvenience and the test failure.


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

https://reviews.llvm.org/D96638

Files:
  clang/lib/Driver/ToolChain.cpp


Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -22,6 +22,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/XRayArgs.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
@@ -413,12 +414,21 @@
   return std::string(Path.str());
 }
 
-static std::string buildCompilerRTBasename(const ToolChain &toolchain,
-                                           const ArgList &Args,
-                                           StringRef Component,
-                                           ToolChain::FileType Type,
-                                           bool AddArch) {
-  const llvm::Triple &TT = toolchain.getTriple();
+static Optional<std::string> findInLibraryPath(const ToolChain &TC,
+                                               StringRef filename) {
+  for (const auto &LibPath : TC.getLibraryPaths()) {
+    SmallString<128> P(LibPath);
+    llvm::sys::path::append(P, filename);
+    if (TC.getVFS().exists(P))
+      return std::string(P.str());
+  }
+  return llvm::None;
+}
+
+std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
+                                             StringRef Component,
+                                             FileType Type) const {
+  const llvm::Triple &TT = getTriple();
   bool IsITANMSVCWindows =
       TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
@@ -433,44 +443,35 @@
     Suffix = IsITANMSVCWindows ? ".lib" : ".a";
     break;
   case ToolChain::FT_Shared:
-    Suffix = TT.isOSWindows()
-                 ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+    Suffix = Triple.isOSWindows()
+                 ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
                  : ".so";
     break;
   }
 
-  std::string ArchAndEnv;
-  if (AddArch) {
-    StringRef Arch = getArchNameForCompilerRTLib(toolchain, Args);
-    const char *Env = TT.isAndroid() ? "-android" : "";
-    ArchAndEnv = ("-" + Arch + Env).str();
+  std::string CRTWithoutArch =
+      (Prefix + Twine("clang_rt.") + Component + Suffix).str();
+  if (findInLibraryPath(*this, CRTWithoutArch).hasValue()) {
+    return CRTWithoutArch;
   }
-  return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
-}
 
-std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
-                                             StringRef Component,
-                                             FileType Type) const {
-  std::string absolutePath = getCompilerRT(Args, Component, Type);
-  return llvm::sys::path::filename(absolutePath).str();
+  StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
+  const char *Env = TT.isAndroid() ? "-android" : "";
+  std::string ArchAndEnv = ("-" + Arch + Env).str();
+
+  return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
                                      FileType Type) const {
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-      buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto &LibPath : getLibraryPaths()) {
-    SmallString<128> P(LibPath);
-    llvm::sys::path::append(P, CRTBasename);
-    if (getVFS().exists(P))
-      return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+    return value.getValue();
   }
 
   // Fall back to the old expected compiler-rt name if the new one does not
   // exist.
-  CRTBasename =
-      buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/true);
   SmallString<128> Path(getCompilerRTPath());
   llvm::sys::path::append(Path, CRTBasename);
   return std::string(Path.str());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96638.326022.patch
Type: text/x-patch
Size: 4068 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210224/5189bada/attachment-0001.bin>