[clang] [HLSL] Add validation for the -enable-16bit-types option (PR #85340)

Joshua Batista via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 26 16:58:15 PDT 2024


https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/85340

>From 74a869a282d532ec426dbc1c954779ec2972aa5c Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 14 Mar 2024 17:04:12 -0700
Subject: [PATCH 01/19] add test

---
 .../clang/Basic/DiagnosticDriverKinds.td      |  3 +-
 clang/lib/Driver/ToolChains/HLSL.cpp          | 86 ++++++++++++++++---
 2 files changed, 74 insertions(+), 15 deletions(-)

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;
 }
 

>From f706462324cd511fca5e96c98be46ffc98d91cf4 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 14 Mar 2024 17:17:04 -0700
Subject: [PATCH 02/19] add helpful comment that a condition is unreachable

---
 clang/lib/Driver/ToolChains/HLSL.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index bf8fc42a27816c..0b7b101f4a8cb9 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -303,6 +303,8 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     }
 
     if (!parsedTargetProfile.has_value())
+      // This should be unreachable, target profile validation happens
+      // before this point.
       return DAL;
     else {
       if (parsedTargetProfile.value().Major < 6 ||

>From 06cb543abd7190ac4c83db85661d5d99d51e06d9 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 15 Mar 2024 12:53:47 -0700
Subject: [PATCH 03/19] add test, remove fixme comment

---
 clang/lib/Driver/ToolChains/HLSL.cpp          |  4 +---
 .../enable_16bit_types_validation.hlsl        | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Options/enable_16bit_types_validation.hlsl

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 0b7b101f4a8cb9..35422df2b34fea 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -278,9 +278,7 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
   if (!DAL->hasArg(options::OPT_O_Group)) {
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
   }
-  // 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;
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
new file mode 100644
index 00000000000000..ad97e68cc60abf
--- /dev/null
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -0,0 +1,19 @@
+// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 -HV 2016 %s 2>&1 -###   | FileCheck -check-prefix=both_invalid %s
+// RUN: not %clang_dxc -enable-16bit-types -T lib_6_4 -HV 2017 %s 2>&1 -###   | FileCheck -check-prefix=HV_invalid %s
+// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=TP_invalid %s
+// RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %ss
+
+
+// both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
+// HV_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
+// TP_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
+// valid: "dxil-unknown-shadermodel6.4-library"
+// valid-SAME: "-std=hlsl2021"
+// valid-SAME: "-fnative-half-type"
+
+[numthreads(1,1,1)]
+void main()
+{
+  return;
+}
+

>From 0d300b0bb10396d3fc391382079632d4bf108e84 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 18 Mar 2024 13:50:36 -0700
Subject: [PATCH 04/19] fix test typo

---
 clang/test/Options/enable_16bit_types_validation.hlsl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index ad97e68cc60abf..205383c01b5ae4 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -1,7 +1,7 @@
 // RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 -HV 2016 %s 2>&1 -###   | FileCheck -check-prefix=both_invalid %s
 // RUN: not %clang_dxc -enable-16bit-types -T lib_6_4 -HV 2017 %s 2>&1 -###   | FileCheck -check-prefix=HV_invalid %s
 // RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=TP_invalid %s
-// RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %ss
+// RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %s
 
 
 // both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021

>From 656bafbf79104f80f08267da9ba996838709d896 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 18 Mar 2024 15:41:14 -0700
Subject: [PATCH 05/19] move implementation to compilerinvocation, adjust test

---
 clang/lib/Driver/ToolChains/HLSL.cpp          | 88 +++----------------
 clang/lib/Frontend/CompilerInvocation.cpp     | 12 +++
 .../enable_16bit_types_validation.hlsl        |  6 +-
 3 files changed, 29 insertions(+), 77 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 35422df2b34fea..1169b5d8c92dd6 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -66,48 +66,15 @@ bool isLegalShaderModel(Triple &T) {
   return false;
 }
 
-struct ShaderModel {
-  StringRef TargetKind;
-  unsigned Major;
-  unsigned Minor;
-  bool OfflineLibMinor = false;
-};
-
-std::optional<ShaderModel> GetShaderModelFromString(StringRef Profile) {
+std::optional<std::string> tryParseProfile(StringRef Profile) {
+  // [ps|vs|gs|hs|ds|cs|ms|as]_[major]_[minor]
   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>(SM.value().TargetKind)
+      StringSwitch<Triple::EnvironmentType>(Parts[0])
           .Case("ps", Triple::EnvironmentType::Pixel)
           .Case("vs", Triple::EnvironmentType::Vertex)
           .Case("gs", Triple::EnvironmentType::Geometry)
@@ -121,11 +88,21 @@ 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(SM.value().Major, SM.value().Minor).getAsString());
+              VersionTuple(Major, Minor).getAsString());
   T.setEnvironment(Kind);
   if (isLegalShaderModel(T))
     return T.getTriple();
@@ -279,43 +256,6 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
   }
 
-  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())
-      // This should be unreachable, target profile validation happens
-      // before this point.
-      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;
 }
 
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7bd91d4791ecf0..cf5e6ee08f75b9 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4292,6 +4292,18 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
       }
