Addressing code review feedback

- remove unused "class." parsing
- use llvm_unreachable instead of DXASSERT
- remove priority 2 from WaveMatrix execution tests
- use StringSwitch in validator arg parsing
- remove unused lit configs

include/dxc/Support/HLSLOptions.h

@@ -115,9 +115,10 @@ struct RewriterOpts {
 };
 
 enum class ValidatorSelection : int {
-  Auto,     // Try DXIL.dll; fallback to internal validator
-  Internal, // Force internal validator (even if DXIL.dll is present)
-  External  // Use DXIL.dll, failing compilation if not available
+  Auto,        // Try DXIL.dll; fallback to internal validator
+  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.

lib/DXIL/DxilUtil.cpp

@@ -626,7 +626,6 @@ bool IsHLSLResourceDescType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -645,7 +644,6 @@ bool IsHLSLNodeOutputType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -661,7 +659,6 @@ bool IsHLSLNodeOutputArrayType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -678,7 +675,6 @@ bool IsHLSLEmptyNodeOutputType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -694,7 +690,6 @@ bool IsHLSLEmptyNodeOutputArrayType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -710,7 +705,6 @@ bool IsHLSLNodeInputRecordType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -732,7 +726,6 @@ bool IsHLSLNodeEmptyInputRecordType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -748,7 +741,6 @@ bool IsHLSLNodeEmptyOutputRecordType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.
@@ -764,7 +756,6 @@ bool IsHLSLRWNodeInputRecordType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     if (name.startswith("RWDispatchNodeInputRecord<") ||
@@ -781,7 +772,6 @@ bool IsHLSLNodeOutputRecordType(llvm::Type *Ty) {
       return false;
     StringRef name = ST->getName();
 
-    ConsumePrefix(name, "class.");
     ConsumePrefix(name, "struct.");
 
     // TODO: don't check names.

lib/DxcSupport/HLSLOptions.cpp

@@ -975,13 +975,12 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
 
   llvm::StringRef valSelectStr = Args.getLastArgValue(OPT_select_validator);
   if (!valSelectStr.empty()) {
-    if (valSelectStr.equals_lower("auto")) {
-      opts.SelectValidator = ValidatorSelection::Auto;
-    } else if (valSelectStr.equals_lower("internal")) {
-      opts.SelectValidator = ValidatorSelection::Internal;
-    } else if (valSelectStr.equals_lower("external")) {
-      opts.SelectValidator = ValidatorSelection::External;
-    } else {
+    opts.SelectValidator = llvm::StringSwitch<ValidatorSelection>(valSelectStr)
+     .Case("auto", ValidatorSelection::Auto)
+     .Case("internal", ValidatorSelection::Internal)
+     .Case("external", ValidatorSelection::External)
+     .Default(ValidatorSelection::Invalid);
+    if (opts.SelectValidator == ValidatorSelection::Invalid) {
       errors << "Unsupported value '" << valSelectStr
              << "for -select-validator option.";
       return 1;

test/DXILValidation/lit.local.cfg

@@ -1,3 +0,0 @@
-# Enable .ll suffixes for this directory
-# They are disabled above to avoid tests unsupported by HLSL
-config.suffixes = ['.ll']

test/lit.cfg

@@ -224,9 +224,6 @@ else:
 # also have a post-assertion to not match a trailing hyphen (foo-).
 NOJUNK = r"(?<!\.|-|\^|/)"
 
-config.substitutions.append( ('%dxv',
-                            lit.util.which('dxv', llvm_tools_dir)) )
-
 # HLSL Change begin - disabling tools we don't build
 for pattern in [#r"\bbugpoint\b(?!-)",
                 #NOJUNK + r"\bllc\b",

tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp

@@ -774,7 +774,7 @@ void AddAnnotateWaveMatrix(HLModule &HLM,
       IRBuilder<> Builder(dxilutil::FindAllocaInsertionPt(Arg->getParent()));
       CreateAnnotateWaveMatrix(HLM, V, WMP, Builder);
     } else {
-      DXASSERT(false, "WaveMatrix value is unexpected type");
+      llvm_unreachable("WaveMatrix value is unexpected type");
     }
   }
 }

tools/clang/lib/Frontend/ASTUnit.cpp

@@ -2017,8 +2017,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
       if (Diags->setSeverityForGroup(diag::Flavor::WarningOrError,
                                      StringRef(groupNames[i]),
                                      diag::Severity::Ignored)) {
-        assert(false &&
-               "otherwise, there is a problem with diagnostic definitions.");
+        llvm_unreachable("there is a problem with diagnostic definitions.");
         return nullptr;
       }
     }

tools/clang/unittests/HLSLExec/ExecutionTest.cpp

@@ -531,17 +531,9 @@ public:
                        L"Table:ShaderOpArithTable.xml#PackUnpackOpTable")
   END_TEST_METHOD()
 
-  BEGIN_TEST_METHOD(WaveMatrixLoadStoreTests)
-  TEST_METHOD_PROPERTY(L"Priority", L"2")
-  END_TEST_METHOD()
-
-  BEGIN_TEST_METHOD(WaveMatrixScalarTests)
-  TEST_METHOD_PROPERTY(L"Priority", L"2")
-  END_TEST_METHOD()
-
-  BEGIN_TEST_METHOD(WaveMatrixMathTests)
-  TEST_METHOD_PROPERTY(L"Priority", L"2")
-  END_TEST_METHOD()
+  TEST_METHOD(WaveMatrixLoadStoreTests);
+  TEST_METHOD(WaveMatrixScalarTests);
+  TEST_METHOD(WaveMatrixMathTests);
 
   dxc::DxcDllSupport m_support;