Get default -fms-compatibility-version from cl.exe's version

[amccarth]

Summary

-fms-compatibility-version was defaulting to 18 (VS 2013), which is a pain if your environment is pointing to version 19 (VS 2015) libraries.

If cl.exe can be found, this patch uses its version number as the default instead. It re-uses the existing code to find the Visual Studio binaries folder and WinAPI methods to check its version. You can still explicitly specify a compatibility version on the command line. If you don't have cl.exe, this should be a no-op and you'll get the old default of 18.

This affected the tests, which assumed that if you didn't specific a version, that it would default to 18, but this won't be true for all machines. So a couple test cases had to be eliminated and a couple others had to be tweaked to allow for various outputs.

Addresses: https://llvm.org/bugs/show_bug.cgi?id=27215

[thakis]

lib/Driver/MSVCToolChain.cpp

477 ↗

(On Diff #56819)

We already stat a bunch of directories to find the sdk include path. Can we use the result of that instead of looking at cl.exe? Then we wouldn't have to do additional stats.

[majnemer]

lib/Driver/MSVCToolChain.cpp

472 ↗

(On Diff #56819)

Why not use the GetFileVersionInfoSizeW variant?

[amccarth]

lib/Driver/MSVCToolChain.cpp

477 ↗

(On Diff #56819)

I'm just a simple CPM/VMS/Windows developer. Your Linux terms (stat) frighten and confuse me.

Seriously, though, this API isn't a file system check. It's digging out the version record from the file's resources.

We _could_ guess at the version from the names of the directories in the path, but that would require mapping names to versions, and if it's installed in a non-standard place it wouldn't help at all.

Also, -fms-compatibility-version is really about the version of the compiler (cl.exe), not that of the standard library nor of the SDK, so trying to check something else as a proxy for the version seems prone to obscure failures.

I share your concern about speed, especially since getting the version happens twice (once for the triple and once for the compatibility version), but invoking clang and having it choose the wrong default costs a lot of time, too.

The bug report correctly says we shouldn't spin up a process to run cl /version--that would be far more expensive. And if you put -fms-compatibility-version on the command line, then this function won't be called as it won't need to figure out the default.

[amccarth]

lib/Driver/MSVCToolChain.cpp

472 ↗

(On Diff #56819)

I started down that road, but it seemed overkill to convert the path to a wide string. I'm happy to do it if you think it worthwhile.

[majnemer]

lib/Driver/MSVCToolChain.cpp

472 ↗

(On Diff #56819)

I think it's worth it, we get bug reports whenever we break this sort of thing...

477 ↗

(On Diff #56819)

Seriously, though, this API isn't a file system check. It's digging out the version record from the file's resources.

Isn't the content stored as a resource in the PE? If so, that means that getting the version information requires reading bytes inside of cl.exe

With regard to -fms-compatibility-version, it shouldn't have anything to do with the platform SDK. However, it is fundamentally the case that the CRT and the compiler have the same version. Otherwise, really terrible things happen (MSVC19 assumes char16_t is a keyword which it provides but the MSVC18 STL assumes it is responsible for providing the keyword).

[rnk]

lib/Driver/MSVCToolChain.cpp

41 ↗

(On Diff #56819)

Personally, I think this is OK but I know Aaron Ballman and other people don't like using pragma comment lib. The alternative is hacking CMake goo, which is always best avoided when possible.

472 ↗

(On Diff #56819)

+1, you can use ConvertUTF8toWide to make this easy.

477 ↗

(On Diff #56819)

I think one extra file read is probably worth the convenience it buys for our users. It's easy to win back by having the user pass an explicit -fms-compatibility-version flag.

lib/Driver/Tools.cpp

3337–3338 ↗

(On Diff #56819)

IMO you should make this a virtual method on Toolchain that does nothing and is only overridden in MSVCToolChain. You can also cache it if you do that.

tools/driver/driver.cpp

504 ↗

(On Diff #56819)

Yeah, it's a typo, but you don't have any other changes in this file, so I wouldn't touch it as part of this change.

[amccarth]

Addressed most comments: now using wide-chars for WinAPI calls, made getMSVCVersionFromExe a virtual method, removed extraneous typo-correction.

[amccarth]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

Yes, it looks in the executable (which I tried to emphasize with the method name).

I don't think this is very expensive given that Explorer often makes zillions of such calls, but I'm open to other suggestions.

I know that you can't use a library that's newer than the compiler (because it may use new language features), but I don't know if that applies in the other direction or how we would safely and reliably map directory names to library versions and therefore to compiler versions.

[probinson]

In D20136#426586, @amccarth wrote:

now using wide-chars for WinAPI calls,

Dōmo arigatō gozaimashita!

[thakis]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

I agree that figuring out the right value for fmsc-version automatically somehow is definitely something we should do.

I forgot that getVisualStudioBinariesFolder already works by looking for cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE parsing to get at the version, and that probably is in cold cache territory. (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx suggests that this function might open several files).

getVisualStudioBinariesFolder checks:

1. getenv("VCINSTALLDIR")
2. cl.exe in getenv("PATH")
3. registry (via getVisualStudioInstallDir)

The common cases are 1 and 3. For 1, for default installs, the version number is part of the directory name (for default installs, what most people have). For 3, the version number is in the registry key we query. So in most cases we shouldn't have to look at cl.exe itself. And for the cases where we would have to look, maybe it's ok to require an explicit fmsc-version flag.

[amccarth]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

The version number in the directory name and the registry is the version number of Visual Studio not of the compiler. Yes, we could do a mapping (VS 14 comes bundled with CL 19), assuming Microsoft continues to keep VS releases and compiler releases in sync, and it means this code will forever need updates to the mapping data.

The mapping would give just the major version number, which might be all that matters now, but if there's ever a CL 19.1 that has different compatibility requirements (and is maybe released out-of-band with Visual Studio), we'd be stuck.

Getting the actual version from the compiler seems the most accurate and future-proof way to check. If that's too expensive, then maybe we should abandon the idea of detecting the default for compatibility.

[amccarth]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

I'll do some research to figure out the actual costs. I suspect that walking the PATH for the executable may be far more expensive, but I'll get some numbers and report back.

[thakis]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

Compilers being released independently of VC versions and fractional compat numbers sounds like things we can worry about when they happen (probably not soon, right?).

We already walk PATH, so that wouldn't be an additional cost.

Be sure to measure cold disk cache perf impact (which is tricky on Windows since there's as far as I know no way to tell the OS to drop its caches). As far as I know file metadata is stored with the directory node on NTFS, so stating files doesn't warm up file content accesses.

[amccarth]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

Compilers being released independently of VC versions and fractional compat numbers sounds like things we can worry about when they happen (probably not soon, right?).

It already happens. Herb Sutter talks about it in one of his blogs: "Soon after VC++11 ships we have announced we will do out-of-band releases for additional C++11 conformance which will naturally also include more C11 features that are in the C subset of C++11." In this case, it's just the build number (of major.minor.build) that's updating, but it's for increasing conformance, which is exactly a compatibility issue.

We already walk PATH, so that wouldn't be an additional cost.

I suspect we may be walking it more than once, which can be expensive even if the cache is all warmed up. This is one of the things I'm checking. If it's a problem, I'll propose a patch to cache the result from the first walk.

stating files doesn't warm up file content accesses.

That is correct.

[amccarth]

lib/Driver/MSVCToolChain.cpp

478 ↗

(On Diff #56833)

My machine in Windows 7 Enterprise, with cl.exe on a spinning HD (not SSD), and with Bit9 in "monitor" mode, which drags down all i/o operations.

First, I used procmon to make sure there were no surprising file ops happening as a result of my change. As expected, it creates the file handle, maps the file, and performs three read non-contiguous operations, loading a total of 5 pages (1 page = 4096 bytes). I'm guessing that the three reads correspond to the file header, a resource index, and the actual version resource. In the cold cache case, the file operations themselves (as reported by procmon) totaled 17 ms.

Subsequent calls from the same process did not even repeat the reads. It seems the system might cache the resource information once loaded.

Procmon also highlighted that the path search is doing twice as many checks as necessary. For each segment it checks for both cl.exe and cl.exe.exe. The path searching (according to procmon) took a little more than 1 ms per walk, and, with this change, it happens three times, for a total of almost 4 ms. This could be eliminated by caching the result in the MSVCToolChain object so subsequent calls are essentially free and/or reduced by eliminating the spurious checks for cl.exe.exe.

With release builds of Clang (with and without my change) and procmon turned off, I built a trivial C++ program multiple times. These are effectively warm-cache results, which I think are appropriate, since you're either building one-off interactively (in which case 20 ms isn't a big deal) or you're building a lot, in which case the cache will likely be warm for all but the first invocation.

W/o this change: 1.426 s average (1.421, 1.431, 1.458, 1.408, 1.411)
With this change: 1.442 s average (1.467, 1.442, 1.417, 1.433, 1.453)

So the delta is 16 ms, some of which is the extra path walking, and is on the order of the run-to-run variance.

[thakis]

Ok, that seems fine then. Thanks much for checking!

[amccarth]

Are there any remaining concerns with this patch? Is everyone satisfied with the performance numbers?

[aaron.ballman]

lib/Driver/MSVCToolChain.cpp

42 ↗

(On Diff #56833)

Eh, I am lightening up on this sort of thing, so this is fine by me.

467 ↗

(On Diff #56833)

Elide braces (here and elsewhere) per usual project coding conventions.

483 ↗

(On Diff #56833)

Pure pedantry: uint8_t instead of char, or is this data really a textual string in practice?

[amccarth]

lib/Driver/MSVCToolChain.cpp

42 ↗

(On Diff #56833)

I was following the pattern I saw in llvm\lib\Support\Windows\Path.inc (and elsewhere), so I thought it was the way we did things around here.

483 ↗

(On Diff #56833)

It's a mix. The part we're looking at is binary data, but the rest of the block is text.

I though the API wanted a pointer to char, so I chose char to avoid unnecessary casts.

But I must've misread the reference page, because just now I double-checked and I see that the API wants a void pointer, so I'll go ahead and use uint8_t, which satisfies my inner pedant as well.

[amccarth]

Addressed additional comments.

[majnemer]

lib/Driver/MSVCToolChain.cpp

481 ↗

(On Diff #57196)

It might be nicer to use a SmallVector<uint8_t, sizeof(VS_FIXEDFILEINFO)>, or whatever VersionSize typically is, here to avoid heap allocation in the common case.

[amccarth]

lib/Driver/MSVCToolChain.cpp

481 ↗

(On Diff #57196)

What's the cutoff for "small"? The version block in cl.exe is about 9KB.

[majnemer]

lib/Driver/MSVCToolChain.cpp

481 ↗

(On Diff #57196)

Using 10K is probably fine, the default stack size on Windows is a massive 1 MB and this function is not reentrant.

[amccarth]

lib/Driver/MSVCToolChain.cpp

481 ↗

(On Diff #57196)

My mistake. It's a smidge over 1KB, (still more than sizeof(VS_FIXEDFILEINFO)) so I've make a SmallVector of 2KB.

[rnk]

LG, sounds like people are happy with this

[thakis]

One consequence from this that I just realized is that linking a binary depending on clang stuff with (morally):

c++ -o foo foo.o $($LLVMBUILD/bin/llvm-config --ldflags) -lclangFrontend -lclangDriver -lclangParse -lclangSema -lclangSerialization -lclangAnalysis -lclangAST -lclangEdit -lclangLex -lclangBasic $($LLVMBUILD/bin/llvm-config --libs) $($LLVMBUILD/bin/llvm-config --system-libs)

now fails with

clangDriver.lib(MSVCToolChain.cpp.obj) : error LNK2019: unresolved external symbol GetFileVersionInfoSizeW referenced in
 function "private: class clang::VersionTuple __cdecl clang::driver::toolchains::MSVCToolChain::getMSVCVersionFromExe(vo
id)const " (?getMSVCVersionFromExe@MSVCToolChain@toolchains@driver@clang@@AEBA?AVVersionTuple@4@XZ)
clangDriver.lib(MSVCToolChain.cpp.obj) : error LNK2019: unresolved external symbol GetFileVersionInfoW referenced in fun
ction "private: class clang::VersionTuple __cdecl clang::driver::toolchains::MSVCToolChain::getMSVCVersionFromExe(void)c
onst " (?getMSVCVersionFromExe@MSVCToolChain@toolchains@driver@clang@@AEBA?AVVersionTuple@4@XZ)
clangDriver.lib(MSVCToolChain.cpp.obj) : error LNK2019: unresolved external symbol VerQueryValueW referenced in function
 "private: class clang::VersionTuple __cdecl clang::driver::toolchains::MSVCToolChain::getMSVCVersionFromExe(void)const
" (?getMSVCVersionFromExe@MSVCToolChain@toolchains@driver@clang@@AEBA?AVVersionTuple@4@XZ)
pptest.exe : fatal error LNK1120: 3 unresolved externals

and folks have to manually add mincore.lib to their link. llvm-config --system-libs takes care of things like this for LLVM, but we don't have a clang-config as far as I know (does anybody know why not?). So folks now have to manually pass more libraries than just the clang libraries. Not a big deal, but I figured I'd point it out.

[amccarth]

and folks have to manually add mincore.lib to their link.

I could load the library dynamically on demand and use GetProcAddress, but I suspect that would further degrade the performance. I could probably write my own code to find the version in the binary, but I doubt that crosses the reward/work threshold.

[rnk]

In D20136#640689, @amccarth wrote:

and folks have to manually add mincore.lib to their link.

I could load the library dynamically on demand and use GetProcAddress, but I suspect that would further degrade the performance. I could probably write my own code to find the version in the binary, but I doubt that crosses the reward/work threshold.

#pragma comment(lib, "mincore"), maybe?

[thakis]

Aha, looks like this did have a #pragma comment(lib when it went in (for version.lib which is actually the correct one), but takumi removed it in http://llvm.org/viewvc/llvm-project?rev=269557&view=rev

cfe/trunk/include/clang/Driver/ToolChain.h

 //===--- ToolChain.h - Collections of tools for one platform ----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_DRIVER_TOOLCHAIN_H
 #define LLVM_CLANG_DRIVER_TOOLCHAIN_H
 
 #include "clang/Basic/Sanitizers.h"
+#include "clang/Basic/VersionTuple.h"
 #include "clang/Driver/Action.h"
 #include "clang/Driver/Multilib.h"
 #include "clang/Driver/Types.h"
 #include "clang/Driver/Util.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Target/TargetOptions.h"
public:
   virtual void AddCudaIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                                   llvm::opt::ArgStringList &CC1Args) 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(); }
 };
 
 } // end namespace driver
 } // end namespace clang
 
 #endif

cfe/trunk/lib/Driver/MSVCToolChain.cpp

 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
 #include <cstdio>
 
 // Include the necessary headers to interface with the Windows registry and
 // environment.
 #if defined(LLVM_ON_WIN32)
 #define USE_WIN32
 #endif
 
 #ifdef USE_WIN32
   #define WIN32_LEAN_AND_MEAN
   #define NOGDI
   #ifndef NOMINMAX
     #define NOMINMAX
   #endif
   #include <windows.h>
+
+  #pragma comment(lib, "version.lib")
 #endif
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
 MSVCToolChain::MSVCToolChain(const Driver &D, const llvm::Triple& Triple,
bool MSVCToolChain::getVisualStudioBinariesFolder(const char *clangProgramPath,
   default:
     // Whatever this is, Visual Studio doesn't have a toolchain for it.
     return false;
   }
   path = BinDir.str();
   return true;
 }
 
+VersionTuple MSVCToolChain::getMSVCVersionFromExe() const {
+  VersionTuple Version;
+#ifdef USE_WIN32
+  std::string BinPath;
+  if (!getVisualStudioBinariesFolder("", BinPath))
+    return Version;
+  SmallString<128> ClExe = BinPath;
+  llvm::sys::path::append(ClExe, "cl.exe");
+
+  std::wstring ClExeWide;
+  if (!llvm::ConvertUTF8toWide(ClExe.c_str(), ClExeWide))
+    return Version;
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+                                                      nullptr);
+  if (VersionSize == 0)
+    return Version;
+
+  SmallVector<uint8_t, 4 * 1024> VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
+                             VersionBlock.data()))
+    return Version;
+
+  VS_FIXEDFILEINFO *FileInfo = nullptr;
+  UINT FileInfoSize = 0;
+  if (!::VerQueryValueW(VersionBlock.data(), L"\\",
+                        reinterpret_cast<LPVOID *>(&FileInfo), &FileInfoSize) ||
+      FileInfoSize < sizeof(*FileInfo))
+    return Version;
+
+  const unsigned Major = (FileInfo->dwFileVersionMS >> 16) & 0xFFFF;
+  const unsigned Minor = (FileInfo->dwFileVersionMS      ) & 0xFFFF;
+  const unsigned Micro = (FileInfo->dwFileVersionLS >> 16) & 0xFFFF;
+
+  Version = VersionTuple(Major, Minor, Micro);
+#endif
+  return Version;
+}
+
 // Get Visual Studio installation directory.
 bool MSVCToolChain::getVisualStudioInstallDir(std::string &path) const {
   // First check the environment variables that vsvars32.bat sets.
   const char *vcinstalldir = getenv("VCINSTALLDIR");
   if (vcinstalldir) {
     path = vcinstalldir;
     path = path.substr(0, path.find("\\VC"));
     return true;

 
 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, Triple, Args,
+      tools::visualstudio::getMSVCVersion(/*D=*/nullptr, *this, Triple, Args,
                                           /*IsWindowsMSVC=*/true);
   if (MSVT.empty())
     return TripleStr;
 
   MSVT = VersionTuple(MSVT.getMajor(), MSVT.getMinor().getValueOr(0),
                       MSVT.getSubminor().getValueOr(0));
 
   if (Triple.getEnvironment() == llvm::Triple::MSVC) {

cfe/trunk/lib/Driver/ToolChains.h

public:
   bool getWindowsSDKLibraryPath(std::string &path) const;
   /// \brief Check if Universal CRT should be used if available
   bool useUniversalCRT(std::string &visualStudioDir) const;
   bool getUniversalCRTSdkDir(std::string &path, std::string &ucrtVersion) const;
   bool getUniversalCRTLibraryPath(std::string &path) const;
   bool getVisualStudioInstallDir(std::string &path) const;
   bool getVisualStudioBinariesFolder(const char *clangProgramPath,
                                      std::string &path) const;
+  VersionTuple getMSVCVersionFromExe() const override;
 
   std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args,
                                           types::ID InputType) const override;
   SanitizerMask getSupportedSanitizers() const override;
 
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
                                      llvm::opt::ArgStringList &CC1Args,

cfe/trunk/lib/Driver/Tools.h

void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;
 };
 } // end namespace dragonfly
 
 /// Visual studio tools.
 namespace visualstudio {
-VersionTuple getMSVCVersion(const Driver *D, const llvm::Triple &Triple,
+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)
       : Tool("visualstudio::Linker", "linker", TC, RF_Full,
              llvm::sys::WEM_UTF16) {}
 

cfe/trunk/lib/Driver/Tools.cpp

#ifdef LLVM_ON_UNIX
   std::string UID = llvm::utostr(getuid());
 #else
   // FIXME: Windows seems to have an 'SID' that might work.
   std::string UID = "9999";
 #endif
   Result.append(UID.begin(), UID.end());
 }
 
-VersionTuple visualstudio::getMSVCVersion(const Driver *D,
+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);
if (MSCVersion) {
       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 void addPGOAndCoverageFlags(Compilation &C, const Driver &D,
                                    const InputInfo &Output, const ArgList &Args,
                                    ArgStringList &CmdArgs) {
   auto *ProfileGenerateArg = Args.getLastArg(
       options::OPT_fprofile_instr_generate,
if (Args.hasFlag(options::OPT_fms_compatibility,
                    options::OPT_fno_ms_compatibility,
                    (IsWindowsMSVC &&
                     Args.hasFlag(options::OPT_fms_extensions,
                                  options::OPT_fno_ms_extensions, true))))
     CmdArgs.push_back("-fms-compatibility");
 
   // -fms-compatibility-version=18.00 is default.
   VersionTuple MSVT = visualstudio::getMSVCVersion(
-      &D, getToolChain().getTriple(), Args, IsWindowsMSVC);
+      &D, getToolChain(), getToolChain().getTriple(), Args, IsWindowsMSVC);
   if (!MSVT.empty())
     CmdArgs.push_back(
         Args.MakeArgString("-fms-compatibility-version=" + MSVT.getAsString()));
 
   bool IsMSVC2015Compatible = MSVT.getMajor() >= 19;
   if (ImplyVCPPCXXVer) {
     if (IsMSVC2015Compatible)
       CmdArgs.push_back("-std=c++14");

cfe/trunk/test/Driver/cl-options.c

 // RUN: %clang_cl /c /GR- -### -- %s 2>&1 | FileCheck -check-prefix=NoRTTI %s
 // NoRTTI: "-fno-rtti-data"
 // NoRTTI-NOT: "-fno-rtti"
 // RUN: %clang_cl /c /GR -### -- %s 2>&1 | FileCheck -check-prefix=RTTI %s
 // RTTI-NOT: "-fno-rtti-data"
 // RTTI-NOT: "-fno-rtti"
 
 // thread safe statics are off for versions < 19.
-// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
+// RUN: %clang_cl /c -### -fms-compatibility-version=18 -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
 // RUN: %clang_cl /Zc:threadSafeInit /Zc:threadSafeInit- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
 // NoThreadSafeStatics: "-fno-threadsafe-statics"
 
 // RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
 // ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"

cfe/trunk/test/Driver/msc-version.c

 //
-// Verify defaults
-//
-
-// RUN: %clang -target i686-windows -fms-compatibility -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-NO-MSC-VERSION
-
-// CHECK-NO-MSC-VERSION: _MSC_BUILD 1
-// CHECK-NO-MSC-VERSION: _MSC_FULL_VER 180000000
-// CHECK-NO-MSC-VERSION: _MSC_VER 1800
-
-
-//
 // Verify -fms-compatibility-version parsing
 //
 
 // RUN: %clang -target i686-windows -fms-compatibility -fms-compatibility-version=14 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR
 
 // CHECK-MSC-VERSION-MAJOR: _MSC_BUILD 1
 // CHECK-MSC-VERSION-MAJOR: _MSC_FULL_VER 140000000
 // CHECK-MSC-VERSION-MAJOR: _MSC_VER 1400

cfe/trunk/test/Driver/msvc-triple.c

-// RUN: %clang -target i686-pc-windows-msvc   -S -emit-llvm %s -o - | FileCheck %s --check-prefix=DEFAULT
 // RUN: %clang -target i686-pc-windows-msvc19 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=TARGET-19
 // RUN: %clang -target i686-pc-windows-msvc   -S -emit-llvm %s -o - -fms-compatibility-version=19 | FileCheck %s --check-prefix=OVERRIDE-19
 // RUN: %clang -target i686-pc-windows-msvc-elf -S -emit-llvm %s -o - | FileCheck %s --check-prefix=ELF-DEFAULT
 
-// DEFAULT:     target triple = "i686-pc-windows-msvc18.0.0"
 // TARGET-19:   target triple = "i686-pc-windows-msvc19.0.0"
 // OVERRIDE-19: target triple = "i686-pc-windows-msvc19.0.0"
-// ELF-DEFAULT: target triple = "i686-pc-windows-msvc18.0.0-elf"
+// ELF-DEFAULT: target triple = "i686-pc-windows-msvc{{.*}}-elf"

cfe/trunk/test/Misc/diag-format.c

 
 
 
 #ifdef foo
 #endif bad // extension!
 // DEFAULT: {{.*}}:36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2010: {{.*}}(36,7) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2013: {{.*}}(36,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
-// MSVC: {{.*}}(36,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
+// MSVC: {{.*}}(36,8){{ ?}}: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2015: {{.*}}(36,8): warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // VI: {{.*}} +36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2015_ORIG: {{.*}}(36): warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // NO_COLUMN: {{.*}}:36: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2010-FALLBACK: {{.*}}(36,7) : error(clang): extra tokens at end of #endif directive
 // MSVC2013-FALLBACK: {{.*}}(36,8) : error(clang): extra tokens at end of #endif directive
 // MSVC2015-FALLBACK: {{.*}}(36,8): error(clang): extra tokens at end of #endif directive
 int x;