NFC: remove dead external validation code paths from dxcompiler (microsoft#7451)

bob80905 · github-actions[bot] · web-flow · commit 77b2ff676070 · 2025-06-05T13:45:30.000-07:00
DXC has now been changed to use the internal validator (loaded by
dxcompiler.dll) by default. This PR removes the ability for dxc.exe to
load dxil.dll in preparation for a series of changes to fix external
validation handling.

---------

Co-authored-by: github-actions[bot] &lt;github-actions[bot]@users.noreply.github.com&gt;
diff --git a/include/dxc/DxilContainer/DxcContainerBuilder.h b/include/dxc/DxilContainer/DxcContainerBuilder.h
@@ -45,8 +45,7 @@ class DxcContainerBuilder : public IDxcContainerBuilder {
     return DoBasicQueryInterface<IDxcContainerBuilder>(this, riid, ppvObject);
   }
 
-  void Init(const char *warning = nullptr) {
-    m_warning = warning;
+  void Init() {
     m_RequireValidation = false;
     m_HasPrivateData = false;
     m_HashFunction = nullptr;
@@ -67,7 +66,6 @@ class DxcContainerBuilder : public IDxcContainerBuilder {
 
   PartList m_parts;
   CComPtr<IDxcBlob> m_pContainer;
-  const char *m_warning;
   bool m_RequireValidation;
   bool m_HasPrivateData;
   // Function to compute hash when valid dxil container is built
diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h
@@ -114,13 +114,6 @@ struct RewriterOpts {
   bool DeclGlobalCB = false;          // OPT_rw_decl_global_cb
 };
 
-enum class ValidatorSelection : int {
-  Auto,        // Force internal validator (even if DXIL.dll is present)
-  Internal,    // Force internal validator (even if DXIL.dll is present)
-  External,    // Use DXIL.dll, failing compilation if not available
-  Invalid = -1 // Invalid
-};
-
 /// Use this class to capture all options.
 class DxcOpts {
 public:
@@ -225,8 +218,6 @@ class DxcOpts {
   bool ResMayAlias = false;                  // OPT_res_may_alias
   unsigned long ValVerMajor = UINT_MAX,
                 ValVerMinor = UINT_MAX; // OPT_validator_version
-  ValidatorSelection SelectValidator =
-      ValidatorSelection::Auto;         // OPT_select_validator
   unsigned ScanLimit = 0;               // OPT_memdep_block_scan_limit
   bool ForceZeroStoreLifetimes = false; // OPT_force_zero_store_lifetimes
   bool EnableLifetimeMarkers = false;   // OPT_enable_lifetime_markers
diff --git a/lib/DxilContainer/DxcContainerBuilder.cpp b/lib/DxilContainer/DxcContainerBuilder.cpp
@@ -146,18 +146,14 @@ DxcContainerBuilder::SerializeContainer(IDxcOperationResult **ppResult) {
     // Combine existing warnings and errors from validation
     CComPtr<IDxcBlobEncoding> pErrorBlob;
     CDxcMallocHeapPtr<char> errorHeap(m_pMalloc);
-    SIZE_T warningLength = m_warning ? strlen(m_warning) : 0;
-    SIZE_T valErrorLength =
+    SIZE_T totalErrorLength =
         pValErrorUtf8 ? pValErrorUtf8->GetStringLength() : 0;
-    SIZE_T totalErrorLength = warningLength + valErrorLength;
     if (totalErrorLength) {
       SIZE_T errorSizeInBytes = totalErrorLength + 1;
       errorHeap.AllocateBytes(errorSizeInBytes);
-      if (warningLength)
-        memcpy(errorHeap.m_pData, m_warning, warningLength);
-      if (valErrorLength)
-        memcpy(errorHeap.m_pData + warningLength,
-               pValErrorUtf8->GetStringPointer(), valErrorLength);
+
+      memcpy(errorHeap.m_pData, pValErrorUtf8->GetStringPointer(),
+             totalErrorLength);
       errorHeap.m_pData[totalErrorLength] = L'\0';
       IFT(hlsl::DxcCreateBlobWithEncodingOnMalloc(errorHeap.m_pData, m_pMalloc,
                                                   errorSizeInBytes, DXC_CP_UTF8,
diff --git a/tools/clang/tools/dxcompiler/CMakeLists.txt b/tools/clang/tools/dxcompiler/CMakeLists.txt
@@ -57,7 +57,6 @@ set(SOURCES
   DXCompiler.rc
   DXCompiler.def
   dxcfilesystem.cpp
-  dxillib.cpp
   dxcutil.cpp
   dxcdisassembler.cpp
   dxcpdbutils.cpp
@@ -75,7 +74,6 @@ set(SOURCES
   dxcutil.cpp
   dxcdisassembler.cpp
   dxcpdbutils.cpp
-  dxillib.cpp
   dxcvalidator.cpp
   dxclinker.cpp
   dxcshadersourceinfo.cpp
diff --git a/tools/clang/tools/dxcompiler/DXCompiler.cpp b/tools/clang/tools/dxcompiler/DXCompiler.cpp
@@ -19,7 +19,6 @@
 #ifdef LLVM_ON_WIN32
 #include "dxcetw.h"
 #endif
-#include "dxillib.h"
 
 namespace hlsl {
 HRESULT SetupRegistryPassForHLSL();
@@ -65,7 +64,6 @@ static HRESULT InitMaybeFail() throw() {
   fsSetup = true;
   IFC(hlsl::SetupRegistryPassForHLSL());
   IFC(hlsl::SetupRegistryPassForPIX());
-  IFC(DxilLibInitialize());
   if (hlsl::options::initHlslOptTable()) {
     hr = E_FAIL;
     goto Cleanup;
@@ -110,12 +108,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);
diff --git a/tools/clang/tools/dxcompiler/dxcapi.cpp b/tools/clang/tools/dxcompiler/dxcapi.cpp
@@ -25,7 +25,6 @@
 #include "dxcetw.h"
 #endif
 #include "dxc/DxilContainer/DxcContainerBuilder.h"
-#include "dxillib.h"
 #include <memory>
 
 HRESULT CreateDxcCompiler(REFIID riid, _Out_ LPVOID *ppv);
@@ -59,20 +58,11 @@ HRESULT CreateDxcContainerReflection(REFIID riid, _Out_ LPVOID *ppv) {
 HRESULT CreateDxcContainerBuilder(REFIID riid, _Out_ LPVOID *ppv) {
   // Call dxil.dll's containerbuilder
   *ppv = nullptr;
-  const char *warning;
-  HRESULT hr = DxilLibCreateInstance(CLSID_DxcContainerBuilder,
-                                     (IDxcContainerBuilder **)ppv);
-  if (FAILED(hr)) {
-    warning = "Unable to create container builder from dxil.dll. Resulting "
-              "container will not be signed.\n";
-  } else {
-    return hr;
-  }
 
   CComPtr<DxcContainerBuilder> Result =
       DxcContainerBuilder::Alloc(DxcGetThreadMallocNoRef());
   IFROOM(Result.p);
-  Result->Init(warning);
+  Result->Init();
   return Result->QueryInterface(riid, ppv);
 }
 
diff --git a/tools/clang/tools/dxcompiler/dxcassembler.cpp b/tools/clang/tools/dxcompiler/dxcassembler.cpp
@@ -19,7 +19,6 @@
 #include "dxc/Support/dxcfilesystem.h"
 #include "dxc/Support/microcom.h"
 #include "dxcutil.h"
-#include "dxillib.h"
 
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/LLVMContext.h"
diff --git a/tools/clang/tools/dxcompiler/dxclinker.cpp b/tools/clang/tools/dxcompiler/dxclinker.cpp
@@ -18,7 +18,6 @@
 #include "dxc/Support/dxcapi.impl.h"
 #include "dxc/Support/microcom.h"
 #include "dxc/dxcapi.h"
-#include "dxillib.h"
 
 #include "llvm/ADT/SmallVector.h"
 #include <algorithm>
diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp
@@ -56,7 +56,6 @@
 #include "dxcompileradapter.h"
 #include "dxcshadersourceinfo.h"
 #include "dxcversion.inc"
-#include "dxillib.h"
 #include <algorithm>
 #include <cfloat>
 
@@ -850,11 +849,9 @@ class DxcCompiler : public IDxcCompiler3,
           compiler.getCodeGenOpts().HLSLValidatorMajorVer = opts.ValVerMajor;
           compiler.getCodeGenOpts().HLSLValidatorMinorVer = opts.ValVerMinor;
         } else {
-          // Version from dxil.dll, or internal validator if unavailable
           dxcutil::GetValidatorVersion(
               &compiler.getCodeGenOpts().HLSLValidatorMajorVer,
-              &compiler.getCodeGenOpts().HLSLValidatorMinorVer,
-              opts.SelectValidator);
+              &compiler.getCodeGenOpts().HLSLValidatorMinorVer);
         }
 
         // Root signature-only container validation is only supported on 1.5 and
@@ -934,7 +931,7 @@ class DxcCompiler : public IDxcCompiler3,
             CComPtr<IDxcBlobEncoding> pValErrors;
             // Validation failure communicated through diagnostic error
             dxcutil::ValidateRootSignatureInContainer(
-                pOutputBlob, &compiler.getDiagnostics(), opts.SelectValidator);
+                pOutputBlob, &compiler.getDiagnostics());
           }
         }
       } else if (opts.VerifyDiagnostics) {
@@ -1054,8 +1051,7 @@ class DxcCompiler : public IDxcCompiler3,
               std::move(serializeModule), pOutputBlob, m_pMalloc,
               SerializeFlags, pOutputStream, 0, opts.GetPDBName(),
               &compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream,
-              pRootSigStream, pRootSignatureBlob, pPrivateBlob,
-              opts.SelectValidator);
+              pRootSigStream, pRootSignatureBlob, pPrivateBlob);
 
           inputs.pVersionInfo = static_cast<IDxcVersionInfo *>(this);
 
@@ -1108,8 +1104,7 @@ class DxcCompiler : public IDxcCompiler3,
                 CComPtr<IDxcBlobEncoding> pValErrors;
                 // Validation failure communicated through diagnostic error
                 dxcutil::ValidateRootSignatureInContainer(
-                    pRootSignature, &compiler.getDiagnostics(),
-                    opts.SelectValidator);
+                    pRootSignature, &compiler.getDiagnostics());
               }
               IFT(pResult->SetOutputObject(DXC_OUT_ROOT_SIGNATURE,
                                            pRootSignature));
@@ -1324,13 +1319,6 @@ class DxcCompiler : public IDxcCompiler3,
       CComPtr<IDxcResult> pResult;
       hr = e.hr;
       std::string msg("Internal Compiler error: ");
-      switch (hr) {
-      case DXC_E_VALIDATOR_MISSING:
-        msg = "Error: external validator selected, but DXIL.dll not found.";
-        break;
-      default:
-        break;
-      }
       msg += e.msg;
       if (SUCCEEDED(DxcResult::Create(
               e.hr, DXC_OUT_NONE,
diff --git a/tools/clang/tools/dxcompiler/dxcutil.cpp b/tools/clang/tools/dxcompiler/dxcutil.cpp
@@ -19,7 +19,6 @@
 #include "dxc/Support/WinIncludes.h"
 #include "dxc/Support/dxcapi.impl.h"
 #include "dxc/dxcapi.h"
-#include "dxillib.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/DebugInfo.h"
@@ -50,32 +49,8 @@ namespace {
 // AssembleToContainer helper functions.
 
 // return true if the internal validator was used, false otherwise
-bool CreateValidator(CComPtr<IDxcValidator> &pValidator,
-                     hlsl::options::ValidatorSelection SelectValidator =
-                         hlsl::options::ValidatorSelection::Auto) {
-  bool bInternal =
-      SelectValidator == hlsl::options::ValidatorSelection::Internal;
-  bool bExternal =
-      SelectValidator == hlsl::options::ValidatorSelection::External;
-  bool bAuto = SelectValidator == hlsl::options::ValidatorSelection::Auto;
-
-  // default behavior uses internal validator, as well as
-  // explicitly specifying internal
-  if (bInternal || bAuto) {
-    IFT(CreateDxcValidator(IID_PPV_ARGS(&pValidator)));
-    return true;
-  }
-
-  if (bExternal) {
-    // if external was explicitly specified, but no
-    // external validator could be found (no DXIL.dll), then error
-    IFTBOOL(DxilLibIsEnabled(), DXC_E_VALIDATOR_MISSING);
-    IFT(DxilLibCreateInstance(CLSID_DxcValidator, &pValidator));
-
-    return false;
-  }
-
-  return false;
+void CreateValidator(CComPtr<IDxcValidator> &pValidator) {
+  IFT(CreateDxcValidator(IID_PPV_ARGS(&pValidator)));
 }
 
 } // namespace
@@ -89,23 +64,20 @@ AssembleInputs::AssembleInputs(
     uint32_t ValidationFlags, llvm::StringRef DebugName,
     clang::DiagnosticsEngine *pDiag, hlsl::DxilShaderHash *pShaderHashOut,
     AbstractMemoryStream *pReflectionOut, AbstractMemoryStream *pRootSigOut,
-    CComPtr<IDxcBlob> pRootSigBlob, CComPtr<IDxcBlob> pPrivateBlob,
-    hlsl::options::ValidatorSelection SelectValidator)
+    CComPtr<IDxcBlob> pRootSigBlob, CComPtr<IDxcBlob> pPrivateBlob)
     : pM(std::move(pM)), pOutputContainerBlob(pOutputContainerBlob),
       pMalloc(pMalloc), SerializeFlags(SerializeFlags),
       ValidationFlags(ValidationFlags), pModuleBitcode(pModuleBitcode),
       DebugName(DebugName), pDiag(pDiag), pShaderHashOut(pShaderHashOut),
       pReflectionOut(pReflectionOut), pRootSigOut(pRootSigOut),
-      pRootSigBlob(pRootSigBlob), pPrivateBlob(pPrivateBlob),
-      SelectValidator(SelectValidator) {}
+      pRootSigBlob(pRootSigBlob), pPrivateBlob(pPrivateBlob) {}
 
-void GetValidatorVersion(unsigned *pMajor, unsigned *pMinor,
-                         hlsl::options::ValidatorSelection SelectValidator) {
+void GetValidatorVersion(unsigned *pMajor, unsigned *pMinor) {
   if (pMajor == nullptr || pMinor == nullptr)
     return;
 
   CComPtr<IDxcValidator> pValidator;
-  CreateValidator(pValidator, SelectValidator);
+  CreateValidator(pValidator);
 
   CComPtr<IDxcVersionInfo> pVersionInfo;
   if (SUCCEEDED(pValidator.QueryInterface(&pVersionInfo))) {
@@ -177,76 +149,19 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) {
   std::unique_ptr<llvm::Module> llvmModuleWithDebugInfo;
 
   CComPtr<IDxcValidator> pValidator;
-  bool bInternalValidator = CreateValidator(pValidator, inputs.SelectValidator);
-  // Warning on internal Validator
-
-  CComPtr<IDxcValidator2> pValidator2;
-  if (!bInternalValidator) {
-    pValidator.QueryInterface(&pValidator2);
-  }
-
-  if (bInternalValidator || pValidator2) {
-    // If using the internal validator or external validator supports
-    // IDxcValidator2, we'll use the modules directly. In this case, we'll want
-    // to make a clone to avoid SerializeDxilContainerForModule stripping all
-    // the debug info. The debug info will be stripped from the orginal module,
-    // but preserved in the cloned module.
-    if (llvm::getDebugMetadataVersionFromModule(*inputs.pM) != 0) {
-      llvmModuleWithDebugInfo.reset(llvm::CloneModule(inputs.pM.get()));
-    }
-  }
+  CreateValidator(pValidator);
 
-  // Verify validator version can validate this module
-  CComPtr<IDxcVersionInfo> pValidatorVersion;
-  IFT(pValidator->QueryInterface(&pValidatorVersion));
-  UINT32 ValMajor, ValMinor;
-  IFT(pValidatorVersion->GetVersion(&ValMajor, &ValMinor));
-  DxilModule &DM = inputs.pM.get()->GetOrCreateDxilModule();
-  unsigned ReqValMajor, ReqValMinor;
-  DM.GetValidatorVersion(ReqValMajor, ReqValMinor);
-  if (DXIL::CompareVersions(ValMajor, ValMinor, ReqValMajor, ReqValMinor) < 0) {
-    // Module is expecting to be validated by a newer validator.
-    if (inputs.pDiag) {
-      unsigned diagID = inputs.pDiag->getCustomDiagID(
-          clang::DiagnosticsEngine::Level::Error,
-          "The module cannot be validated by the version of the validator "
-          "currently attached.");
-      inputs.pDiag->Report(diagID);
-    }
-    return E_FAIL;
-  }
+  if (llvm::getDebugMetadataVersionFromModule(*inputs.pM) != 0)
+    llvmModuleWithDebugInfo.reset(llvm::CloneModule(inputs.pM.get()));
 
   AssembleToContainer(inputs);
 
   CComPtr<IDxcOperationResult> pValResult;
-  // Important: in-place edit is required so the blob is reused and thus
-  // dxil.dll can be released.
+  // In-place edit to avoid an extra copy
   inputs.ValidationFlags |= DxcValidatorFlags_InPlaceEdit;
-  if (bInternalValidator) {
-    IFT(RunInternalValidator(pValidator, llvmModuleWithDebugInfo.get(),
-                             inputs.pOutputContainerBlob,
-                             inputs.ValidationFlags, &pValResult));
-  } else {
-    if (pValidator2 && llvmModuleWithDebugInfo) {
-      // If metadata was stripped, re-serialize the input module.
-      CComPtr<AbstractMemoryStream> pDebugModuleStream;
-      IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pDebugModuleStream));
-      raw_stream_ostream outStream(pDebugModuleStream.p);
-      WriteBitcodeToFile(llvmModuleWithDebugInfo.get(), outStream, true);
-      outStream.flush();
-
-      DxcBuffer debugModule = {};
-      debugModule.Ptr = pDebugModuleStream->GetPtr();
-      debugModule.Size = pDebugModuleStream->GetPtrSize();
-
-      IFT(pValidator2->ValidateWithDebug(inputs.pOutputContainerBlob,
-                                         inputs.ValidationFlags, &debugModule,
-                                         &pValResult));
-    } else {
-      IFT(pValidator->Validate(inputs.pOutputContainerBlob,
-                               inputs.ValidationFlags, &pValResult));
-    }
-  }
+  IFT(RunInternalValidator(pValidator, llvmModuleWithDebugInfo.get(),
+                           inputs.pOutputContainerBlob, inputs.ValidationFlags,
+                           &pValResult));
   IFT(pValResult->GetStatus(&valHR));
   if (inputs.pDiag) {
     if (FAILED(valHR)) {
@@ -271,9 +186,8 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) {
   return valHR;
 }
 
-HRESULT ValidateRootSignatureInContainer(
-    IDxcBlob *pRootSigContainer, clang::DiagnosticsEngine *pDiag,
-    hlsl::options::ValidatorSelection SelectValidator) {
+HRESULT ValidateRootSignatureInContainer(IDxcBlob *pRootSigContainer,
+                                         clang::DiagnosticsEngine *pDiag) {
   HRESULT valHR = S_OK;
   CComPtr<IDxcValidator> pValidator;
   CComPtr<IDxcOperationResult> pValResult;
diff --git a/tools/clang/tools/dxcompiler/dxcutil.h b/tools/clang/tools/dxcompiler/dxcutil.h
diff --git a/tools/clang/tools/dxcompiler/dxillib.cpp b/tools/clang/tools/dxcompiler/dxillib.cpp
diff --git a/tools/clang/tools/dxcompiler/dxillib.h b/tools/clang/tools/dxcompiler/dxillib.h
diff --git a/tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp b/tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp
