implement stage 1, remove dxil_dll_path and selectValidator

Author: bob80905

include/dxc/Support/HLSLOptions.td

@@ -317,8 +317,6 @@ def print_before_all : Flag<["-", "/"], "print-before-all">, Group<hlslcomp_Grou
   HelpText<"Print LLVM IR before each pass.">;
 def print_before : Separate<["-", "/"], "print-before">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
   HelpText<"Print LLVM IR before a specific pass. May be specificied multiple times.">;
-def dxil_dll_path : Separate<["-", "/"], "dxil-dll-path">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
-  HelpText<"Specify an absolute path to a DXIL.dll file to set the metadata version and validate the DXIL output.">;
 def print_after_all : Flag<["-", "/"], "print-after-all">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
   HelpText<"Print LLVM IR after each pass.">;
 def print_after : Separate<["-", "/"], "print-after">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,

include/dxc/Support/dxcapi.use.h

@@ -13,7 +13,10 @@
 #define __DXCAPI_USE_H__
 
 #include "dxc/dxcapi.h"
+#include <cstdlib>     // for getenv
 #include <string>
+#include <filesystem>  // C++17 and later
+#include <dxc/Support/Global.h> // for hresult handling with DXC_FAILED
 
 namespace dxc {
 
@@ -26,17 +29,15 @@ class DxcDllSupport {
   HMODULE m_dll;
   DxcCreateInstanceProc m_createFn;
   DxcCreateInstance2Proc m_createFn2;
-  std::string DxilDLLPath = "";
 
-  HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName) {
+  HRESULT virtual InitializeInternal(LPCSTR dllName, LPCSTR fnName) {
     if (m_dll != nullptr)
       return S_OK;
 
 #ifdef _WIN32
     m_dll = LoadLibraryA(dllName);
     if (m_dll == nullptr)
       return HRESULT_FROM_WIN32(GetLastError());
-
     m_createFn = (DxcCreateInstanceProc)GetProcAddress(m_dll, fnName);
 
     if (m_createFn == nullptr) {
@@ -77,8 +78,6 @@ class DxcDllSupport {
   }
 
 public:
-  LPCSTR GetDxilDLLPath() { return DxilDLLPath.data(); }
-  void SetDxilDLLPath(LPCSTR p) { DxilDLLPath = p; }
   DxcDllSupport() : m_dll(nullptr), m_createFn(nullptr), m_createFn2(nullptr) {}
 
   DxcDllSupport(DxcDllSupport &&other) {
@@ -90,14 +89,13 @@ class DxcDllSupport {
     other.m_createFn2 = nullptr;
   }
 
-  ~DxcDllSupport() { Cleanup(); }
+  virtual ~DxcDllSupport() { Cleanup(); }
 
-  HRESULT Initialize() {
-    // load dxcompiler.dll
+  HRESULT virtual Initialize() {
     return InitializeInternal(kDxCompilerLib, "DxcCreateInstance");
   }
 
-  HRESULT InitializeForDll(LPCSTR dll, LPCSTR entryPoint) {
+  HRESULT virtual InitializeForDll(LPCSTR dll, LPCSTR entryPoint) {
     return InitializeInternal(dll, entryPoint);
   }
 
@@ -106,7 +104,8 @@ class DxcDllSupport {
     return CreateInstance(clsid, __uuidof(TInterface), (IUnknown **)pResult);
   }
 
-  HRESULT CreateInstance(REFCLSID clsid, REFIID riid, IUnknown **pResult) {
+  HRESULT virtual CreateInstance(REFCLSID clsid, REFIID riid,
+                                 IUnknown **pResult) {
     if (pResult == nullptr)
       return E_POINTER;
     if (m_dll == nullptr)
@@ -122,7 +121,7 @@ class DxcDllSupport {
                            (IUnknown **)pResult);
   }
 
-  HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
+  HRESULT virtual CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
                           IUnknown **pResult) {
     if (pResult == nullptr)
       return E_POINTER;
@@ -134,11 +133,21 @@ class DxcDllSupport {
     return hr;
   }
 
-  bool HasCreateWithMalloc() const { return m_createFn2 != nullptr; }
+  bool virtual HasCreateWithMalloc() const { return m_createFn2 != nullptr; }
+
+  bool virtual IsEnabled() const { return m_dll != nullptr; }
 
-  bool IsEnabled() const { return m_dll != nullptr; }
+  bool virtual GetCreateInstanceProcs(DxcCreateInstanceProc *pCreateFn,
+                              DxcCreateInstance2Proc *pCreateFn2) const {
+    if (pCreateFn == nullptr || pCreateFn2 == nullptr ||
+         m_createFn == nullptr)
+        return false;
+    *pCreateFn = m_createFn;
+    *pCreateFn2 = m_createFn2;
+    return true;
+  }
 
-  void Cleanup() {
+  void virtual Cleanup() {
     if (m_dll != nullptr) {
       m_createFn = nullptr;
       m_createFn2 = nullptr;
@@ -151,7 +160,7 @@ class DxcDllSupport {
     }
   }
 
-  HMODULE Detach() {
+  HMODULE virtual Detach() {
     HMODULE hModule = m_dll;
     m_dll = nullptr;
     return hModule;
@@ -184,6 +193,56 @@ void WriteOperationErrorsToConsole(IDxcOperationResult *pResult,
 void WriteOperationResultToConsole(IDxcOperationResult *pRewriteResult,
                                    bool outputWarnings);
 
+class DxcDllExtValidationSupport : public DxcDllSupport {
+  // this instance of DxcDllSupport manages the lifetime of
+  // dxil.dll
+  DxcDllSupport *m_DxilSupport = nullptr;
+
+  std::string DxilDLLPathExt = "";
+  bool InitializationSuccess = false;
+  // override DxcDllSupport's implementation of InitializeInternal,
+  // adding the environment variable value check for a path to a dxil.dll
+  // for external validation
+  HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName){
+
+    // Load dxcompiler.dll
+    HRESULT result = m_DxilSupport->InitializeForDll(dllName, fnName);
+    InitializationSuccess = DXC_FAILED(result) ? false : true;
+    if (!InitializationSuccess){
+      return result;
+    }
+
+    // now handle internal or external dxil.dll
+    const char *envVal = std::getenv("DXC_DXIL_DLL_PATH");
+    bool ValidateInternally = false;
+    if (!envVal || std::string(envVal).empty()) {
+      ValidateInternally = true;
+    }
+
+    if (!ValidateInternally){
+      std::string DllPathStr(envVal);
+      DxilDLLPathExt = DllPathStr;
+      std::filesystem::path DllPath(DllPathStr);
+
+      // Check if path is absolute and exists
+      if (!DllPath.is_absolute() || !std::filesystem::exists(DllPath)) {
+        InitializationSuccess = false;
+        // TODO: Ideally emit some diagnostic that the given absolute path doesn't exist
+        return HRESULT_FROM_WIN32(GetLastError());
+      }
+      result = m_DxilSupport->InitializeForDll(DllPathStr.data(), fnName);
+      if (DXC_FAILED(result)) {
+        InitializationSuccess = false;
+      }
+    }
+    // nothing to do if we are validating internally, dxcompiler.dll
+    // is loaded and it'll take care of validation.
+    return InitializationSuccess;
+  }
+
+  std::string GetDxilDLLPathExt() { return DxilDLLPathExt; }
+
+};
 } // namespace dxc
 
 #endif

lib/DxcSupport/HLSLOptions.cpp

@@ -25,7 +25,6 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
-#include <filesystem>
 #include <optional>
 
 using namespace llvm::opt;
@@ -410,15 +409,6 @@ LangStd parseHLSLVersion(llvm::StringRef Ver) {
 }
 namespace options {
 
-bool AbsolutePathExists(const std::filesystem::path &p) {
-  // First check if it is an absolute path
-  if (!p.is_absolute()) {
-    return false;
-  }
-  // Then check if it exists
-  return std::filesystem::exists(p);
-}
-
 /// Reads all options from the given argument strings, populates opts, and
 /// validates reporting errors and warnings.
 int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
@@ -1043,19 +1033,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
     opts.ValVerMinor = (unsigned long)minor64;
   }
 
-  llvm::StringRef dxilDLLPathStr = Args.getLastArgValue(OPT_dxil_dll_path);
-  std::filesystem::path p = dxilDLLPathStr.str();
-
-  // when the dxil_dll_path argument has an empty string,
-  // then the internal validator should be used
-  // otherwise, check that this path exists and is absolute
-  if (dxilDLLPathStr.str() == "" || AbsolutePathExists(p)) {
-    opts.DxilDLLPath = dxilDLLPathStr.str();
-  } else {
-    errors << "Absolute path \"" << dxilDLLPathStr << "\" does not exist.";
-    return 1;
-  }
-
   if (opts.IsLibraryProfile() && Minor == 0xF) {
     if (opts.ValVerMajor != UINT_MAX && opts.ValVerMajor != 0) {
       errors << "Offline library profile cannot be used with non-zero "
@@ -1418,8 +1395,6 @@ int SetupDxcDllSupport(const DxcOpts &opts, dxc::DxcDllSupport &dxcSupport,
       return 1;
     }
   }
-
-  dxcSupport.SetDxilDLLPath(opts.DxilDLLPath.data());
   return 0;
 }
 

tools/clang/test/DXC/Inputs/dxil_dll_path.hlsl

@@ -1448,7 +1448,6 @@ int dxc::main(int argc, const char **argv_) {
       SetUnhandledExceptionFilter(ExceptionFilter);
 #endif
 
-    // Setup dxcompiler DLL.
     {
       std::string dllErrorString;
       llvm::raw_string_ostream dllErrorStream(dllErrorString);
@@ -1462,7 +1461,6 @@ int dxc::main(int argc, const char **argv_) {
     }
 
     EnsureEnabled(dxcSupport);
-
     DxcContext context(dxcOpts, dxcSupport);
     // Handle help request, which overrides any other processing.
     if (dxcOpts.ShowHelp) {

tools/clang/tools/dxclib/dxc.cpp

@@ -109,12 +109,6 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD Reason, LPVOID reserved) {
     ::hlsl::options::cleanupHlslOptTable();
     ::llvm::sys::fs::CleanupPerThreadFileSystem();
     ::llvm::llvm_shutdown();
-    if (reserved ==
-        NULL) { // FreeLibrary has been called or the DLL load failed
-      DxilLibCleanup(DxilLibCleanUpType::UnloadLibrary);
-    } else { // Process termination. We should not call FreeLibrary()
-      DxilLibCleanup(DxilLibCleanUpType::ProcessTermination);
-    }
     DxcClearThreadMalloc();
     DxcCleanupThreadMalloc();
     DxcEtw_DXCompilerShutdown_Stop(S_OK);

tools/clang/tools/dxcompiler/DXCompiler.cpp

@@ -60,11 +60,7 @@ HRESULT CreateDxcContainerBuilder(REFIID riid, _Out_ LPVOID *ppv) {
   // Call dxil.dll's containerbuilder
   *ppv = nullptr;
   const char *warning;
-  HRESULT hr = DxilLibInitialize();
-  if (FAILED(hr)) {
-    warning = "Unable to load dxil.dll.\n";
-  }
-  hr = DxilLibCreateInstance(CLSID_DxcContainerBuilder,
+  HRESULT hr = DxilLibCreateInstance(CLSID_DxcContainerBuilder,
                              (IDxcContainerBuilder **)ppv);
   if (FAILED(hr)) {
     warning = "Unable to create container builder from dxil.dll. Resulting "

tools/clang/tools/dxcompiler/dxcapi.cpp

@@ -850,12 +850,9 @@ class DxcCompiler : public IDxcCompiler3,
           compiler.getCodeGenOpts().HLSLValidatorMajorVer = opts.ValVerMajor;
           compiler.getCodeGenOpts().HLSLValidatorMinorVer = opts.ValVerMinor;
         } else {
-          // Version internal validator, or dxil dll if specified
-          // with dxil_dll_path option
           dxcutil::GetValidatorVersion(
               &compiler.getCodeGenOpts().HLSLValidatorMajorVer,
-              &compiler.getCodeGenOpts().HLSLValidatorMinorVer,
-              opts.DxilDLLPath);
+              &compiler.getCodeGenOpts().HLSLValidatorMinorVer);
         }
 
         // Root signature-only container validation is only supported on 1.5 and
@@ -935,7 +932,7 @@ class DxcCompiler : public IDxcCompiler3,
             CComPtr<IDxcBlobEncoding> pValErrors;
             // Validation failure communicated through diagnostic error
             dxcutil::ValidateRootSignatureInContainer(
-                pOutputBlob, &compiler.getDiagnostics(), opts.DxilDLLPath);
+                pOutputBlob, &compiler.getDiagnostics());
           }
         }
       } else if (opts.VerifyDiagnostics) {
@@ -1055,8 +1052,7 @@ class DxcCompiler : public IDxcCompiler3,
               std::move(serializeModule), pOutputBlob, m_pMalloc,
               SerializeFlags, pOutputStream, 0, opts.GetPDBName(),
               &compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream,
-              pRootSigStream, pRootSignatureBlob, pPrivateBlob,
-              opts.DxilDLLPath);
+              pRootSigStream, pRootSignatureBlob, pPrivateBlob);
 
           inputs.pVersionInfo = static_cast<IDxcVersionInfo *>(this);
 
@@ -1109,8 +1105,7 @@ class DxcCompiler : public IDxcCompiler3,
                 CComPtr<IDxcBlobEncoding> pValErrors;
                 // Validation failure communicated through diagnostic error
                 dxcutil::ValidateRootSignatureInContainer(
-                    pRootSignature, &compiler.getDiagnostics(),
-                    opts.DxilDLLPath);
+                    pRootSignature, &compiler.getDiagnostics());
               }
               IFT(pResult->SetOutputObject(DXC_OUT_ROOT_SIGNATURE,
                                            pRootSignature));