Browse Source

Change PerThreadFileSystem solution to still require explicit Setup

- Added AutoCleanupPerThreadFileSystem for calling cleanup on scope exit,
  useful for command line users.
- Use global singleton and ensure cleanup on exit if not called
- Put setup/cleanup in VerifierTest.cpp where the TestModule[Setup|Cleanup]
  methods live.
- Remove GlobalPerThreadSys from FileCheckForTest, since this has to happen
  at the module level instead.
- Added Setup/Cleanup to all the appropriate places
Tex Riddell 7 years ago
parent
commit
283b71ca16

+ 2 - 3
include/llvm/Support/FileSystem.h

@@ -53,12 +53,11 @@ namespace fs {
 class MSFileSystem;
 class MSFileSystem;
 typedef _Inout_ MSFileSystem* MSFileSystemRef;
 typedef _Inout_ MSFileSystem* MSFileSystemRef;
 
 
-std::error_code GetFileSystemTlsError() throw();
+std::error_code GetFileSystemTlsStatus() throw();
 
 
-// Setup/Cleanup are no longer necessary, but SetupPerThreadFileSystem will
-//  return GetFileSystemTlsError();
 std::error_code SetupPerThreadFileSystem() throw();
 std::error_code SetupPerThreadFileSystem() throw();
 void CleanupPerThreadFileSystem() throw();
 void CleanupPerThreadFileSystem() throw();
+struct AutoCleanupPerThreadFileSystem { ~AutoCleanupPerThreadFileSystem() { CleanupPerThreadFileSystem(); } };
 
 
 /// <summary>Gets a reference to the file system installed for the current thread (possibly NULL).</summary>
 /// <summary>Gets a reference to the file system installed for the current thread (possibly NULL).</summary>
 /// <remarks>In practice, consumers of the library should always install a file system.</remarks>
 /// <remarks>In practice, consumers of the library should always install a file system.</remarks>

+ 31 - 12
lib/Support/Windows/MSFileSystem.inc.cpp

@@ -58,18 +58,28 @@ class ThreadLocalStorage {
   DWORD m_Tls;
   DWORD m_Tls;
   DWORD m_dwError;
   DWORD m_dwError;
 public:
 public:
-  ThreadLocalStorage() : m_Tls(TlsAlloc()), m_dwError(0) {
-    if (m_Tls == TLS_OUT_OF_INDEXES)
-      m_dwError = ::GetLastError();
+  ThreadLocalStorage() : m_Tls(TLS_OUT_OF_INDEXES), m_dwError(ERROR_NOT_READY) {}
+  DWORD Setup() {
+    if (m_Tls == TLS_OUT_OF_INDEXES) {
+      m_Tls = TlsAlloc();
+      m_dwError = (m_Tls == TLS_OUT_OF_INDEXES) ? ::GetLastError() : 0;
+    }
+    return m_dwError;
   }
   }
-  ~ThreadLocalStorage() { TlsFree(m_Tls); }
-  _T GetValue() throw() {
+  void Cleanup() {
+    if (m_Tls != TLS_OUT_OF_INDEXES)
+      TlsFree(m_Tls);
+    m_Tls = TLS_OUT_OF_INDEXES;
+    m_dwError = ERROR_NOT_READY;
+  }
+  ~ThreadLocalStorage() { Cleanup(); }
+  _T GetValue() const {
     if (m_Tls != TLS_OUT_OF_INDEXES)
     if (m_Tls != TLS_OUT_OF_INDEXES)
       return (_T)TlsGetValue(m_Tls);
       return (_T)TlsGetValue(m_Tls);
     else
     else
       return nullptr;
       return nullptr;
   }
   }
-  bool SetValue(_T value) throw() {
+  bool SetValue(_T value) {
     if (m_Tls != TLS_OUT_OF_INDEXES) {
     if (m_Tls != TLS_OUT_OF_INDEXES) {
       return TlsSetValue(m_Tls, (void*)value);
       return TlsSetValue(m_Tls, (void*)value);
     } else {
     } else {
@@ -81,11 +91,14 @@ public:
   DWORD GetError() const {
   DWORD GetError() const {
     return m_dwError;
     return m_dwError;
   }
   }
+  operator bool() const { return m_Tls != TLS_OUT_OF_INDEXES; }
 };
 };
+
 static ThreadLocalStorage<MSFileSystemRef> g_PerThreadSystem;
 static ThreadLocalStorage<MSFileSystemRef> g_PerThreadSystem;
+
 }
 }
 
 
-error_code GetFileSystemTlsError() throw() {
+error_code GetFileSystemTlsStatus() throw() {
   DWORD dwError = g_PerThreadSystem.GetError();
   DWORD dwError = g_PerThreadSystem.GetError();
   if (dwError)
   if (dwError)
     return error_code(dwError, system_category());
     return error_code(dwError, system_category());
@@ -93,19 +106,25 @@ error_code GetFileSystemTlsError() throw() {
     return error_code();
     return error_code();
 }
 }
 
 
-// No longer necessary to call, but returns error code if TlsAlloc() failed.
 error_code SetupPerThreadFileSystem() throw() {
 error_code SetupPerThreadFileSystem() throw() {
-  return GetFileSystemTlsError();
+  assert(!g_PerThreadSystem && g_PerThreadSystem.GetError() == ERROR_NOT_READY &&
+          "otherwise, PerThreadSystem already set up.");
+  if (g_PerThreadSystem.Setup())
+    return GetFileSystemTlsStatus();
+  return error_code();
+}
+void CleanupPerThreadFileSystem() throw() {
+  g_PerThreadSystem.Cleanup();
 }
 }
-void CleanupPerThreadFileSystem() throw() {}
 
 
-MSFileSystemRef GetCurrentThreadFileSystem() throw()
-{
+MSFileSystemRef GetCurrentThreadFileSystem() throw() {
+  assert(g_PerThreadSystem && "otherwise, TLS not initialized");
   return g_PerThreadSystem.GetValue();
   return g_PerThreadSystem.GetValue();
 }
 }
 
 
 error_code SetCurrentThreadFileSystem(MSFileSystemRef value) throw()
 error_code SetCurrentThreadFileSystem(MSFileSystemRef value) throw()
 {
 {
+  assert(g_PerThreadSystem && "otherwise, TLS not initialized");
   // For now, disallow reentrancy in APIs (i.e., replace the current instance with another one).
   // For now, disallow reentrancy in APIs (i.e., replace the current instance with another one).
   if (value != nullptr)
   if (value != nullptr)
   {
   {

+ 3 - 0
tools/clang/tools/driver/driver.cpp

@@ -373,6 +373,9 @@ static int ExecuteCC1Tool(ArrayRef<const char *> argv, StringRef Tool) {
 // HLSL Change: changed calling convention to __cdecl
 // HLSL Change: changed calling convention to __cdecl
 int __cdecl main(int argc_, const char **argv_) {
 int __cdecl main(int argc_, const char **argv_) {
   // HLSL Change Starts
   // HLSL Change Starts
+  if (llvm::sys::fs::SetupPerThreadFileSystem())
+    return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   llvm::sys::fs::MSFileSystem* msfPtr;
   llvm::sys::fs::MSFileSystem* msfPtr;
   HRESULT hr;
   HRESULT hr;
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))

+ 3 - 0
tools/clang/tools/dxl/dxl.cpp

@@ -120,6 +120,9 @@ using namespace hlsl::options;
 int __cdecl main(int argc, _In_reads_z_(argc) char **argv) {
 int __cdecl main(int argc, _In_reads_z_(argc) char **argv) {
   const char *pStage = "Operation";
   const char *pStage = "Operation";
   int retVal = 0;
   int retVal = 0;
+  if (llvm::sys::fs::SetupPerThreadFileSystem())
+    return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   if (FAILED(DxcInitThreadMalloc())) return 1;
   if (FAILED(DxcInitThreadMalloc())) return 1;
   DxcSetThreadMallocOrDefault(nullptr);
   DxcSetThreadMallocOrDefault(nullptr);
   try {
   try {

+ 5 - 0
tools/clang/tools/dxopt/dxopt.cpp

@@ -27,6 +27,8 @@
 #include <iostream>
 #include <iostream>
 #include <limits>
 #include <limits>
 
 
+#include "llvm/Support/FileSystem.h"
+
 inline bool wcseq(LPCWSTR a, LPCWSTR b) {
 inline bool wcseq(LPCWSTR a, LPCWSTR b) {
   return (a == nullptr && b == nullptr) || (a != nullptr && b != nullptr && wcscmp(a, b) == 0);
   return (a == nullptr && b == nullptr) || (a != nullptr && b != nullptr && wcscmp(a, b) == 0);
 }
 }
@@ -211,6 +213,9 @@ static void PrintHelp() {
 int __cdecl wmain(int argc, const wchar_t **argv_) {
 int __cdecl wmain(int argc, const wchar_t **argv_) {
   const char *pStage = "Operation";
   const char *pStage = "Operation";
   int retVal = 0;
   int retVal = 0;
+  if (llvm::sys::fs::SetupPerThreadFileSystem())
+    return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   try {
   try {
     // Parse command line options.
     // Parse command line options.
     pStage = "Argument processing";
     pStage = "Argument processing";

+ 0 - 2
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -1750,8 +1750,6 @@ bool CompilerTest::InitSupport() {
   if (!m_dllSupport.IsEnabled()) {
   if (!m_dllSupport.IsEnabled()) {
     VERIFY_SUCCEEDED(m_dllSupport.Initialize());
     VERIFY_SUCCEEDED(m_dllSupport.Initialize());
     m_ver.Initialize(m_dllSupport);
     m_ver.Initialize(m_dllSupport);
-    if (llvm::sys::fs::GetFileSystemTlsResult())
-      return false;
   }
   }
   return true;
   return true;
 }
 }

+ 0 - 2
tools/clang/unittests/HLSL/DxilModuleTest.cpp

@@ -64,8 +64,6 @@ bool DxilModuleTest::InitSupport() {
   if (!m_dllSupport.IsEnabled()) {
   if (!m_dllSupport.IsEnabled()) {
     VERIFY_SUCCEEDED(m_dllSupport.Initialize());
     VERIFY_SUCCEEDED(m_dllSupport.Initialize());
   }
   }
-  if (llvm::sys::fs::GetFileSystemTlsResult())
-    return false;
   return true;
   return true;
 }
 }
 
 

+ 0 - 18
tools/clang/unittests/HLSL/FileCheckForTest.cpp

@@ -1298,26 +1298,8 @@ bool ReadCheckFile(SourceMgr &SM,
   return false;
   return false;
 }
 }
 
 
-// This would typically be done in DLL load.
-struct GlobalPerThreadSys {
-  bool success;
-
-  GlobalPerThreadSys() {
-    success = ::llvm::sys::fs::SetupPerThreadFileSystem() ? false : true;
-
-  }
-  ~GlobalPerThreadSys() {
-    if (success)
-      ::llvm::sys::fs::CleanupPerThreadFileSystem();
-  }
-};
-
 int run_main() {
 int run_main() {
   // HLSL Change Starts
   // HLSL Change Starts
-  GlobalPerThreadSys gpts;
-  if (!gpts.success) {
-    return 1;
-  }
   llvm::sys::fs::MSFileSystem* msfPtr;
   llvm::sys::fs::MSFileSystem* msfPtr;
   HRESULT hr;
   HRESULT hr;
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))

+ 4 - 0
tools/clang/unittests/HLSL/VerifierTest.cpp

@@ -15,6 +15,7 @@
 #include "HLSLTestData.h"
 #include "HLSLTestData.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "dxc/Support/HLSLOptions.h"
 #include "dxc/Support/HLSLOptions.h"
+#include "llvm/Support/FileSystem.h"
 
 
 #include <fstream>
 #include <fstream>
 
 
@@ -131,6 +132,8 @@ public:
 };
 };
 
 
 bool TestModuleSetup() {
 bool TestModuleSetup() {
+  if (llvm::sys::fs::SetupPerThreadFileSystem())
+    return false;
   // Use this module-level function to set up LLVM dependencies.
   // Use this module-level function to set up LLVM dependencies.
   if (hlsl::options::initHlslOptTable()) {
   if (hlsl::options::initHlslOptTable()) {
     return false;
     return false;
@@ -143,6 +146,7 @@ bool TestModuleCleanup() {
   // In particular, clean up managed static allocations used by
   // In particular, clean up managed static allocations used by
   // parsing options with the LLVM library.
   // parsing options with the LLVM library.
   ::hlsl::options::cleanupHlslOptTable();
   ::hlsl::options::cleanupHlslOptTable();
+  llvm::sys::fs::CleanupPerThreadFileSystem();
   ::llvm::llvm_shutdown();
   ::llvm::llvm_shutdown();
   return true;
   return true;
 }
 }

+ 3 - 0
tools/clang/unittests/dxc_batch/dxc_batch.cpp

@@ -832,6 +832,9 @@ int DxcBatchContext::BatchCompile(bool bMultiThread, bool bLibLink) {
 int __cdecl wmain(int argc, const wchar_t **argv_) {
 int __cdecl wmain(int argc, const wchar_t **argv_) {
   const char *pStage = "Initialization";
   const char *pStage = "Initialization";
   int retVal = 0;
   int retVal = 0;
+  if (llvm::sys::fs::SetupPerThreadFileSystem())
+    return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   try {
   try {
     auto t_start = std::chrono::high_resolution_clock::now();
     auto t_start = std::chrono::high_resolution_clock::now();
 
 

+ 1 - 0
tools/clang/utils/TableGen/TableGen.cpp

@@ -252,6 +252,7 @@ int main(int argc, char **argv) {
   HRESULT hr;
   HRESULT hr;
   if (std::error_code ec = llvm::sys::fs::SetupPerThreadFileSystem())
   if (std::error_code ec = llvm::sys::fs::SetupPerThreadFileSystem())
       return 1;
       return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))
     return 1;
     return 1;
   std::unique_ptr<llvm::sys::fs::MSFileSystem> msf(msfPtr);
   std::unique_ptr<llvm::sys::fs::MSFileSystem> msf(msfPtr);

+ 1 - 0
utils/TableGen/TableGen.cpp

@@ -182,6 +182,7 @@ int main(int argc, char **argv) {
   // HLSL Change Starts
   // HLSL Change Starts
   if (std::error_code ec = llvm::sys::fs::SetupPerThreadFileSystem())
   if (std::error_code ec = llvm::sys::fs::SetupPerThreadFileSystem())
       return 1;
       return 1;
+  llvm::sys::fs::AutoCleanupPerThreadFileSystem auto_cleanup_fs;
   llvm::sys::fs::MSFileSystem* msfPtr;
   llvm::sys::fs::MSFileSystem* msfPtr;
   HRESULT hr;
   HRESULT hr;
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))
   if (!SUCCEEDED(hr = CreateMSFileSystemForDisk(&msfPtr)))