[HLSL] Add validation for the -enable-16bit-types option

[bob80905]

Previously, the clang compiler with the dxc driver would accept the -enable-16bit-types flag without checking to see if the required conditions are met for proper processing of the flag.
Specifically, -enable-16bit-types requires a shader model of at least 6.2 and an HLSL version of at least 2021.
This PR adds a validation check for these other options having the required values, and emits an error if these constraints are not met.
Fixes #57876

[llvmbot]

@llvm/pr-subscribers-hlsl
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Joshua Batista (bob80905)

Changes

Previously, the clang compiler with the dxc driver would accept the -enable-16bit-types flag without checking to see if the required conditions are met for proper processing of the flag.
Specifically, -enable-16bit-types requires a shader model of at least 6.2 and an HLSL version of at least 2021.
This PR adds a validation check for these other options having the required values, and emits an error if these constraints are not met.
Fixes #57876

---

Full diff: https://github.com/llvm/llvm-project/pull/85340.diff

2 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+2-1)
• (modified) clang/lib/Driver/ToolChains/HLSL.cpp (+72-14)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index e33a1f4c45b949..fae9132bd0a9c9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -753,7 +753,8 @@ def err_drv_hlsl_unsupported_target : Error<
   "HLSL code generation is unsupported for target '%0'">;
 def err_drv_hlsl_bad_shader_required_in_target : Error<
   "%select{shader model|Vulkan environment|shader stage}0 is required as %select{OS|environment}1 in target '%2' for HLSL code generation">;
-
+def err_drv_hlsl_enable_16bit_types_option_invalid: Error<
+  "enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021">;
 def err_drv_hlsl_bad_shader_unsupported : Error<
   "%select{shader model|Vulkan environment|shader stage}0 '%1' in target '%2' is invalid for HLSL code generation">;
 def warn_drv_dxc_missing_dxv : Warning<"dxv not found. "
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 05aac9caa7fb29..bf8fc42a27816c 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -66,15 +66,48 @@ bool isLegalShaderModel(Triple &T) {
   return false;
 }
 
-std::optional<std::string> tryParseProfile(StringRef Profile) {
-  // [ps|vs|gs|hs|ds|cs|ms|as]_[major]_[minor]
+struct ShaderModel {
+  StringRef TargetKind;
+  unsigned Major;
+  unsigned Minor;
+  bool OfflineLibMinor = false;
+};
+
+std::optional<ShaderModel> GetShaderModelFromString(StringRef Profile) {
   SmallVector<StringRef, 3> Parts;
   Profile.split(Parts, "_");
   if (Parts.size() != 3)
     return std::nullopt;
 
+  unsigned long long Major = 0;
+  if (llvm::getAsUnsignedInteger(Parts[1], 0, Major))
+    return std::nullopt;
+
+  unsigned long long Minor = 0;
+  bool isOfflineLibMinor = false;
+  if (Parts[0] == "lib" && Parts[2] == "x")
+    isOfflineLibMinor = true;
+  else if (llvm::getAsUnsignedInteger(Parts[2], 0, Minor))
+    return std::nullopt;
+
+  ShaderModel ret;
+  ret.TargetKind = Parts[0];
+  ret.Major = Major;
+  ret.Minor = Minor;
+  ret.OfflineLibMinor = isOfflineLibMinor;
+
+  return ret;
+}
+
+std::optional<std::string> tryParseProfile(StringRef Profile) {
+  std::optional<ShaderModel> SM = GetShaderModelFromString(Profile);
+  if (!SM.has_value()) {
+    return std::nullopt;
+  }
+  // [ps|vs|gs|hs|ds|cs|ms|as]_[major]_[minor]
+
   Triple::EnvironmentType Kind =
-      StringSwitch<Triple::EnvironmentType>(Parts[0])
+      StringSwitch<Triple::EnvironmentType>(SM.value().TargetKind)
           .Case("ps", Triple::EnvironmentType::Pixel)
           .Case("vs", Triple::EnvironmentType::Vertex)
           .Case("gs", Triple::EnvironmentType::Geometry)
@@ -88,21 +121,11 @@ std::optional<std::string> tryParseProfile(StringRef Profile) {
   if (Kind == Triple::EnvironmentType::UnknownEnvironment)
     return std::nullopt;
 
-  unsigned long long Major = 0;
-  if (llvm::getAsUnsignedInteger(Parts[1], 0, Major))
-    return std::nullopt;
-
-  unsigned long long Minor = 0;
-  if (Parts[2] == "x" && Kind == Triple::EnvironmentType::Library)
-    Minor = OfflineLibMinor;
-  else if (llvm::getAsUnsignedInteger(Parts[2], 0, Minor))
-    return std::nullopt;
-
   // dxil-unknown-shadermodel-hull
   llvm::Triple T;
   T.setArch(Triple::ArchType::dxil);
   T.setOSName(Triple::getOSTypeName(Triple::OSType::ShaderModel).str() +
-              VersionTuple(Major, Minor).getAsString());
+              VersionTuple(SM.value().Major, SM.value().Minor).getAsString());
   T.setEnvironment(Kind);
   if (isLegalShaderModel(T))
     return T.getTriple();
@@ -258,6 +281,41 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
   // FIXME: add validation for enable_16bit_types should be after HLSL 2018 and
   // shader model 6.2.
   // See: https://github.com/llvm/llvm-project/issues/57876
+  if (DAL->hasArg(options::OPT_fnative_half_type)) {
+
+    bool HVArgIsValid = true;
+    bool TPArgIsValid = true;
+
+    const StringRef HVArg =
+        DAL->getLastArgValue(options::OPT_std_EQ, "hlsl2021");
+
+    const StringRef TPArg =
+        DAL->getLastArgValue(options::OPT_target_profile, "");
+    std::optional<ShaderModel> parsedTargetProfile =
+        GetShaderModelFromString(TPArg);
+
+    unsigned long long HV_year;
+    StringRef HV_year_str = HVArg.drop_front(4);
+    if (HV_year_str != "202x") {
+      llvm::getAsUnsignedInteger(HV_year_str, 0, HV_year);
+      if (HV_year < 2021)
+        HVArgIsValid = false;
+    }
+
+    if (!parsedTargetProfile.has_value())
+      return DAL;
+    else {
+      if (parsedTargetProfile.value().Major < 6 ||
+          (parsedTargetProfile.value().Major == 6 &&
+           parsedTargetProfile.value().Minor < 2))
+        TPArgIsValid = false;
+    }
+
+    // if the HLSL Version is not at least 2021, or the shader model is not at
+    // least 6.2, then enable-16bit-types is an invalid flag.
+    if (!(HVArgIsValid && TPArgIsValid))
+      getDriver().Diag(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
+  }
   return DAL;
 }
 

[damyanp]

I'm worried about what this'll do when T.isSPIRVLogical().

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[bogner]

When printing option flags in diagnostics they're usually in single quotes and include the leading dash, like '-enable-16bit-types' option is only valid...

[llvm-beanz]

One nit, but otherwise looks good.