[clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)

Finn Plummer via cfe-commits cfe-commits at lists.llvm.org
Wed May 28 10:36:21 PDT 2025


https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140181

>From 1c40e031725f1bd21c5c0d24d333781f99bd2a0a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 23:14:10 +0000
Subject: [PATCH 01/18] pre-req: add missing token to Lexer

---
 clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def | 3 +++
 clang/unittests/Lex/LexHLSLRootSignatureTest.cpp        | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
index ddebe82987197..5d16eaa5b72f6 100644
--- a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
+++ b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
@@ -100,6 +100,9 @@ KEYWORD(flags)
 KEYWORD(numDescriptors)
 KEYWORD(offset)
 
+// StaticSampler Keywords:
+KEYWORD(mipLODBias)
+
 // Unbounded Enum:
 UNBOUNDED_ENUM(unbounded, "unbounded")
 
diff --git a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
index 3e38c281f4fb1..b610b8f10f8da 100644
--- a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
@@ -136,6 +136,8 @@ TEST_F(LexHLSLRootSignatureTest, ValidLexAllTokensTest) {
     space visibility flags
     numDescriptors offset
 
+    mipLODBias
+
     unbounded
     DESCRIPTOR_RANGE_OFFSET_APPEND
 

>From f3ace332f9984dfefea388b9bc26da0e7643c6c5 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 23:15:04 +0000
Subject: [PATCH 02/18] pre-req: add parsing of MipLODBias as an uint

- defines a float data member of StaticSampler that will be used to test
functionality of parsing a float
---
 .../clang/Parse/ParseHLSLRootSignature.h      |  1 +
 clang/lib/Parse/ParseHLSLRootSignature.cpp    | 21 +++++++++++++++++++
 .../Parse/ParseHLSLRootSignatureTest.cpp      |  3 ++-
 .../llvm/Frontend/HLSL/HLSLRootSignature.h    |  1 +
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index da17388a2aea5..2ec806c380b37 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -111,6 +111,7 @@ class RootSignatureParser {
 
   struct ParsedStaticSamplerParams {
     std::optional<llvm::hlsl::rootsig::Register> Reg;
+    std::optional<float> MipLODBias;
   };
   std::optional<ParsedStaticSamplerParams> parseStaticSamplerParams();
 
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index a37505e360fc0..d16644f990847 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -376,6 +376,10 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
 
   Sampler.Reg = Params->Reg.value();
 
+  // Fill in optional values
+  if (Params->MipLODBias.has_value())
+    Sampler.MipLODBias = Params->MipLODBias.value();
+
   if (consumeExpectedToken(TokenKind::pu_r_paren,
                            diag::err_hlsl_unexpected_end_of_params,
                            /*param of=*/TokenKind::kw_StaticSampler))
@@ -661,6 +665,23 @@ RootSignatureParser::parseStaticSamplerParams() {
         return std::nullopt;
       Params.Reg = Reg;
     }
+
+    // `mipLODBias` `=` NUMBER
+    if (tryConsumeExpectedToken(TokenKind::kw_mipLODBias)) {
+      if (Params.MipLODBias.has_value()) {
+        getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+            << CurToken.TokKind;
+        return std::nullopt;
+      }
+
+      if (consumeExpectedToken(TokenKind::pu_equal))
+        return std::nullopt;
+
+      auto MipLODBias = parseUIntParam();
+      if (!MipLODBias.has_value())
+        return std::nullopt;
+      Params.MipLODBias = (float)*MipLODBias;
+    }
   } while (tryConsumeExpectedToken(TokenKind::pu_comma));
 
   return Params;
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index cd3b12dbe4d24..c7dae1c27ceeb 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -225,7 +225,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
 
 TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   const llvm::StringLiteral Source = R"cc(
-    StaticSampler(s0)
+    StaticSampler(s0, mipLODBias = 0)
   )cc";
 
   TrivialModuleLoader ModLoader;
@@ -247,6 +247,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.ViewType, RegisterType::SReg);
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.Number, 0u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0u);
 
   ASSERT_TRUE(Consumer->isSatisfied());
 }
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index f9de86b567fea..fea9a9962991c 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -157,6 +157,7 @@ raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause);
 
 struct StaticSampler {
   Register Reg;
+  float MipLODBias = 0.f;
 };
 
 /// Models RootElement : RootFlags | RootConstants | RootParam

>From 9be00bde6ea222beff5b4f401d8fae87fd597697 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 15 May 2025 23:42:27 +0000
Subject: [PATCH 03/18] add support for parsing signed integer for float param

