STL_MSFT created this revision.

This is a minimally intrusive change, which I've verified works for both x86 
and x64.

The idea is to add another bool data member, `IsDevDivInternal`. (This could be 
unified with `IsVS2017OrNewer`, since it's a 3-way setting. Either a build is 
DevDiv-internal, released VS 2017 or newer, or released VS 2015 or older. I can 
easily make such a change if desired.) When looking at the directory structure, 
if instead of `"VC"` we see `"x86ret"`, `"x86chk"`, `"amd64ret"`, or 
`"amd64chk"`, we recognize this as a DevDiv-internal build.

After we get past the directory structure validation, we use this knowledge to 
regenerate paths appropriately. `llvmArchToDevDivInternalArch()` knows how we 
use `"i386"` subdirectories, and `MSVCToolChain::getSubDirectoryPath()` uses 
that. It also knows that DevDiv-internal builds have an `"inc"` subdirectory 
instead of `"include"`.

This may still not be the "right" fix in any sense, but I believe that it's 
non-intrusive in the sense that if the special directory names aren't found, 
`IsDevDivInternal` remains `false` and no codepaths are affected. I searched 
for all references to `IsVS2017OrNewer`, which are places where Clang cares 
about VS's directory structure, and the only one that isn't being patched is 
some logic to deal with cross-compilation. I'm fine with that not working for 
DevDiv-internal builds for the moment (we typically test the native compilers).

Is there still time to get a patch like this into 5.0.0, or is it too late?


https://reviews.llvm.org/D36860

Files:
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MSVC.h

Index: lib/Driver/ToolChains/MSVC.h
===================================================================
--- lib/Driver/ToolChains/MSVC.h
+++ lib/Driver/ToolChains/MSVC.h
@@ -131,6 +131,7 @@
 private:
   std::string VCToolChainPath;
   bool IsVS2017OrNewer = false;
+  bool IsDevDivInternal = false;
   CudaInstallationDetector CudaInstallation;
 };
 
Index: lib/Driver/ToolChains/MSVC.cpp
===================================================================
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -76,7 +76,8 @@
 
 // Check various environment variables to try and find a toolchain.
 static bool findVCToolChainViaEnvironment(std::string &Path,
-                                          bool &IsVS2017OrNewer) {
+                                          bool &IsVS2017OrNewer,
+                                          bool &IsDevDivInternal) {
   // These variables are typically set by vcvarsall.bat
   // when launching a developer command prompt.
   if (llvm::Optional<std::string> VCToolsInstallDir =
@@ -138,6 +139,13 @@
           Path = ParentPath;
           IsVS2017OrNewer = false;
           return true;
+        } else if (llvm::sys::path::filename(ParentPath) == "x86ret"
+            || llvm::sys::path::filename(ParentPath) == "x86chk"
+            || llvm::sys::path::filename(ParentPath) == "amd64ret"
+            || llvm::sys::path::filename(ParentPath) == "amd64chk") {
+          Path = ParentPath;
+          IsDevDivInternal = true;
+          return true;
         }
 
       } else {
@@ -677,7 +685,7 @@
   // what they want to use.
   // Failing that, just try to find the newest Visual Studio version we can
   // and use its default VC toolchain.
-  findVCToolChainViaEnvironment(VCToolChainPath, IsVS2017OrNewer) ||
+  findVCToolChainViaEnvironment(VCToolChainPath, IsVS2017OrNewer, IsDevDivInternal) ||
       findVCToolChainViaSetupConfig(VCToolChainPath, IsVS2017OrNewer) ||
       findVCToolChainViaRegistry(VCToolChainPath, IsVS2017OrNewer);
 }
@@ -766,6 +774,21 @@
   }
 }
 
+// Similar to the above function, but for DevDiv internal builds.
+static const char *llvmArchToDevDivInternalArch(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;
+  switch (Arch) {
+  case ArchType::x86:
+    return "i386";
+  case ArchType::x86_64:
+    return "amd64";
+  case ArchType::arm:
+    return "arm";
+  default:
+    return "";
+  }
+}
+
 // Get the path to a specific subdirectory in the current toolchain for
 // a given target architecture.
 // VS2017 changed the VC toolchain layout, so this should be used instead
@@ -776,7 +799,9 @@
   llvm::SmallString<256> Path(VCToolChainPath);
   switch (Type) {
   case SubDirectoryType::Bin:
-    if (IsVS2017OrNewer) {
+    if (IsDevDivInternal) {
+      llvm::sys::path::append(Path, "bin", llvmArchToDevDivInternalArch(TargetArch));
+    } else if (IsVS2017OrNewer) {
       bool HostIsX64 =
           llvm::Triple(llvm::sys::getProcessTriple()).isArch64Bit();
       llvm::sys::path::append(Path, "bin", (HostIsX64 ? "HostX64" : "HostX86"),
@@ -787,12 +812,20 @@
     }
     break;
   case SubDirectoryType::Include:
-    llvm::sys::path::append(Path, "include");
+    if (IsDevDivInternal) {
+      llvm::sys::path::append(Path, "inc");
+    } else {
+      llvm::sys::path::append(Path, "include");
+    }
     break;
   case SubDirectoryType::Lib:
-    llvm::sys::path::append(
-        Path, "lib", IsVS2017OrNewer ? llvmArchToWindowsSDKArch(TargetArch)
-                                     : llvmArchToLegacyVCArch(TargetArch));
+    if (IsDevDivInternal) {
+      llvm::sys::path::append(Path, "lib", llvmArchToDevDivInternalArch(TargetArch));
+    } else if (IsVS2017OrNewer) {
+      llvm::sys::path::append(Path, "lib", llvmArchToWindowsSDKArch(TargetArch));
+    } else {
+      llvm::sys::path::append(Path, "lib", llvmArchToLegacyVCArch(TargetArch));
+    }
     break;
   }
   return Path.str();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to