+      // validate that if fnative-half-type is given, that
+      // the language standard is at least hlsl2021, and that
+      // the target shader model is at least 6.2
+      if (Args.getLastArg(OPT_fnative_half_type)) {
+        bool LangStdArgIsValid = Opts.LangStd >= LangStandard::lang_hlsl2021;
+        bool TPArgIsValid = T.getOSVersion() >= VersionTuple(6, 2);
+
+        // 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 (!(LangStdArgIsValid && TPArgIsValid))
+          Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
+      }
     } else
       Diags.Report(diag::err_drv_hlsl_unsupported_target) << T.str();
   }
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index 205383c01b5ae4..6d5d2e51e68b25 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -1,6 +1,6 @@
-// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 -HV 2016 %s 2>&1 -###   | FileCheck -check-prefix=both_invalid %s
-// RUN: not %clang_dxc -enable-16bit-types -T lib_6_4 -HV 2017 %s 2>&1 -###   | FileCheck -check-prefix=HV_invalid %s
-// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=TP_invalid %s
+// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 -HV 2016 %s 2>&1  | FileCheck -check-prefix=both_invalid %s
+// RUN: not %clang_dxc -enable-16bit-types -T lib_6_4 -HV 2017 %s 2>&1 | FileCheck -check-prefix=HV_invalid %s
+// RUN: not %clang_dxc -enable-16bit-types -T cs_6_0 /HV 2021 %s 2>&1  | FileCheck -check-prefix=TP_invalid %s
 // RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %s
 
 

>From 6797c3e7cfdfc44abf35c49f2d3c90c57bbee645 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 19 Mar 2024 10:43:26 -0700
Subject: [PATCH 06/19] adjust test SM

---
 clang/test/CodeGenHLSL/builtins/RWBuffer-elementtype.hlsl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CodeGenHLSL/builtins/RWBuffer-elementtype.hlsl b/clang/test/CodeGenHLSL/builtins/RWBuffer-elementtype.hlsl
index 87002ccd462d3f..036c9c28ef2779 100644
--- a/clang/test/CodeGenHLSL/builtins/RWBuffer-elementtype.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/RWBuffer-elementtype.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -finclude-default-header -fnative-half-type -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.2-compute -finclude-default-header -fnative-half-type -emit-llvm -o - %s | FileCheck %s
 
 RWBuffer<int16_t> BufI16;
 RWBuffer<uint16_t> BufU16;

>From 0fd5354625f49ad691f2a496b2b38d2dfafb5e61 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Mar 2024 15:56:45 -0700
Subject: [PATCH 07/19] Update
 clang/include/clang/Basic/DiagnosticDriverKinds.td