---
 .../clang/Parse/ParseHLSLRootSignature.h      |  4 ++
 clang/lib/Parse/ParseHLSLRootSignature.cpp    | 51 ++++++++++++++++++-
 .../Parse/ParseHLSLRootSignatureTest.cpp      | 37 +++++++++++++-
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 2ec806c380b37..9d6c9e2225bcf 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -118,6 +118,7 @@ class RootSignatureParser {
   // Common parsing methods
   std::optional<uint32_t> parseUIntParam();
   std::optional<llvm::hlsl::rootsig::Register> parseRegister();
+  std::optional<float> parseFloatParam();
 
   /// Parsing methods of various enums
   std::optional<llvm::hlsl::rootsig::ShaderVisibility> parseShaderVisibility();
@@ -129,6 +130,9 @@ class RootSignatureParser {
   /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
   /// 32-bit integer
   std::optional<uint32_t> handleUIntLiteral();
+  /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
+  /// 32-bit integer
+  std::optional<int32_t> handleIntLiteral(bool Negated);
 
   /// Flags may specify the value of '0' to denote that there should be no
   /// flags set.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index d16644f990847..c4b77f41b2942 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -677,7 +677,7 @@ RootSignatureParser::parseStaticSamplerParams() {
       if (consumeExpectedToken(TokenKind::pu_equal))
         return std::nullopt;
 
-      auto MipLODBias = parseUIntParam();
+      auto MipLODBias = parseFloatParam();
       if (!MipLODBias.has_value())
         return std::nullopt;
       Params.MipLODBias = (float)*MipLODBias;
@@ -730,6 +730,30 @@ std::optional<Register> RootSignatureParser::parseRegister() {
   return Reg;
 }
 
+std::optional<float> RootSignatureParser::parseFloatParam() {
+  assert(CurToken.TokKind == TokenKind::pu_equal &&
+         "Expects to only be invoked starting at given keyword");
+  // Consume sign modifier
+  bool Signed = tryConsumeExpectedToken({TokenKind::pu_plus, TokenKind::pu_minus});
+  bool Negated = Signed && CurToken.TokKind == TokenKind::pu_minus;
+
+  // Handle an uint and interpret it as a float
+  if (!Signed && tryConsumeExpectedToken(TokenKind::int_literal)) {
+    auto UInt = handleUIntLiteral();
+    if (!UInt.has_value())
+      return std::nullopt;
+    return (float)UInt.value();
+  } else if (tryConsumeExpectedToken(TokenKind::int_literal)) {
+    auto Int = handleIntLiteral(Negated);
+    if (!Int.has_value())
+      return std::nullopt;
+
+    return (float)Int.value();
+  }
+
+  return std::nullopt;
+}
+
 std::optional<llvm::hlsl::rootsig::ShaderVisibility>
 RootSignatureParser::parseShaderVisibility() {
   assert(CurToken.TokKind == TokenKind::pu_equal &&
@@ -856,6 +880,31 @@ std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
   return Val.getExtValue();
 }
 
+std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
+  // Parse the numeric value and do semantic checks on its specification
+  clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc,
+                                      PP.getSourceManager(), PP.getLangOpts(),
+                                      PP.getTargetInfo(), PP.getDiagnostics());
+  if (Literal.hadError)
+    return true; // Error has already been reported so just return
+
+  assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits");
+
+  llvm::APSInt Val = llvm::APSInt(32, true);
+  if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
+    // Report that the value has overflowed
+    PP.getDiagnostics().Report(CurToken.TokLoc,
+                               diag::err_hlsl_number_literal_overflow)
+        << 0 << CurToken.NumSpelling;
+    return std::nullopt;
+  }
+
+  if (Negated)
+    return static_cast<int32_t>((-Val).getExtValue());
+
+  return static_cast<int32_t>(Val.getExtValue());
+}
+
 bool RootSignatureParser::verifyZeroFlag() {
   assert(CurToken.TokKind == TokenKind::int_literal);
   auto X = handleUIntLiteral();
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c7dae1c27ceeb..ecb7d0715eaad 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -247,7 +247,42 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.ViewType, RegisterType::SReg);
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.Number, 0u);
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0u);
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = 0),
+    StaticSampler(s0, mipLODBias = +1),
+    StaticSampler(s0, mipLODBias = -1)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test no diagnostics produced
+  Consumer->setNoDiag();
+
+  ASSERT_FALSE(Parser.parse());
+
+  RootElement Elem = Elements[0];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
+
+  Elem = Elements[1];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 1.f);
+
+  Elem = Elements[2];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -1.f);
 
   ASSERT_TRUE(Consumer->isSatisfied());
 }

>From d4608c342d2872fb48195231a29507f5efe7a162 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 16 May 2025 00:20:46 +0000
Subject: [PATCH 04/18] add support for parsing a float for float param

