[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
Finn Plummer via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon May 26 14:26:24 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140181
>From f9fbe391091fbf23203d6cc997e19d05d92a4a18 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 1/9] 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 f434caece16ba262807968623d719a09f2a455ff 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 2/9] 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 80fedc2f16574..2e85fd3011d05 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 6e4bb4d59e109..2aa3b1f8e31c9 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -378,6 +378,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))
@@ -663,6 +667,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 14c3101f3eafa..31df2b73c2ac1 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 25df9a7235ef3..6b4da48a302bc 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -160,6 +160,7 @@ struct DescriptorTableClause {
struct StaticSampler {
Register Reg;
+ float MipLODBias = 0.f;
};
/// Models RootElement : RootFlags | RootConstants | RootParam
>From 388169b41590e66139b2d39b8a036f285d1b7f6a 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 3/9] 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 2e85fd3011d05..f886d38600d6e 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 2aa3b1f8e31c9..cbf6f34d88277 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -679,7 +679,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;
@@ -732,6 +732,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 &&
@@ -858,6 +882,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 31df2b73c2ac1..d31d16b163258 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 2317c031224eb49179e5c810d45fef93aacb2c52 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 4/9] 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 f886d38600d6e..c12b022a030ef 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 cbf6f34d88277..264c98c5e71d3 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"
@@ -736,7 +738,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
@@ -749,8 +752,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;
@@ -866,9 +873,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)) {
@@ -888,9 +896,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()) {
@@ -902,11 +911,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 d31d16b163258..632d2fec27fb4 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 8cb2f9556cf68e4c335806b9526903d0c83e3a8c 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 5/9] 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 264c98c5e71d3..c3589fa190432 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -883,7 +883,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;
}
@@ -906,7 +906,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;
}
@@ -925,8 +925,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:
@@ -937,16 +937,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 632d2fec27fb4..539232e0bf2c2 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -764,6 +764,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 b4eba5d052fb93b7b92d389f3ac4172e20e0faba 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 6/9] 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 c3589fa190432..db2e922160062 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -742,8 +742,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 90839888551c2250b0e6c10da9ebe0a142772507 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 7/9] 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 db2e922160062..ca8765f2a33fd 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -902,7 +902,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 539232e0bf2c2..c70289f3b3da4 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 5cc8aaa0313db775eef96b96389cf4218c590d3c 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 8/9] 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 ca8765f2a33fd..30bb9be299b6d 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -684,7 +684,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 03aa01ab6b2a08c20dbd0da16cd6bbf22b3665ad 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 9/9] 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 c70289f3b3da4..33ff1950df4c1 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));
More information about the llvm-branch-commits
mailing list