self review

Author: bob80905

include/dxc/Support/HLSLOptions.h

@@ -218,7 +218,6 @@ class DxcOpts {
   bool ResMayAlias = false;                  // OPT_res_may_alias
   unsigned long ValVerMajor = UINT_MAX,
                 ValVerMinor = UINT_MAX; // OPT_validator_version
-  std::string DxilDLLPath = "";         // OPT_dxil_dll_path
   unsigned ScanLimit = 0;               // OPT_memdep_block_scan_limit
   bool ForceZeroStoreLifetimes = false; // OPT_force_zero_store_lifetimes
   bool EnableLifetimeMarkers = false;   // OPT_enable_lifetime_markers

include/dxc/Support/dxcapi.use.h

@@ -194,25 +194,28 @@ void WriteOperationResultToConsole(IDxcOperationResult *pRewriteResult,
                                    bool outputWarnings);
 
 class DxcDllExtValidationSupport : public DxcDllSupport {
-  // this instance of DxcDllSupport manages the lifetime of
-  // dxil.dll
+  // DxcDllExtValidationSupport manages the
+  // lifetime of dxcompiler.dll, while the member, m_DxilSupport,
+  // manages the lifetime of dxil.dll
   DxcDllSupport *m_DxilSupport = nullptr;
 
   std::string DxilDLLPathExt = "";
-  bool InitializationSuccess = false;
+  bool ExternalDxilDLLInitializationSuccess = 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){
+    HRESULT result = InitializeForDll(dllName, fnName);
+    // if dxcompiler.dll fails to load, then we won't try loading dxil.dll,
+    // so set this boolean to false.
+    ExternalDxilDLLInitializationSuccess = DXC_FAILED(result) ? false : true;
+    if (!ExternalDxilDLLInitializationSuccess) {
       return result;
     }
 
-    // now handle internal or external dxil.dll
+    // now handle external dxil.dll
     const char *envVal = std::getenv("DXC_DXIL_DLL_PATH");
     bool ValidateInternally = false;
     if (!envVal || std::string(envVal).empty()) {
@@ -226,18 +229,23 @@ class DxcDllExtValidationSupport : public DxcDllSupport {
 
       // Check if path is absolute and exists
       if (!DllPath.is_absolute() || !std::filesystem::exists(DllPath)) {
-        InitializationSuccess = false;
+        ExternalDxilDLLInitializationSuccess = 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;
+        ExternalDxilDLLInitializationSuccess = false;
+        return result;
       }
+      ExternalDxilDLLInitializationSuccess = true;
+    } else {
+      // nothing to do if we are validating internally, dxcompiler.dll
+      // is loaded and it'll take care of validation.
+      // No external dxil.dll will be loaded in this case
+      ExternalDxilDLLInitializationSuccess = false;
     }
-    // nothing to do if we are validating internally, dxcompiler.dll
-    // is loaded and it'll take care of validation.
-    return InitializationSuccess;
+    return S_OK;
   }
 
   std::string GetDxilDLLPathExt() { return DxilDLLPathExt; }

tools/clang/tools/dxclib/dxc.cpp

@@ -1448,6 +1448,7 @@ int dxc::main(int argc, const char **argv_) {
       SetUnhandledExceptionFilter(ExceptionFilter);
 #endif
 
+    // Setup a helper DLL.
     {
       std::string dllErrorString;
       llvm::raw_string_ostream dllErrorStream(dllErrorString);

tools/clang/tools/dxcompiler/dxcapi.cpp

@@ -61,7 +61,7 @@ HRESULT CreateDxcContainerBuilder(REFIID riid, _Out_ LPVOID *ppv) {
   *ppv = nullptr;
   const char *warning;
   HRESULT hr = DxilLibCreateInstance(CLSID_DxcContainerBuilder,
-                             (IDxcContainerBuilder **)ppv);
+                                     (IDxcContainerBuilder **)ppv);
   if (FAILED(hr)) {
     warning = "Unable to create container builder from dxil.dll. Resulting "
               "container will not be signed.\n";

tools/clang/tools/dxcompiler/dxcutil.cpp

@@ -60,8 +60,8 @@ bool CreateValidator(CComPtr<IDxcValidator> &pValidator) {
     return true;
   }
 
-  // otherwise, use the external validator provided by the dxil_dll_path
-  // argument
+  // otherwise, use the external validator provided by
+  // the environment variable
   else {
     // this code loads the dll path on an as-needed basis.
     // if a request to initialize the dxil lib arrives more than once