Co-authored-by: Damyan Pepper <damyanp at microsoft.com>
---
 clang/include/clang/Basic/DiagnosticDriverKinds.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index fae9132bd0a9c9..5bcc5666ac70dd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -754,7 +754,7 @@ def err_drv_hlsl_unsupported_target : Error<
 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">;
+  "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. "

>From d0cc4e9bc6268d47b997311de1e3c90ef17287cf Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Mar 2024 16:53:47 -0700
Subject: [PATCH 08/19] address Damyang

---
 clang/lib/Frontend/CompilerInvocation.cpp     | 20 ++++++++-----------
 .../enable_16bit_types_validation.hlsl        |  6 +++---
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index cf5e6ee08f75b9..a64146c823e6a2 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4284,6 +4284,14 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
           Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
               << ShaderModel << T.getOSName() << T.str();
         }
+        // validate that if fnative-half-type is given, that
+        // the language standard is at least hlsl2021, and that
+        // the target shader model is at least 6.2
+        if (Args.getLastArg(OPT_fnative_half_type)) {
+          if (!(Opts.LangStd >= LangStandard::lang_hlsl2021 &&
+                T.getOSVersion() >= VersionTuple(6, 2)))
+            Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
+        }
       } else if (T.isSPIRVLogical()) {
         if (!T.isVulkanOS() || T.getVulkanVersion() == VersionTuple(0)) {
           Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
@@ -4292,18 +4300,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
       }
-      // validate that if fnative-half-type is given, that
-      // the language standard is at least hlsl2021, and that
-      // the target shader model is at least 6.2
-      if (Args.getLastArg(OPT_fnative_half_type)) {
-        bool LangStdArgIsValid = Opts.LangStd >= LangStandard::lang_hlsl2021;
-        bool TPArgIsValid = T.getOSVersion() >= VersionTuple(6, 2);
-
-        // 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 (!(LangStdArgIsValid && TPArgIsValid))
-          Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
-      }
     } else
       Diags.Report(diag::err_drv_hlsl_unsupported_target) << T.str();
   }
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index 6d5d2e51e68b25..b833de2072b8d3 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -4,9 +4,9 @@
 // RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %s
 
 
-// both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
-// HV_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
-// TP_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and Hlsl Version [-HV] is >= 2021
+// both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// HV_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// TP_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
 // valid: "dxil-unknown-shadermodel6.4-library"
 // valid-SAME: "-std=hlsl2021"
 // valid-SAME: "-fnative-half-type"

>From 372a459f38d4aed478089167d0a3ff54b5518f8d Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 22 Mar 2024 12:57:01 -0700
Subject: [PATCH 09/19] add check for spirv

---
 clang/lib/Frontend/CompilerInvocation.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index a64146c823e6a2..1d1045f43059c5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4297,6 +4297,11 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
           Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
               << VulkanEnv << T.getOSName() << T.str();
         }
+        if (Args.getLastArg(OPT_fnative_half_type)) {
+          if (!(Opts.LangStd >= LangStandard::lang_hlsl2021))
+            Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
+                << VulkanEnv << T.getOSName() << T.str();
+        }
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
       }

>From cc21d79dc49c0a16d275e7ee153aa1a3dc7e426b Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 25 Mar 2024 11:46:30 -0700
Subject: [PATCH 10/19] spirv test try 1

---
 .../Options/enable_16bit_types_validation_spirv.hlsl   | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 clang/test/Options/enable_16bit_types_validation_spirv.hlsl

diff --git a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
new file mode 100644
index 00000000000000..5ed75dc0147000
--- /dev/null
+++ b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2016 -emit-llvm -disable-llvm-passes  -o - %s | FileCheck %s --check-prefix=SPIRV
+// kRUN: %clang_dxc -T lib_6_4 -HV 2016 -enable-16bit-types %s | FileCheck %s --check-prefix=SPIRV
+// SPIRV: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+
+[numthreads(1,1,1)]
+void main()
+{
+  return;
+}
+

