Fix -validator-version issue, add tests

- default should be UINT_MAX, not zero, since zero is used
- compare validator version against target profile minimum for error
- add back implicit -Vd for lib_6_1/2 in CompileWithDebug since
  target profile is supplied outside options, so option validation
  would not know to fail without -Vd.
- add tests for various cases

include/dxc/Support/HLSLOptions.h

@@ -165,7 +165,7 @@ public:
   unsigned long AutoBindingSpace = UINT_MAX; // OPT_auto_binding_space
   bool ExportShadersOnly = false; // OPT_export_shaders_only
   bool ResMayAlias = false; // OPT_res_may_alias
-  unsigned long ValVerMajor = 0, ValVerMinor = 0; // OPT_validator_version
+  unsigned long ValVerMajor = UINT_MAX, ValVerMinor = UINT_MAX; // OPT_validator_version
 
   bool IsRootSignatureProfile();
   bool IsLibraryProfile();

include/dxc/Support/HLSLOptions.td

@@ -242,8 +242,8 @@ def export_shaders_only : Flag<["-", "/"], "export-shaders-only">, Group<hlslcom
   HelpText<"Only export shaders when compiling a library">;
 def default_linkage : Separate<["-", "/"], "default-linkage">, Group<hlslcomp_Group>, Flags<[CoreOption]>,
   HelpText<"Set default linkage for non-shader functions when compiling or linking to a library target (internal, external)">;
-def validator_version : Separate<["-", "/"], "validator-version">, Group<hlslcomp_Group>, Flags<[CoreOption]>,
-  HelpText<"Override validator version for module.  Format: <major>,<minor>.  Default: DXIL.dll version or current internal version.">;
+def validator_version : Separate<["-", "/"], "validator-version">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
+  HelpText<"Override validator version for module.  Format: <major.minor> ; Default: DXIL.dll version or current internal version.">;
 
 // SPIRV Change Starts
 def spirv : Flag<["-"], "spirv">, Group<spirv_Group>, Flags<[CoreOption, DriverOption]>,

lib/DxcSupport/HLSLOptions.cpp

@@ -669,7 +669,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   }
 
   if (opts.IsLibraryProfile() && Minor == 0xF) {
-    if (opts.ValVerMajor != 0) {
+    if (opts.ValVerMajor != UINT_MAX && opts.ValVerMajor != 0) {
       errors << "Offline library profile cannot be used with non-zero -validator-version.";
       return 1;
     }
@@ -690,7 +690,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
                 "targets.";
       return 1;
     }
-    if (opts.ValVerMajor != 0) {
+    if (opts.ValVerMajor != UINT_MAX && opts.ValVerMajor != 0) {
       errors << "non-zero -validator-version cannot be used with library profiles lib_6_1 or lib_6_2.";
       return 1;
     }

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -366,6 +366,22 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM)
     Diags.Report(DiagID) << CGM.getCodeGenOpts().HLSLProfile;
     return;
   }