---
 .../clang/Parse/ParseHLSLRootSignature.h      |  4 +-
 clang/lib/Parse/ParseHLSLRootSignature.cpp    | 60 ++++++++++++++++---
 .../Parse/ParseHLSLRootSignatureTest.cpp      | 42 ++++++++++++-
 3 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 9d6c9e2225bcf..dcdc7deab68c9 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -130,9 +130,11 @@ class RootSignatureParser {
   /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
   /// 32-bit integer
   std::optional<uint32_t> handleUIntLiteral();
-  /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
+  /// Use NumericLiteralParser to convert CurToken.NumSpelling into a signed
   /// 32-bit integer
   std::optional<int32_t> handleIntLiteral(bool Negated);
+  /// Use NumericLiteralParser to convert CurToken.NumSpelling into a float
+  std::optional<float> handleFloatLiteral(bool Negated);
 
   /// Flags may specify the value of '0' to denote that there should be no
   /// flags set.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index c4b77f41b2942..05636a48af534 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <float.h>
+
 #include "clang/Parse/ParseHLSLRootSignature.h"
 
 #include "clang/Lex/LiteralSupport.h"
@@ -734,7 +736,8 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
   assert(CurToken.TokKind == TokenKind::pu_equal &&
          "Expects to only be invoked starting at given keyword");
   // Consume sign modifier
-  bool Signed = tryConsumeExpectedToken({TokenKind::pu_plus, TokenKind::pu_minus});
+  bool Signed =
+      tryConsumeExpectedToken({TokenKind::pu_plus, TokenKind::pu_minus});
   bool Negated = Signed && CurToken.TokKind == TokenKind::pu_minus;
 
   // Handle an uint and interpret it as a float
@@ -747,8 +750,12 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
     auto Int = handleIntLiteral(Negated);
     if (!Int.has_value())
       return std::nullopt;
-
     return (float)Int.value();
+  } else if (tryConsumeExpectedToken(TokenKind::float_literal)) {
+    auto Float = handleFloatLiteral(Negated);
+    if (!Float.has_value())
+      return std::nullopt;
+    return Float.value();
   }
 
   return std::nullopt;
@@ -864,9 +871,10 @@ std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
                                       PP.getSourceManager(), PP.getLangOpts(),
                                       PP.getTargetInfo(), PP.getDiagnostics());
   if (Literal.hadError)
-    return true; // Error has already been reported so just return
+    return std::nullopt; // Error has already been reported so just return
 
-  assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits");
+  assert(Literal.isIntegerLiteral() &&
+         "NumSpelling can only consist of digits");
 
   llvm::APSInt Val = llvm::APSInt(32, false);
   if (Literal.GetIntegerValue(Val)) {
@@ -886,9 +894,10 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
                                       PP.getSourceManager(), PP.getLangOpts(),
                                       PP.getTargetInfo(), PP.getDiagnostics());
   if (Literal.hadError)
-    return true; // Error has already been reported so just return
+    return std::nullopt; // Error has already been reported so just return
 
-  assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits");
+  assert(Literal.isIntegerLiteral() &&
+         "NumSpelling can only consist of digits");
 
   llvm::APSInt Val = llvm::APSInt(32, true);
   if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
@@ -900,11 +909,48 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
   }
 
   if (Negated)
-    return static_cast<int32_t>((-Val).getExtValue());
+    Val = -Val;
 
   return static_cast<int32_t>(Val.getExtValue());
 }
 