>From 00b2c6195078a9c6f2d164bf74be71177bf611a9 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 14 Mar 2024 17:04:12 -0700
Subject: [PATCH 11/19] add test

---
 clang/lib/Driver/ToolChains/HLSL.cpp | 89 +++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 14 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 1169b5d8c92dd6..d14ab93e656aed 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();
@@ -256,6 +279,44 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
   }
 
+  // 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;
 }
 

>From 9df037d518b1e892d0666bab4aa03bfc8df50214 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 14 Mar 2024 17:17:04 -0700
Subject: [PATCH 12/19] add helpful comment that a condition is unreachable

---
 clang/lib/Driver/ToolChains/HLSL.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index d14ab93e656aed..04dd2b611bcb9d 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -304,6 +304,8 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     }
 
     if (!parsedTargetProfile.has_value())
+      // This should be unreachable, target profile validation happens
+      // before this point.
       return DAL;
     else {
       if (parsedTargetProfile.value().Major < 6 ||

>From e52704135b8b88175e438a8a14e8e65bd4d47172 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 15 Mar 2024 12:53:47 -0700
Subject: [PATCH 13/19] add test, remove fixme comment

---
 clang/lib/Driver/ToolChains/HLSL.cpp                  | 3 ---
 clang/test/Options/enable_16bit_types_validation.hlsl | 1 +
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 04dd2b611bcb9d..35422df2b34fea 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -279,9 +279,6 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
   }
 
-  // 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;
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index b833de2072b8d3..b7f293fb6cbc66 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -7,6 +7,7 @@
 // both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
 // HV_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
 // TP_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+
 // valid: "dxil-unknown-shadermodel6.4-library"
 // valid-SAME: "-std=hlsl2021"
 // valid-SAME: "-fnative-half-type"

>From f0121080e224874a12cb43e46eef2f087670df98 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 18 Mar 2024 15:41:14 -0700
Subject: [PATCH 14/19] move implementation to compilerinvocation, adjust test

---
 clang/lib/Driver/ToolChains/HLSL.cpp      | 88 ++++-------------------
 clang/lib/Frontend/CompilerInvocation.cpp | 12 ++++
 2 files changed, 26 insertions(+), 74 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 35422df2b34fea..1169b5d8c92dd6 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -66,48 +66,15 @@ bool isLegalShaderModel(Triple &T) {
   return false;
 }
 
-struct ShaderModel {
-  StringRef TargetKind;
-  unsigned Major;
-  unsigned Minor;
-  bool OfflineLibMinor = false;
-};
-
-std::optional<ShaderModel> GetShaderModelFromString(StringRef Profile) {
+std::optional<std::string> tryParseProfile(StringRef Profile) {
+  // [ps|vs|gs|hs|ds|cs|ms|as]_[major]_[minor]
   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>(SM.value().TargetKind)
+      StringSwitch<Triple::EnvironmentType>(Parts[0])
           .Case("ps", Triple::EnvironmentType::Pixel)
           .Case("vs", Triple::EnvironmentType::Vertex)
           .Case("gs", Triple::EnvironmentType::Geometry)
@@ -121,11 +88,21 @@ 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(SM.value().Major, SM.value().Minor).getAsString());
+              VersionTuple(Major, Minor).getAsString());
   T.setEnvironment(Kind);
   if (isLegalShaderModel(T))
     return T.getTriple();
@@ -279,43 +256,6 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
   }
 
-  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())
-      // This should be unreachable, target profile validation happens
-      // before this point.
-      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;
 }
 
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 1d1045f43059c5..24f4a460a66aa5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4305,6 +4305,18 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
       }