+  if (CGM.getCodeGenOpts().HLSLValidatorMajorVer != 0) {
+    // Check validator version against minimum for target profile:
+    unsigned MinMajor, MinMinor;
+    SM->GetMinValidatorVersion(MinMajor, MinMinor);
+    if (DXIL::CompareVersions(CGM.getCodeGenOpts().HLSLValidatorMajorVer,
+                              CGM.getCodeGenOpts().HLSLValidatorMinorVer,
+                              MinMajor, MinMinor) < 0) {
+      DiagnosticsEngine &Diags = CGM.getDiags();
+      unsigned DiagID =
+          Diags.getCustomDiagID(DiagnosticsEngine::Error,
+            "validator version %0,%1 does not support target profile.");
+      Diags.Report(DiagID) << CGM.getCodeGenOpts().HLSLValidatorMajorVer
+                           << CGM.getCodeGenOpts().HLSLValidatorMinorVer;
+      return;
+    }
+  }
   m_bIsLib = SM->IsLib();
   // TODO: add AllResourceBound.
   if (CGM.getCodeGenOpts().HLSLAvoidControlFlow && !CGM.getCodeGenOpts().HLSLAllResourcesBound) {

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -616,7 +616,15 @@ public:
       bool produceFullContainer = !opts.CodeGenHighLevel && !opts.AstDump && !opts.OptDump && rootSigMajor == 0;
       bool needsValidation = produceFullContainer && !opts.DisableValidation;
 
-      if (opts.ValVerMajor > 0) {
+      // Disable validation for lib_6_1 and lib_6_2.
+      // This is needed for this API because the profile is provided separately,
+      // so the option parsing will not have verified that /Vd was provided.
+      if (compiler.getCodeGenOpts().HLSLProfile == "lib_6_1" ||
+          compiler.getCodeGenOpts().HLSLProfile == "lib_6_2") {
+        needsValidation = false;
+      }
+
+      if (opts.ValVerMajor != UINT_MAX) {
         // user-specified validator version override
         compiler.getCodeGenOpts().HLSLValidatorMajorVer = opts.ValVerMajor;
         compiler.getCodeGenOpts().HLSLValidatorMinorVer = opts.ValVerMinor;

tools/clang/unittests/HLSL/DxilModuleTest.cpp

@@ -61,6 +61,15 @@ public:
   TEST_METHOD(Precise5)
   TEST_METHOD(Precise6)
   TEST_METHOD(Precise7)
+
+  TEST_METHOD(SetValidatorVersion)
+
+  void VerifyValidatorVersionFails(
+    LPCWSTR shaderModel, const std::vector<LPCWSTR> &arguments,
+    const std::vector<LPCSTR> &expectedErrors);
+  void VerifyValidatorVersionMatches(
+    LPCWSTR shaderModel, const std::vector<LPCWSTR> &arguments,
+    unsigned expectedMajor = UINT_MAX, unsigned expectedMinor = UINT_MAX);
 };
 
 bool DxilModuleTest::InitSupport() {
@@ -150,7 +159,7 @@ public:
     return *DM;
   }
 
-private:
+public:
   static ::llvm::sys::fs::MSFileSystem *CreateMSFileSystem() {
     ::llvm::sys::fs::MSFileSystem *msfPtr;
     VERIFY_SUCCEEDED(CreateMSFileSystemForDisk(&msfPtr));
@@ -425,3 +434,85 @@ TEST_F(DxilModuleTest, Precise7) {
   }
   VERIFY_ARE_EQUAL(numChecks, 4);
 }
+
+void DxilModuleTest::VerifyValidatorVersionFails(
+    LPCWSTR shaderModel, const std::vector<LPCWSTR> &arguments,
+    const std::vector<LPCSTR> &expectedErrors) {
+
+  LPCSTR shader =
+    "[shader(\"pixel\")]"
+    "float4 main() : SV_Target {\n"
+    "  return 0;\n"
+    "}\n";
+
+  Compiler c(m_dllSupport);
+  c.Compile(shader, shaderModel, arguments, {});
+  CheckOperationResultMsgs(c.pCompileResult, expectedErrors, false, false);
+}
+
+void DxilModuleTest::VerifyValidatorVersionMatches(
+    LPCWSTR shaderModel, const std::vector<LPCWSTR> &arguments,
+    unsigned expectedMajor, unsigned expectedMinor) {
+
+  LPCSTR shader =
+    "[shader(\"pixel\")]"
+    "float4 main() : SV_Target {\n"
+    "  return 0;\n"
+    "}\n";
+
+  Compiler c(m_dllSupport);
+  c.Compile(shader, shaderModel, arguments, {});
+  DxilModule &DM = c.GetDxilModule();
+  unsigned vMajor, vMinor;
+  DM.GetValidatorVersion(vMajor, vMinor);
+
+  if (expectedMajor == UINT_MAX) {
+    // Expect current version
+    VERIFY_ARE_EQUAL(vMajor, c.m_ver.m_ValMajor);
+    VERIFY_ARE_EQUAL(vMinor, c.m_ver.m_ValMinor);
+  } else {
+    VERIFY_ARE_EQUAL(vMajor, expectedMajor);
+    VERIFY_ARE_EQUAL(vMinor, expectedMinor);
+  }
+}
+
+TEST_F(DxilModuleTest, SetValidatorVersion) {
+  Compiler c(m_dllSupport);
+  if (c.SkipDxil_Test(1, 4)) return;
+
+  // Current version
+  VerifyValidatorVersionMatches(L"ps_6_2", {});
+  VerifyValidatorVersionMatches(L"lib_6_3", {});
+
+  // Current version, with validation disabled
+  VerifyValidatorVersionMatches(L"ps_6_2", {L"-Vd"});
+  VerifyValidatorVersionMatches(L"lib_6_3", {L"-Vd"});
+
+  // Override validator version
+  VerifyValidatorVersionMatches(L"ps_6_2", {L"-validator-version", L"1,2"}, 1,2);
+  VerifyValidatorVersionMatches(L"lib_6_3", {L"-validator-version", L"1,3"}, 1,3);
+
+  // Override validator version, with validation disabled
+  VerifyValidatorVersionMatches(L"ps_6_2", {L"-Vd", L"-validator-version", L"1,2"}, 1,2);
+  VerifyValidatorVersionMatches(L"lib_6_3", {L"-Vd", L"-validator-version", L"1,3"}, 1,3);
+
+  // Never can validate (version 0,0):
+  VerifyValidatorVersionMatches(L"lib_6_1", {L"-Vd"}, 0, 0);
+  VerifyValidatorVersionMatches(L"lib_6_2", {L"-Vd"}, 0, 0);
+  VerifyValidatorVersionMatches(L"lib_6_2", {L"-Vd", L"-validator-version", L"0,0"}, 0, 0);
+  VerifyValidatorVersionMatches(L"lib_6_x", {}, 0, 0);
+  VerifyValidatorVersionMatches(L"lib_6_x", {L"-validator-version", L"0,0"}, 0, 0);
+
+  // Failure cases:
+  VerifyValidatorVersionFails(L"ps_6_2", {L"-validator-version", L"1,1"}, {
+    "validator version 1,1 does not support target profile."});
+
+  VerifyValidatorVersionFails(L"lib_6_2", {L"-Tlib_6_2"}, {
+    "Must disable validation for unsupported lib_6_1 or lib_6_2 targets"});
+
+  VerifyValidatorVersionFails(L"lib_6_2", {L"-Vd", L"-validator-version", L"1,2"}, {
+    "-validator-version cannot be used with library profiles lib_6_1 or lib_6_2."});
+
+  VerifyValidatorVersionFails(L"lib_6_x", {L"-validator-version", L"1,3"}, {
+    "Offline library profile cannot be used with non-zero -validator-version."});
+}