+std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {
+  // Parse the numeric value and do semantic checks on its specification
+  clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc,
+                                      PP.getSourceManager(), PP.getLangOpts(),
+                                      PP.getTargetInfo(), PP.getDiagnostics());
+  if (Literal.hadError)
+    return std::nullopt; // Error has already been reported so just return
+
+  assert(Literal.isFloatingLiteral() &&
+         "NumSpelling is consistent with isNumberChar in "
+         "LexHLSLRootSignature.cpp");
+
+  // DXC used `strtod` to convert the token string to a float which corresponds
+  // to:
+  auto DXCSemantics = llvm::APFloat::Semantics::S_IEEEdouble;
+  auto DXCRoundingMode = llvm::RoundingMode::NearestTiesToEven;
+
+  llvm::APFloat Val =
+      llvm::APFloat(llvm::APFloat::EnumToSemantics(DXCSemantics));
+  llvm::APFloat::opStatus Status = Literal.GetFloatValue(Val, DXCRoundingMode);
+
+  // The float is valid with opInexect as this just denotes if rounding occured
+  if (Status != llvm::APFloat::opStatus::opOK &&
+      Status != llvm::APFloat::opStatus::opInexact)
+    return std::nullopt;
+
+  if (Negated)
+    Val = -Val;
+
+  double DoubleVal = Val.convertToDouble();
+  if (FLT_MAX < DoubleVal || DoubleVal < -FLT_MAX) {
+    return std::nullopt;
+  }
+
+  return static_cast<float>(DoubleVal);
+}
+
 bool RootSignatureParser::verifyZeroFlag() {
   assert(CurToken.TokKind == TokenKind::int_literal);
   auto X = handleUIntLiteral();
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index ecb7d0715eaad..130548d8e74ae 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -256,7 +256,15 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
   const llvm::StringLiteral Source = R"cc(
     StaticSampler(s0, mipLODBias = 0),
     StaticSampler(s0, mipLODBias = +1),
-    StaticSampler(s0, mipLODBias = -1)
+    StaticSampler(s0, mipLODBias = -1),
+    StaticSampler(s0, mipLODBias = 42.),
+    StaticSampler(s0, mipLODBias = +4.2),
+    StaticSampler(s0, mipLODBias = -.42),
+    StaticSampler(s0, mipLODBias = .42e+3),
+    StaticSampler(s0, mipLODBias = 42E-12),
+    StaticSampler(s0, mipLODBias = 42.f),
+    StaticSampler(s0, mipLODBias = 4.2F),
+    StaticSampler(s0, mipLODBias = 42.e+10f),
   )cc";
 
   TrivialModuleLoader ModLoader;
@@ -284,6 +292,38 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
   ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -1.f);
 
+  Elem = Elements[3];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
+
+  Elem = Elements[4];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
+
+  Elem = Elements[5];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -.42f);
+
+  Elem = Elements[6];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420.f);
+
+  Elem = Elements[7];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.000000000042f);
+
+  Elem = Elements[8];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
+
+  Elem = Elements[9];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
+
+  Elem = Elements[10];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420000000000.f);
+
   ASSERT_TRUE(Consumer->isSatisfied());
 }
 

>From 5d2a90f886ecc846939fa2643ac006450068fcd0 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 16 May 2025 01:52:44 +0000
Subject: [PATCH 05/18] add testing for over/underflow and addresses other
 opStatus

- updates the error message to account for floats and fixes the previous
use cases
---
 .../clang/Basic/DiagnosticParseKinds.td       |  6 +-
 clang/lib/Parse/ParseHLSLRootSignature.cpp    | 38 +++++++--
 .../Parse/ParseHLSLRootSignatureTest.cpp      | 84 +++++++++++++++++++
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fd525140d0482..554d70de86902 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1856,7 +1856,11 @@ def err_hlsl_unexpected_end_of_params
     : Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">;
 def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
 def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">;
-def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
+def err_hlsl_number_literal_overflow : Error<
+  "%select{integer|float}0 literal is too large to be represented as a "
+  "%select{32-bit %select{signed|}1 integer|float}0 type">;
+def err_hlsl_number_literal_underflow : Error<
+  "float literal has a magnitude that is too small to be represented as a float type">;
 def err_hlsl_rootsig_non_zero_flag : Error<"flag value is neither a literal 0 nor a named value">;
 
 } // end of Parser diagnostics
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 05636a48af534..0593a5d73cde7 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -881,7 +881,7 @@ std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
-        << 0 << CurToken.NumSpelling;
+        << /*integer type*/ 0 << /*is signed*/ 0;
     return std::nullopt;
   }
 
@@ -904,7 +904,7 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
-        << 0 << CurToken.NumSpelling;
+        << /*integer type*/ 0 << /*is signed*/ 1;
     return std::nullopt;
   }
 
@@ -923,8 +923,8 @@ std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {
     return std::nullopt; // Error has already been reported so just return
 
   assert(Literal.isFloatingLiteral() &&
-         "NumSpelling is consistent with isNumberChar in "
-         "LexHLSLRootSignature.cpp");
+         "NumSpelling consists only of [0-9.ef+-]. Any malformed NumSpelling "
+         "will be caught and reported by NumericLiteralParser.");
 
   // DXC used `strtod` to convert the token string to a float which corresponds
   // to:
@@ -935,16 +935,40 @@ std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {
       llvm::APFloat(llvm::APFloat::EnumToSemantics(DXCSemantics));
   llvm::APFloat::opStatus Status = Literal.GetFloatValue(Val, DXCRoundingMode);
 
-  // The float is valid with opInexect as this just denotes if rounding occured
-  if (Status != llvm::APFloat::opStatus::opOK &&
-      Status != llvm::APFloat::opStatus::opInexact)
+  // Note: we do not error when opStatus::opInexact by itself as this just
+  // denotes that rounding occured but not that it is invalid
+  assert(!(Status & llvm::APFloat::opStatus::opInvalidOp) &&
+         "NumSpelling consists only of [0-9.ef+-]. Any malformed NumSpelling "
+         "will be caught and reported by NumericLiteralParser.");
+
+  assert(!(Status & llvm::APFloat::opStatus::opDivByZero) &&
+         "It is not possible for a division to be performed when "
+         "constructing an APFloat from a string");
+
+  if (Status & llvm::APFloat::opStatus::opUnderflow) {
+    // Report that the value has underflowed
+    PP.getDiagnostics().Report(CurToken.TokLoc,
+                               diag::err_hlsl_number_literal_underflow);
     return std::nullopt;
+  }
+
+  if (Status & llvm::APFloat::opStatus::opOverflow) {
+    // Report that the value has overflowed
+    PP.getDiagnostics().Report(CurToken.TokLoc,
+                               diag::err_hlsl_number_literal_overflow)
+        << /*float type*/ 1;
+    return std::nullopt;
+  }
 
   if (Negated)
     Val = -Val;
 
   double DoubleVal = Val.convertToDouble();
   if (FLT_MAX < DoubleVal || DoubleVal < -FLT_MAX) {
+    // Report that the value has overflowed
+    PP.getDiagnostics().Report(CurToken.TokLoc,
+                               diag::err_hlsl_number_literal_overflow)
+        << /*float type*/ 1;
     return std::nullopt;
   }
 
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 130548d8e74ae..894c707313eac 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -793,6 +793,90 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) {
   ASSERT_TRUE(Consumer->isSatisfied());
 }
 
+TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedFloatTest) {
+  // This test will check that the lexing fails due to a float overflow
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = 3.402823467e+38F)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test correct diagnostic produced
+  Consumer->setExpected(diag::err_hlsl_number_literal_overflow);
+  ASSERT_TRUE(Parser.parse());
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidLexNegOverflowedFloatTest) {
+  // This test will check that the lexing fails due to negative float overflow
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = -3.402823467e+38F)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test correct diagnostic produced
+  Consumer->setExpected(diag::err_hlsl_number_literal_overflow);
+  ASSERT_TRUE(Parser.parse());
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedDoubleTest) {
+  // This test will check that the lexing fails due to an overflow of double
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = 1.e+500)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test correct diagnostic produced
+  Consumer->setExpected(diag::err_hlsl_number_literal_overflow);
+  ASSERT_TRUE(Parser.parse());
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidLexUnderflowFloatTest) {
+  // This test will check that the lexing fails due to double underflow
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = 10e-309)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test correct diagnostic produced
+  Consumer->setExpected(diag::err_hlsl_number_literal_underflow);
+  ASSERT_TRUE(Parser.parse());
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
 TEST_F(ParseHLSLRootSignatureTest, InvalidNonZeroFlagsTest) {
   // This test will check that parsing fails when a non-zero integer literal
   // is given to flags

>From 80628f5b337725aaf95020e85a811f68beee043a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 16 May 2025 03:32:07 +0000
Subject: [PATCH 06/18] self-review: treat postive integer as unsigned

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 0593a5d73cde7..bebb86f001794 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -740,8 +740,8 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
       tryConsumeExpectedToken({TokenKind::pu_plus, TokenKind::pu_minus});
   bool Negated = Signed && CurToken.TokKind == TokenKind::pu_minus;
 
-  // Handle an uint and interpret it as a float
-  if (!Signed && tryConsumeExpectedToken(TokenKind::int_literal)) {
+  // DXC will treat a postive signed integer as unsigned
+  if (!Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
     auto UInt = handleUIntLiteral();
     if (!UInt.has_value())
       return std::nullopt;

>From 001ea4ddda36a8f84724fadab521cc9b4b18432a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Mon, 26 May 2025 21:22:41 +0000
Subject: [PATCH 07/18] review: remove INT32_MAX constraint

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp           |  2 +-
 clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index bebb86f001794..1dac305d75379 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -900,7 +900,7 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
          "NumSpelling can only consist of digits");
 
   llvm::APSInt Val = llvm::APSInt(32, true);
-  if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
+  if (Literal.GetIntegerValue(Val)) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 894c707313eac..7cb0eb9e1e473 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -265,6 +265,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
     StaticSampler(s0, mipLODBias = 42.f),
     StaticSampler(s0, mipLODBias = 4.2F),
     StaticSampler(s0, mipLODBias = 42.e+10f),
+    StaticSampler(s0, mipLODBias = -2147483648),
+    StaticSampler(s0, mipLODBias = 2147483648),
   )cc";
 
   TrivialModuleLoader ModLoader;
@@ -324,6 +326,14 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
   ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420000000000.f);
 