+      // validate that if fnative-half-type is given, that
+      // the language standard is at least hlsl2021, and that
+      // the target shader model is at least 6.2
+      if (Args.getLastArg(OPT_fnative_half_type)) {
+        bool LangStdArgIsValid = Opts.LangStd >= LangStandard::lang_hlsl2021;
+        bool TPArgIsValid = T.getOSVersion() >= VersionTuple(6, 2);
+
+        // 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 (!(LangStdArgIsValid && TPArgIsValid))
+          Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
+      }
     } else
       Diags.Report(diag::err_drv_hlsl_unsupported_target) << T.str();
   }

>From 019d4dc16d9d5742e24e48eb4ec9f42cb8db0959 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Mar 2024 16:53:47 -0700
Subject: [PATCH 15/19] address Damyang

---
 clang/lib/Frontend/CompilerInvocation.cpp | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 24f4a460a66aa5..1d1045f43059c5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4305,18 +4305,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
       }
-      // validate that if fnative-half-type is given, that
-      // the language standard is at least hlsl2021, and that
-      // the target shader model is at least 6.2
-      if (Args.getLastArg(OPT_fnative_half_type)) {
-        bool LangStdArgIsValid = Opts.LangStd >= LangStandard::lang_hlsl2021;
-        bool TPArgIsValid = T.getOSVersion() >= VersionTuple(6, 2);
-
-        // 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 (!(LangStdArgIsValid && TPArgIsValid))
-          Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
-      }
     } else
       Diags.Report(diag::err_drv_hlsl_unsupported_target) << T.str();
   }

>From 7648cd86ebfa06597aa0c0cd155f1348a7f1a5d4 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 25 Mar 2024 14:16:43 -0700
Subject: [PATCH 16/19] add new spirv diagnostic, update tests

---
 clang/include/clang/Basic/DiagnosticDriverKinds.td          | 6 ++++--
 clang/lib/Frontend/CompilerInvocation.cpp                   | 5 +++--
 clang/test/Options/enable_16bit_types_validation.hlsl       | 6 +++---
 clang/test/Options/enable_16bit_types_validation_spirv.hlsl | 6 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 5bcc5666ac70dd..529f04ee213d5d 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -753,8 +753,10 @@ 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_dxc_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_cc1_hlsl_spirv_fnative_half_type_option_invalid: Error<
