r288998 - Refactor how the MSVC toolchain searches for a compatibility version.

David L. Jones via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 7 15:41:58 PST 2016


Author: dlj
Date: Wed Dec  7 17:41:58 2016
New Revision: 288998

URL: http://llvm.org/viewvc/llvm-project?rev=288998&view=rev
Log:
Refactor how the MSVC toolchain searches for a compatibility version.

Summary:
The MSVC toolchain and Clang driver combination currently uses a fairly complex
sequence of steps to determine the MS compatibility version to pass to cc1.
There is some oddness in this sequence currently, with some code which inspects
flags in the toolchain, and some code which inspects the triple and local
environment in the driver code.

This change is an attempt to consolidate most of this logic so that
Win32-specific code lives in MSVCToolChain.cpp. I'm not 100% happy with the
split, so any suggestions are welcome.

There are a few things you might want to watch for for specifically:

 - On all platforms, if MSVC compatibility flags are provided (and valid), use
   those.
 - The fallback sequence should be the same as before, but is now consolidated
   into MSVCToolChain::getMSVCVersion:
   - Otherwise, try to use the Triple.
   - Otherwise, on Windows, check the executable.
   - Otherwise, on Windows or with --fms-extensions, default to 18.
   - Otherwise, we can't determine the version.
 - MSVCToolChain::ComputeEffectiveTriple no longer calls the base
   ToolChain::ComputeEffectiveClangTriple. The only thing it would change for
   Windows the architecture, which we don't care about for the compatibility
   version.
    - I'm not sure whether this is philosophically correct (but it should
      be easy to add back to MSVCToolChain::getMSVCVersionFromTriple if not).
    - Previously, Tools.cpp just called getTriple() anyhow, so it doesn't look
      like the effective triple was always being used previously anyhow.

Reviewers: hans, compnerd, llvm-commits, rnk

Subscribers: amccarth

Differential Revision: https://reviews.llvm.org/D27477

Modified:
    cfe/trunk/include/clang/Driver/ToolChain.h
    cfe/trunk/lib/Driver/MSVCToolChain.cpp
    cfe/trunk/lib/Driver/ToolChain.cpp
    cfe/trunk/lib/Driver/ToolChains.h
    cfe/trunk/lib/Driver/Tools.cpp
    cfe/trunk/lib/Driver/Tools.h

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Wed Dec  7 17:41:58 2016
@@ -442,15 +442,15 @@ public:
   virtual void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args) const;
 
+  /// \brief On Windows, returns the MSVC compatibility version.
+  virtual VersionTuple computeMSVCVersion(const Driver *D,
+                                          const llvm::opt::ArgList &Args) const;
+
   /// \brief Return sanitizers which are available in this toolchain.
   virtual SanitizerMask getSupportedSanitizers() const;
 
   /// \brief Return sanitizers which are enabled by default.
   virtual SanitizerMask getDefaultSanitizers() const { return 0; }
-
-  /// \brief On Windows, returns the version of cl.exe.  On other platforms,
-  /// returns an empty VersionTuple.
-  virtual VersionTuple getMSVCVersionFromExe() const { return VersionTuple(); }
 };
 
 /// Set a ToolChain's effective triple. Reset it when the registration object

Modified: cfe/trunk/lib/Driver/MSVCToolChain.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MSVCToolChain.cpp?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/MSVCToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/MSVCToolChain.cpp Wed Dec  7 17:41:58 2016
@@ -16,6 +16,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -472,6 +473,14 @@ bool MSVCToolChain::getVisualStudioBinar
   return true;
 }
 
+VersionTuple MSVCToolChain::getMSVCVersionFromTriple() const {
+  unsigned Major, Minor, Micro;
+  getTriple().getEnvironmentVersion(Major, Minor, Micro);
+  if (Major || Minor || Micro)
+    return VersionTuple(Major, Minor, Micro);
+  return VersionTuple();
+}
+
 VersionTuple MSVCToolChain::getMSVCVersionFromExe() const {
   VersionTuple Version;
 #ifdef USE_WIN32
@@ -668,21 +677,34 @@ void MSVCToolChain::AddClangCXXStdlibInc
   // FIXME: There should probably be logic here to find libc++ on Windows.
 }
 
+VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *D,
+                                               const ArgList &Args) const {
+  bool IsWindowsMSVC = getTriple().isWindowsMSVCEnvironment();
+  VersionTuple MSVT = ToolChain::computeMSVCVersion(D, Args);
+  if (MSVT.empty()) MSVT = getMSVCVersionFromTriple();
+  if (MSVT.empty() && IsWindowsMSVC) MSVT = getMSVCVersionFromExe();
+  if (MSVT.empty() &&
+      Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
+                   IsWindowsMSVC)) {
+    // -fms-compatibility-version=18.00 is default.
+    // FIXME: Consider bumping this to 19 (MSVC2015) soon.
+    MSVT = VersionTuple(18);
+  }
+  return MSVT;
+}
+
 std::string
 MSVCToolChain::ComputeEffectiveClangTriple(const ArgList &Args,
                                            types::ID InputType) const {
-  std::string TripleStr =
-      ToolChain::ComputeEffectiveClangTriple(Args, InputType);
-  llvm::Triple Triple(TripleStr);
-  VersionTuple MSVT =
-      tools::visualstudio::getMSVCVersion(/*D=*/nullptr, *this, Triple, Args,
-                                          /*IsWindowsMSVC=*/true);
-  if (MSVT.empty())
-    return TripleStr;
-
+  // The MSVC version doesn't care about the architecture, even though it
+  // may look at the triple internally.
+  VersionTuple MSVT = computeMSVCVersion(/*D=*/nullptr, Args);
   MSVT = VersionTuple(MSVT.getMajor(), MSVT.getMinor().getValueOr(0),
                       MSVT.getSubminor().getValueOr(0));
 
+  // For the rest of the triple, however, a computed architecture name may
+  // be needed.
+  llvm::Triple Triple(ToolChain::ComputeEffectiveClangTriple(Args, InputType));
   if (Triple.getEnvironment() == llvm::Triple::MSVC) {
     StringRef ObjFmt = Triple.getEnvironmentName().split('-').second;
     if (ObjFmt.empty())

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Wed Dec  7 17:41:58 2016
@@ -721,3 +721,57 @@ void ToolChain::AddCudaIncludeArgs(const
 
 void ToolChain::AddIAMCUIncludeArgs(const ArgList &DriverArgs,
                                     ArgStringList &CC1Args) const {}
+
+static VersionTuple separateMSVCFullVersion(unsigned Version) {
+  if (Version < 100)
+    return VersionTuple(Version);
+
+  if (Version < 10000)
+    return VersionTuple(Version / 100, Version % 100);
+
+  unsigned Build = 0, Factor = 1;
+  for (; Version > 10000; Version = Version / 10, Factor = Factor * 10)
+    Build = Build + (Version % 10) * Factor;
+  return VersionTuple(Version / 100, Version % 100, Build);
+}
+
+VersionTuple
+ToolChain::computeMSVCVersion(const Driver *D,
+                              const llvm::opt::ArgList &Args) const {
+  const Arg *MSCVersion = Args.getLastArg(options::OPT_fmsc_version);
+  const Arg *MSCompatibilityVersion =
+      Args.getLastArg(options::OPT_fms_compatibility_version);
+
+  if (MSCVersion && MSCompatibilityVersion) {
+    if (D)
+      D->Diag(diag::err_drv_argument_not_allowed_with)
+          << MSCVersion->getAsString(Args)
+          << MSCompatibilityVersion->getAsString(Args);
+    return VersionTuple();
+  }
+
+  if (MSCompatibilityVersion) {
+    VersionTuple MSVT;
+    if (MSVT.tryParse(MSCompatibilityVersion->getValue())) {
+      if (D)
+        D->Diag(diag::err_drv_invalid_value)
+            << MSCompatibilityVersion->getAsString(Args)
+            << MSCompatibilityVersion->getValue();
+    } else {
+      return MSVT;
+    }
+  }
+
+  if (MSCVersion) {
+    unsigned Version = 0;
+    if (StringRef(MSCVersion->getValue()).getAsInteger(10, Version)) {
+      if (D)
+        D->Diag(diag::err_drv_invalid_value)
+            << MSCVersion->getAsString(Args) << MSCVersion->getValue();
+    } else {
+      return separateMSVCFullVersion(Version);
+    }
+  }
+
+  return VersionTuple();
+}

Modified: cfe/trunk/lib/Driver/ToolChains.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains.h (original)
+++ cfe/trunk/lib/Driver/ToolChains.h Wed Dec  7 17:41:58 2016
@@ -1140,7 +1140,9 @@ public:
   bool getVisualStudioInstallDir(std::string &path) const;
   bool getVisualStudioBinariesFolder(const char *clangProgramPath,
                                      std::string &path) const;
-  VersionTuple getMSVCVersionFromExe() const override;
+  VersionTuple
+  computeMSVCVersion(const Driver *D,
+                     const llvm::opt::ArgList &Args) const override;
 
   std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args,
                                           types::ID InputType) const override;
@@ -1156,6 +1158,9 @@ protected:
 
   Tool *buildLinker() const override;
   Tool *buildAssembler() const override;
+private:
+  VersionTuple getMSVCVersionFromTriple() const;
+  VersionTuple getMSVCVersionFromExe() const;
 };
 
 class LLVM_LIBRARY_VISIBILITY CrossWindowsToolChain : public Generic_GCC {

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Dec  7 17:41:58 2016
@@ -3552,19 +3552,6 @@ static void addDashXForInput(const ArgLi
     CmdArgs.push_back(types::getTypeName(Input.getType()));
 }
 
-static VersionTuple getMSCompatibilityVersion(unsigned Version) {
-  if (Version < 100)
-    return VersionTuple(Version);
-
-  if (Version < 10000)
-    return VersionTuple(Version / 100, Version % 100);
-
-  unsigned Build = 0, Factor = 1;
-  for (; Version > 10000; Version = Version / 10, Factor = Factor * 10)
-    Build = Build + (Version % 10) * Factor;
-  return VersionTuple(Version / 100, Version % 100, Build);
-}
-
 // Claim options we don't want to warn if they are unused. We do this for
 // options that build systems might add but are unused when assembling or only
 // running the preprocessor for example.
@@ -3608,60 +3595,6 @@ static void appendUserToPath(SmallVector
   Result.append(UID.begin(), UID.end());
 }
 
-VersionTuple visualstudio::getMSVCVersion(const Driver *D, const ToolChain &TC,
-                                          const llvm::Triple &Triple,
-                                          const llvm::opt::ArgList &Args,
-                                          bool IsWindowsMSVC) {
-  if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
-                   IsWindowsMSVC) ||
-      Args.hasArg(options::OPT_fmsc_version) ||
-      Args.hasArg(options::OPT_fms_compatibility_version)) {
-    const Arg *MSCVersion = Args.getLastArg(options::OPT_fmsc_version);
-    const Arg *MSCompatibilityVersion =
-        Args.getLastArg(options::OPT_fms_compatibility_version);
-
-    if (MSCVersion && MSCompatibilityVersion) {
-      if (D)
-        D->Diag(diag::err_drv_argument_not_allowed_with)
-            << MSCVersion->getAsString(Args)
-            << MSCompatibilityVersion->getAsString(Args);
-      return VersionTuple();
-    }
-
-    if (MSCompatibilityVersion) {
-      VersionTuple MSVT;
-      if (MSVT.tryParse(MSCompatibilityVersion->getValue()) && D)
-        D->Diag(diag::err_drv_invalid_value)
-            << MSCompatibilityVersion->getAsString(Args)
-            << MSCompatibilityVersion->getValue();
-      return MSVT;
-    }
-
-    if (MSCVersion) {
-      unsigned Version = 0;
-      if (StringRef(MSCVersion->getValue()).getAsInteger(10, Version) && D)
-        D->Diag(diag::err_drv_invalid_value) << MSCVersion->getAsString(Args)
-                                             << MSCVersion->getValue();
-      return getMSCompatibilityVersion(Version);
-    }
-
-    unsigned Major, Minor, Micro;
-    Triple.getEnvironmentVersion(Major, Minor, Micro);
-    if (Major || Minor || Micro)
-      return VersionTuple(Major, Minor, Micro);
-
-    if (IsWindowsMSVC) {
-      VersionTuple MSVT = TC.getMSVCVersionFromExe();
-      if (!MSVT.empty())
-        return MSVT;
-
-      // FIXME: Consider bumping this to 19 (MSVC2015) soon.
-      return VersionTuple(18);
-    }
-  }
-  return VersionTuple();
-}
-
 static Arg *getLastProfileUseArg(const ArgList &Args) {
   auto *ProfileUseArg = Args.getLastArg(
       options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
@@ -5867,9 +5800,8 @@ void Clang::ConstructJob(Compilation &C,
                                  options::OPT_fno_ms_extensions, true))))
     CmdArgs.push_back("-fms-compatibility");
 
-  // -fms-compatibility-version=18.00 is default.
-  VersionTuple MSVT = visualstudio::getMSVCVersion(
-      &D, getToolChain(), getToolChain().getTriple(), Args, IsWindowsMSVC);
+  VersionTuple MSVT =
+      getToolChain().computeMSVCVersion(&getToolChain().getDriver(), Args);
   if (!MSVT.empty())
     CmdArgs.push_back(
         Args.MakeArgString("-fms-compatibility-version=" + MSVT.getAsString()));

Modified: cfe/trunk/lib/Driver/Tools.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=288998&r1=288997&r2=288998&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Tools.h (original)
+++ cfe/trunk/lib/Driver/Tools.h Wed Dec  7 17:41:58 2016
@@ -725,10 +725,6 @@ public:
 
 /// Visual studio tools.
 namespace visualstudio {
-VersionTuple getMSVCVersion(const Driver *D, const ToolChain &TC,
-                            const llvm::Triple &Triple,
-                            const llvm::opt::ArgList &Args, bool IsWindowsMSVC);
-
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
 public:
   Linker(const ToolChain &TC)




More information about the cfe-commits mailing list