+  Elem = Elements[11];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -2147483648.f);
+
+  Elem = Elements[12];
+  ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 2147483648.f);
+
   ASSERT_TRUE(Consumer->isSatisfied());
 }
 

>From 64e13dfb996e9d1acd6821bcd206d83db759b89e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Mon, 26 May 2025 21:23:17 +0000
Subject: [PATCH 08/18] self-review: remove lingering cast

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1dac305d75379..d5d2fab65d018 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -682,7 +682,7 @@ RootSignatureParser::parseStaticSamplerParams() {
       auto MipLODBias = parseFloatParam();
       if (!MipLODBias.has_value())
         return std::nullopt;
-      Params.MipLODBias = (float)*MipLODBias;
+      Params.MipLODBias = MipLODBias;
     }
   } while (tryConsumeExpectedToken(TokenKind::pu_comma));
 

>From c3f460485f5f18766805de601c13dbc27a4d79af Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Mon, 26 May 2025 21:25:26 +0000
Subject: [PATCH 09/18] self-review: use proper float comparison

---
 .../Parse/ParseHLSLRootSignatureTest.cpp      | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 7cb0eb9e1e473..642f4283f5acb 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -247,7 +247,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) {
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.ViewType, RegisterType::SReg);
   ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.Number, 0u);
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
 
   ASSERT_TRUE(Consumer->isSatisfied());
 }
@@ -284,47 +284,47 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) {
 
   RootElement Elem = Elements[0];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f);
 
   Elem = Elements[1];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 1.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 1.f);
 
   Elem = Elements[2];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -1.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -1.f);
 
   Elem = Elements[3];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
 
   Elem = Elements[4];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
 
   Elem = Elements[5];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -.42f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -.42f);
 
   Elem = Elements[6];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420.f);
 
   Elem = Elements[7];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.000000000042f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.000000000042f);
 
   Elem = Elements[8];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f);
 
   Elem = Elements[9];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f);
 
   Elem = Elements[10];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));
-  ASSERT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420000000000.f);
+  ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420000000000.f);
 
   Elem = Elements[11];
   ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem));

>From 211850abed102f41aa9ad994b176499882f30208 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Mon, 26 May 2025 22:31:08 +0000
Subject: [PATCH 10/18] review: add constraint on magnitude of signed integer

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp    | 11 +++++++---
 .../Parse/ParseHLSLRootSignatureTest.cpp      | 22 +++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index d5d2fab65d018..8341e6a62f098 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -876,7 +876,7 @@ std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
   assert(Literal.isIntegerLiteral() &&
          "NumSpelling can only consist of digits");
 
-  llvm::APSInt Val = llvm::APSInt(32, false);
+  llvm::APSInt Val = llvm::APSInt(32, /*IsUnsigned=*/true);
   if (Literal.GetIntegerValue(Val)) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
@@ -899,8 +899,13 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
   assert(Literal.isIntegerLiteral() &&
          "NumSpelling can only consist of digits");
 