+  "fnative-half-type option only valid when hlsl language standard version 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/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 1d1045f43059c5..0fcc48031698f3 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4290,7 +4290,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
         if (Args.getLastArg(OPT_fnative_half_type)) {
           if (!(Opts.LangStd >= LangStandard::lang_hlsl2021 &&
                 T.getOSVersion() >= VersionTuple(6, 2)))
-            Diags.Report(diag::err_drv_hlsl_enable_16bit_types_option_invalid);
+            Diags.Report(diag::err_drv_dxc_enable_16bit_types_option_invalid);
         }
       } else if (T.isSPIRVLogical()) {
         if (!T.isVulkanOS() || T.getVulkanVersion() == VersionTuple(0)) {
@@ -4299,7 +4299,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
         }
         if (Args.getLastArg(OPT_fnative_half_type)) {
           if (!(Opts.LangStd >= LangStandard::lang_hlsl2021))
-            Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
+            Diags.Report(
+                diag::err_drv_cc1_hlsl_spirv_fnative_half_type_option_invalid)
                 << VulkanEnv << T.getOSName() << T.str();
         }
       } else {
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index b7f293fb6cbc66..4ce6c3658837aa 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -4,9 +4,9 @@
 // RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %s
 
 
-// both_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
-// HV_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
-// TP_invalid: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// both_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// HV_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// TP_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
 
 // valid: "dxil-unknown-shadermodel6.4-library"
 // valid-SAME: "-std=hlsl2021"
diff --git a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
index 5ed75dc0147000..c422bcd047eb41 100644
--- a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2016 -emit-llvm -disable-llvm-passes  -o - %s | FileCheck %s --check-prefix=SPIRV
-// kRUN: %clang_dxc -T lib_6_4 -HV 2016 -enable-16bit-types %s | FileCheck %s --check-prefix=SPIRV
-// SPIRV: error: enable_16bit_types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// RUN: not %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2016 -fnative-half-type -emit-llvm -disable-llvm-passes  -o - %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+
+// SPIRV: error: fnative-half-type option only valid when hlsl language standard version is >= 2021
 
 [numthreads(1,1,1)]
 void main()

>From 56c059fa35bb1733d479c2664b90f9d5d1e24b55 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 25 Mar 2024 15:12:07 -0700
Subject: [PATCH 17/19] add positive test for spirv to make sure it works with
 high enough langstd

---
 clang/test/Options/enable_16bit_types_validation_spirv.hlsl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
index c422bcd047eb41..249a317ef84019 100644
--- a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
@@ -1,7 +1,11 @@
 // RUN: not %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2016 -fnative-half-type -emit-llvm -disable-llvm-passes  -o - %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// RUN: %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2021 -fnative-half-type -emit-llvm -disable-llvm-passes  -o - %s 2>&1 | FileCheck %s --check-prefix=valid
 
 // SPIRV: error: fnative-half-type option only valid when hlsl language standard version is >= 2021
 
+// valid: "spirv-unknown-vulkan-library"
+// valid: define spir_func void @main() #0 {
+
 [numthreads(1,1,1)]
 void main()
 {

>From 99f5f15b5f50b52319b1166fc9526fc411f39b1b Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 26 Mar 2024 13:20:31 -0700
Subject: [PATCH 18/19] address feedback

---
 .../clang/Basic/DiagnosticDriverKinds.td      |  4 ++--
 clang/lib/Frontend/CompilerInvocation.cpp     | 20 ++++++++++++-------
 .../enable_16bit_types_validation.hlsl        |  6 +++---
 .../enable_16bit_types_validation_spirv.hlsl  |  2 +-
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 529f04ee213d5d..d05f9b6e18a0af 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -754,9 +754,9 @@ def err_drv_hlsl_unsupported_target : Error<
 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_dxc_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">;
+  "'-enable-16bit-types' option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= hlsl2018, but shader model is '%0' and HLSL Version is '%1'">;
 def err_drv_cc1_hlsl_spirv_fnative_half_type_option_invalid: Error<
-  "fnative-half-type option only valid when hlsl language standard version is >= 2021">;
+  "'-fnative-half-type' option only valid when HLSL language standard version is >= 2018, but language standard version is '%0'">;
 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/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 0fcc48031698f3..61796af7804731 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4284,13 +4284,17 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
           Diags.Report(diag::err_drv_hlsl_bad_shader_unsupported)
               << ShaderModel << T.getOSName() << T.str();
         }
-        // validate that if fnative-half-type is given, that
-        // the language standard is at least hlsl2021, and that
-        // the target shader model is at least 6.2
+        // Validate that if fnative-half-type is given, that
+        // the language standard is at least hlsl2018, and that
+        // the target shader model is at least 6.2.
         if (Args.getLastArg(OPT_fnative_half_type)) {
-          if (!(Opts.LangStd >= LangStandard::lang_hlsl2021 &&
+          const LangStandard &Std =
+              LangStandard::getLangStandardForKind(Opts.LangStd);
+
+          if (!(Opts.LangStd >= LangStandard::lang_hlsl2018 &&
                 T.getOSVersion() >= VersionTuple(6, 2)))
-            Diags.Report(diag::err_drv_dxc_enable_16bit_types_option_invalid);
+            Diags.Report(diag::err_drv_dxc_enable_16bit_types_option_invalid)
+                << T.getOSVersion().getAsString() << Std.getName();
         }
       } else if (T.isSPIRVLogical()) {
         if (!T.isVulkanOS() || T.getVulkanVersion() == VersionTuple(0)) {
@@ -4298,10 +4302,12 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
               << VulkanEnv << T.getOSName() << T.str();
         }
         if (Args.getLastArg(OPT_fnative_half_type)) {
-          if (!(Opts.LangStd >= LangStandard::lang_hlsl2021))
+          const LangStandard &Std =
+              LangStandard::getLangStandardForKind(Opts.LangStd);
+          if (!(Opts.LangStd >= LangStandard::lang_hlsl2018))
             Diags.Report(
                 diag::err_drv_cc1_hlsl_spirv_fnative_half_type_option_invalid)
-                << VulkanEnv << T.getOSName() << T.str();
+                << Std.getName();
         }
       } else {
         llvm_unreachable("expected DXIL or SPIR-V target");
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index 4ce6c3658837aa..06341b06ce501b 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -4,9 +4,9 @@
 // RUN: %clang_dxc -enable-16bit-types -T lib_6_4 /HV 2021 %s 2>&1 -###   | FileCheck -check-prefix=valid %s
 
 
-// both_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
-// HV_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
-// TP_invalid: error: enable-16bit-types option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= 2021
+// both_invalid: error: '-enable-16bit-types' option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= hlsl2018, but shader model is '6.0' and HLSL Version is 'hlsl2016'
+// HV_invalid: error: '-enable-16bit-types' option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= hlsl2018, but shader model is '6.4' and HLSL Version is 'hlsl2017'
+// TP_invalid: error: '-enable-16bit-types' option only valid when target shader model [-T] is >= 6.2 and HLSL Version [-HV] is >= hlsl2018, but shader model is '6.0' and HLSL Version is 'hlsl2021'
 
 // valid: "dxil-unknown-shadermodel6.4-library"
 // valid-SAME: "-std=hlsl2021"
diff --git a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
index 249a317ef84019..d1aa9895f62349 100644
--- a/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation_spirv.hlsl
@@ -1,7 +1,7 @@
 // RUN: not %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2016 -fnative-half-type -emit-llvm -disable-llvm-passes  -o - %s 2>&1 | FileCheck %s --check-prefix=SPIRV
 // RUN: %clang_cc1 -internal-isystem D:\llvm-project\build\x64-Release\lib\clang\19\include -nostdsysteminc -triple spirv-vulkan-library -x hlsl -std=hlsl2021 -fnative-half-type -emit-llvm -disable-llvm-passes  -o - %s 2>&1 | FileCheck %s --check-prefix=valid
 
-// SPIRV: error: fnative-half-type option only valid when hlsl language standard version is >= 2021
+// SPIRV: error: '-fnative-half-type' option only valid when HLSL language standard version is >= 2018, but language standard version is 'hlsl2016'
 
 // valid: "spirv-unknown-vulkan-library"
 // valid: define spir_func void @main() #0 {

>From 429c1c3eaba1e934db41711e3c2c1de6cbb99639 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 26 Mar 2024 16:58:02 -0700
Subject: [PATCH 19/19] Update
 clang/include/clang/Basic/DiagnosticDriverKinds.td

Co-authored-by: Chris B <cbieneman at microsoft.com>
---
 clang/include/clang/Basic/DiagnosticDriverKinds.td | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index d05f9b6e18a0af..3710faf25c7fee 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -753,10 +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_dxc_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 >= hlsl2018, but shader model is '%0' and HLSL Version is '%1'">;
-def err_drv_cc1_hlsl_spirv_fnative_half_type_option_invalid: Error<
-  "'-fnative-half-type' option only valid when HLSL language standard version is >= 2018, but language standard version is '%0'">;
+def err_drv_hlsl_16bit_types_unsupported: Error<
+  "'%0' option requires target HLSL Version >= 2018 and %select{|shader model >= 6.2}1, but HLSL Version is '%2'" %select{|and shader model is '%3'}1>;
 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. "



More information about the cfe-commits mailing list