-  llvm::APSInt Val = llvm::APSInt(32, true);
-  if (Literal.GetIntegerValue(Val)) {
+  llvm::APSInt Val = llvm::APSInt(32, /*IsUnsigned=*/true);
+  // GetIntegerValue will overwrite Val from the parsed Literal and return
+  // true if it overflows as a 32-bit unsigned int. Then check that it also
+  // doesn't overflow as a signed 32-bit int.
+  int64_t MaxMagnitude =
+      -static_cast<int64_t>(std::numeric_limits<int32_t>::min());
+  if (Literal.GetIntegerValue(Val) || MaxMagnitude < Val.getExtValue()) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 642f4283f5acb..b08c07f41141e 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -803,6 +803,28 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) {
   ASSERT_TRUE(Consumer->isSatisfied());
 }
 
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseOverflowedNegativeNumberTest) {
+  // This test will check that parsing fails due to a unsigned integer having
+  // too large of a magnitude to be interpreted as its negative
+  const llvm::StringLiteral Source = R"cc(
+    StaticSampler(s0, mipLODBias = -4294967295)
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test correct diagnostic produced
+  Consumer->setExpected(diag::err_hlsl_number_literal_overflow);
+  ASSERT_TRUE(Parser.parse());
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
 TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedFloatTest) {
   // This test will check that the lexing fails due to a float overflow
   const llvm::StringLiteral Source = R"cc(

>From 265c72961bacdb5798eadc5ac927aeee14569bab Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:42:25 +0000
Subject: [PATCH 11/18] review: remove use of float.h for numeric_limits

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 8341e6a62f098..0ae96a8db89e4 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -6,8 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <float.h>
-
 #include "clang/Parse/ParseHLSLRootSignature.h"
 
 #include "clang/Lex/LiteralSupport.h"
@@ -969,7 +967,8 @@ std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {
     Val = -Val;
 
   double DoubleVal = Val.convertToDouble();
-  if (FLT_MAX < DoubleVal || DoubleVal < -FLT_MAX) {
+  double FloatMax = double(std::numeric_limits<float>::max());
+  if (FloatMax < DoubleVal || DoubleVal < -FloatMax) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)

>From ba4e2ae7b7badfc1ec0382e0a3cd46f6013affd5 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:43:45 +0000
Subject: [PATCH 12/18] review: spell out types

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 0ae96a8db89e4..771cb38b0996e 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -740,17 +740,17 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
 
   // DXC will treat a postive signed integer as unsigned
   if (!Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
-    auto UInt = handleUIntLiteral();
+    std::optional<uint32_t> UInt = handleUIntLiteral();
     if (!UInt.has_value())
       return std::nullopt;
     return (float)UInt.value();
   } else if (tryConsumeExpectedToken(TokenKind::int_literal)) {
-    auto Int = handleIntLiteral(Negated);
+    std::optional<int32_t> = handleIntLiteral(Negated);
     if (!Int.has_value())
       return std::nullopt;
     return (float)Int.value();
   } else if (tryConsumeExpectedToken(TokenKind::float_literal)) {
-    auto Float = handleFloatLiteral(Negated);
+    std::optional<float> Float = handleFloatLiteral(Negated);
     if (!Float.has_value())
       return std::nullopt;
     return Float.value();

>From d5fbc7ca3771d5149d35fe6b53bde7cafddd8394 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:46:41 +0000
Subject: [PATCH 13/18] review: don't use else after return

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 771cb38b0996e..ad26d18736e6b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -744,12 +744,16 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
     if (!UInt.has_value())
       return std::nullopt;
     return (float)UInt.value();
-  } else if (tryConsumeExpectedToken(TokenKind::int_literal)) {
+  }
+
+  if (Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
     std::optional<int32_t> = handleIntLiteral(Negated);
     if (!Int.has_value())
       return std::nullopt;
     return (float)Int.value();
-  } else if (tryConsumeExpectedToken(TokenKind::float_literal)) {
+  }
+
+  if (tryConsumeExpectedToken(TokenKind::float_literal)) {
     std::optional<float> Float = handleFloatLiteral(Negated);
     if (!Float.has_value())
       return std::nullopt;

>From ff0791f29cbf0ad310d8e4db91b7c576e9a264dd Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:47:21 +0000
Subject: [PATCH 14/18] self-review: fix typo

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index ad26d18736e6b..a50ff0156086a 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -747,7 +747,7 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
   }
 
   if (Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
-    std::optional<int32_t> = handleIntLiteral(Negated);
+    std::optional<int32_t> Int = handleIntLiteral(Negated);
     if (!Int.has_value())
       return std::nullopt;
     return (float)Int.value();

>From a2174da0f65794fddfbd1d29389de8b889031dff Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:49:12 +0000
Subject: [PATCH 15/18] review: use c++ style casting

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index a50ff0156086a..244a390a8f04c 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -743,14 +743,14 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
     std::optional<uint32_t> UInt = handleUIntLiteral();
     if (!UInt.has_value())
       return std::nullopt;
-    return (float)UInt.value();
+    return float(UInt.value());
   }
 
   if (Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
     std::optional<int32_t> Int = handleIntLiteral(Negated);
     if (!Int.has_value())
       return std::nullopt;
-    return (float)Int.value();
+    return float(Int.value());
   }
 
   if (tryConsumeExpectedToken(TokenKind::float_literal)) {
@@ -905,8 +905,7 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
   // GetIntegerValue will overwrite Val from the parsed Literal and return
   // true if it overflows as a 32-bit unsigned int. Then check that it also
   // doesn't overflow as a signed 32-bit int.
-  int64_t MaxMagnitude =
-      -static_cast<int64_t>(std::numeric_limits<int32_t>::min());
+  int64_t MaxMagnitude = -int64_t(std::numeric_limits<int32_t>::min());
   if (Literal.GetIntegerValue(Val) || MaxMagnitude < Val.getExtValue()) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
@@ -918,7 +917,7 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
   if (Negated)
     Val = -Val;
 
-  return static_cast<int32_t>(Val.getExtValue());
+  return int32_t(Val.getExtValue());
 }
 
 std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {

>From e60ec79ee635afb367a765e7816e7da97e9178fb Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:51:19 +0000
Subject: [PATCH 16/18] review: initialize directly

---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 244a390a8f04c..d33a4ee58bd92 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -878,7 +878,7 @@ std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
   assert(Literal.isIntegerLiteral() &&
          "NumSpelling can only consist of digits");
 
-  llvm::APSInt Val = llvm::APSInt(32, /*IsUnsigned=*/true);
+  llvm::APSInt Val(32, /*IsUnsigned=*/true);
   if (Literal.GetIntegerValue(Val)) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
@@ -901,7 +901,7 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
   assert(Literal.isIntegerLiteral() &&
          "NumSpelling can only consist of digits");
 
-  llvm::APSInt Val = llvm::APSInt(32, /*IsUnsigned=*/true);
+  llvm::APSInt Val(32, /*IsUnsigned=*/true);
   // GetIntegerValue will overwrite Val from the parsed Literal and return
   // true if it overflows as a 32-bit unsigned int. Then check that it also
   // doesn't overflow as a signed 32-bit int.
@@ -937,9 +937,8 @@ std::optional<float> RootSignatureParser::handleFloatLiteral(bool Negated) {
   auto DXCSemantics = llvm::APFloat::Semantics::S_IEEEdouble;
   auto DXCRoundingMode = llvm::RoundingMode::NearestTiesToEven;
 
-  llvm::APFloat Val =
-      llvm::APFloat(llvm::APFloat::EnumToSemantics(DXCSemantics));
-  llvm::APFloat::opStatus Status = Literal.GetFloatValue(Val, DXCRoundingMode);
+  llvm::APFloat Val(llvm::APFloat::EnumToSemantics(DXCSemantics));
+  llvm::APFloat::opStatus Status(Literal.GetFloatValue(Val, DXCRoundingMode));
 
   // Note: we do not error when opStatus::opInexact by itself as this just
   // denotes that rounding occured but not that it is invalid

>From b507dbaca10330b4544b931fe3cc45198f954477 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 16:56:54 +0000
Subject: [PATCH 17/18] review: add explicit comment of how the float is
 handled

---
 clang/include/clang/Parse/ParseHLSLRootSignature.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index dcdc7deab68c9..a03f33dfd3b4e 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -134,6 +134,14 @@ class RootSignatureParser {
   /// 32-bit integer
   std::optional<int32_t> handleIntLiteral(bool Negated);
   /// Use NumericLiteralParser to convert CurToken.NumSpelling into a float
+  ///
+  /// This matches the behaviour of DXC, which is as follows:
+  ///  - convert the spelling with `strtod`
+  ///  - check for a float overflow
+  ///  - cast the double to a float
+  /// The behaviour of `strtod` is replicated using:
+  ///  Semantics: llvm::APFloat::Semantics::S_IEEEdouble
+  ///  RoundingMode: llvm::RoundingMode::NearestTiesToEven
   std::optional<float> handleFloatLiteral(bool Negated);
 
   /// Flags may specify the value of '0' to denote that there should be no

>From 12e972c1d456624c4bdce46417312f4df0b6f77f Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 28 May 2025 17:03:31 +0000
Subject: [PATCH 18/18] review: let `handleIntLiteral` properly handle a signed
 positive integer

- this code path will currently not ever get evaluated but for the sake
of expected behaviour of a future user we will add this functionality
---
 clang/lib/Parse/ParseHLSLRootSignature.cpp | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index d33a4ee58bd92..8be91f5991b21 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -903,10 +903,17 @@ std::optional<int32_t> RootSignatureParser::handleIntLiteral(bool Negated) {
 
   llvm::APSInt Val(32, /*IsUnsigned=*/true);
   // GetIntegerValue will overwrite Val from the parsed Literal and return
-  // true if it overflows as a 32-bit unsigned int. Then check that it also
-  // doesn't overflow as a signed 32-bit int.
-  int64_t MaxMagnitude = -int64_t(std::numeric_limits<int32_t>::min());
-  if (Literal.GetIntegerValue(Val) || MaxMagnitude < Val.getExtValue()) {
+  // true if it overflows as a 32-bit unsigned int
+  bool Overflowed = Literal.GetIntegerValue(Val);
+
+  // So we then need to check that it doesn't overflow as a 32-bit signed int:
+  int64_t MaxNegativeMagnitude = -int64_t(std::numeric_limits<int32_t>::min());
+  Overflowed |= (Negated && MaxNegativeMagnitude < Val.getExtValue());
+
+  int64_t MaxPositiveMagnitude = int64_t(std::numeric_limits<int32_t>::max());
+  Overflowed |= (!Negated && MaxPositiveMagnitude < Val.getExtValue());
+
+  if (Overflowed) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)



More information about the cfe-commits mailing list