[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing for RootFlags (PR #138055)
@@ -100,6 +104,20 @@ UNBOUNDED_ENUM(unbounded, "unbounded") // Descriptor Range Offset Enum: DESCRIPTOR_RANGE_OFFSET_ENUM(DescriptorRangeOffsetAppend, "DESCRIPTOR_RANGE_OFFSET_APPEND") +// Root Flag Enums: +ROOT_FLAG_ENUM(AllowInputAssemblerInputLayout, "ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT") +ROOT_FLAG_ENUM(DenyVertexShaderRootAccess, "DENY_VERTEX_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyHullShaderRootAccess, "DENY_HULL_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyDomainShaderRootAccess, "DENY_DOMAIN_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyGeometryShaderRootAccess, "DENY_GEOMETRY_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyPixelShaderRootAccess, "DENY_PIXEL_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyAmplificationShaderRootAccess, "DENY_AMPLIFICATION_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(DenyMeshShaderRootAccess, "DENY_MESH_SHADER_ROOT_ACCESS") +ROOT_FLAG_ENUM(AllowStreamOutput, "ALLOW_STREAM_OUTPUT") +ROOT_FLAG_ENUM(LocalRootSignature, "LOCAL_ROOT_SIGNATURE") +ROOT_FLAG_ENUM(CBVSRVUAVHeapDirectlyIndexed, "CBV_SRV_UAV_HEAP_DIRECTLY_INDEXED") +ROOT_FLAG_ENUM(SamplerHeapDirectlyIndexed , "SAMPLER_HEAP_DIRECTLY_INDEXED") Icohedron wrote: ```suggestion ROOT_FLAG_ENUM(SamplerHeapDirectlyIndexed, "SAMPLER_HEAP_DIRECTLY_INDEXED") ``` https://github.com/llvm/llvm-project/pull/138055 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing for RootFlags (PR #138055)
https://github.com/Icohedron approved this pull request. Looks good aside from one nit about an extra space before a comma https://github.com/llvm/llvm-project/pull/138055 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing for RootFlags (PR #138055)
https://github.com/Icohedron edited https://github.com/llvm/llvm-project/pull/138055 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement validation of resource ranges for `RootDescriptors` (PR #140962)
@@ -951,6 +952,108 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr
*RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
+ using KeyT = uint64_t;
+
+ static const unsigned NumVisEnums =
+ (unsigned)llvm::hlsl::rootsig::ShaderVisibility::NumEnums;
+
+private:
+ llvm::hlsl::rootsig::ResourceRange::IMap::Allocator Allocator;
+
+ // Denotes a mapping of a unique combination of ResourceClass and register
+ // space to a ResourceRange
+ using MapT = llvm::SmallDenseMap;
+
+ // Denotes a mapping for each unique visibility
+ MapT RangeMaps[NumVisEnums];
+
+ constexpr static KeyT getKey(const llvm::hlsl::rootsig::RangeInfo &Info) {
+uint64_t SpacePacked = (uint64_t)Info.Space;
+uint64_t ClassPacked = (uint64_t)llvm::to_underlying(Info.Class);
+return (ClassPacked << 32) | SpacePacked;
+ }
+
+public:
+ // Returns std::nullopt if there was no collision. Otherwise, it will
+ // return the RangeInfo of the collision
+ std::optional
+ addRange(const llvm::hlsl::rootsig::RangeInfo &Info) {
+MapT &VisRangeMap = RangeMaps[llvm::to_underlying(Info.Vis)];
+auto [It, _] = VisRangeMap.insert(
+{getKey(Info), llvm::hlsl::rootsig::ResourceRange(Allocator)});
+auto Res = It->second.insert(Info);
+if (Res.has_value())
+ return Res;
+
+MutableArrayRef Maps =
Icohedron wrote:
Perhaps a comment here would be helpful to explain more clearly what this and
the following loop are doing.
https://github.com/llvm/llvm-project/pull/140962
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement validation of resource ranges for `RootDescriptors` (PR #140962)
@@ -951,6 +952,108 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr
*RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
+ using KeyT = uint64_t;
Icohedron wrote:
Is there a reason to store the resource space and ResourceClass together as a
64-bit integer as opposed to, say, a `std::pair`?
https://github.com/llvm/llvm-project/pull/140962
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement validation of resource ranges for `RootDescriptors` (PR #140962)
@@ -951,6 +952,108 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr
*RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
Icohedron wrote:
```suggestion
// KeyT: 32-lsb denotes resource space, and 32-msb denotes ResourceClass enum
```
https://github.com/llvm/llvm-project/pull/140962
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement validation of resource ranges for `RootDescriptors` (PR #140962)
@@ -951,6 +952,108 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr
*RHS,
<< NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
}
+namespace {
+
+// A resource range overlaps with another resource range if they have:
+// - equivalent ResourceClass (SRV, UAV, CBuffer, Sampler)
+// - equivalent resource space
+// - overlapping visbility
+class ResourceRanges {
+public:
+ // KeyT: 32-lsb denotes resource space, and 32-msb denotes resource type enum
+ using KeyT = uint64_t;
+
+ static const unsigned NumVisEnums =
Icohedron wrote:
When to use `unsigned` as opposed to `uint32_t`? I noticed there has been
inconsistency over which type is used.
https://github.com/llvm/llvm-project/pull/140962
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
https://github.com/Icohedron edited https://github.com/llvm/llvm-project/pull/140181 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
@@ -821,22 +873,115 @@ std::optional
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);
+ llvm::APSInt Val = llvm::APSInt(32, /*IsUnsigned=*/true);
if (Literal.GetIntegerValue(Val)) {
// 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;
}
return Val.getExtValue();
}
+std::optional 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 std::nullopt; // Error has already been reported so just return
+
+ assert(Literal.isIntegerLiteral() &&
+ "NumSpelling can only consist of digits");
+
+ 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(std::numeric_limits::min());
+ if (Literal.GetIntegerValue(Val) || MaxMagnitude < Val.getExtValue()) {
Icohedron wrote:
`2147483648` would be parsed by `handleUIntLiteral` instead. But yes, if
`handleUIntLiteral` was called instead for some reason then it would fail to
report an overflow.
https://github.com/llvm/llvm-project/pull/140181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
https://github.com/Icohedron approved this pull request. https://github.com/llvm/llvm-project/pull/140181 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
@@ -821,22 +873,110 @@ std::optional
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)) {
// 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;
}
return Val.getExtValue();
}
+std::optional 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 std::nullopt; // Error has already been reported so just return
+
+ assert(Literal.isIntegerLiteral() &&
+ "NumSpelling can only consist of digits");
+
+ llvm::APSInt Val = llvm::APSInt(32, true);
+ if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
Icohedron wrote:
Why is the `|| INT32_MAX < Val.getExtValue()` necessary? Doesn't
`Literal.GetIntegerValue(Val)` already return true if Val overflows?
https://github.com/llvm/llvm-project/pull/140181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [HLSL][RootSignature] Implement `ResourceRange` as an `IntervalMap` (PR #140957)
@@ -222,6 +222,67 @@ MDNode *MetadataBuilder::BuildDescriptorTableClause(
});
}
+std::optional
+ResourceRange::getOverlapping(const RangeInfo &Info) const {
+ IMap::const_iterator Interval = Intervals.find(Info.LowerBound);
+ if (!Interval.valid() || Info.UpperBound < Interval.start())
+return std::nullopt;
+ return Interval.value();
+}
+
+const RangeInfo *ResourceRange::lookup(uint32_t X) const {
+ return Intervals.lookup(X, nullptr);
+}
+
+std::optional ResourceRange::insert(const RangeInfo &Info) {
+ uint32_t LowerBound = Info.LowerBound;
+ uint32_t UpperBound = Info.UpperBound;
+
+ std::optional Res = std::nullopt;
+ IMap::iterator Interval = Intervals.begin();
+
+ while (true) {
+if (UpperBound < LowerBound)
+ break;
+
+Interval.advanceTo(LowerBound);
+if (!Interval.valid()) // No interval found
+ break;
+
+// Let Interval = [x;y] and [LowerBound;UpperBound] = [a;b] and note that
+// a <= y implicitly from Intervals.find(LowerBound)
Icohedron wrote:
```suggestion
// a <= y implicitly from Interval.advanceTo(LowerBound)
```
https://github.com/llvm/llvm-project/pull/140957
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [HLSL][RootSignature] Implement `ResourceRange` as an `IntervalMap` (PR #140957)
https://github.com/Icohedron approved this pull request. https://github.com/llvm/llvm-project/pull/140957 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [HLSL][RootSignature] Implement `ResourceRange` as an `IntervalMap` (PR #140957)
@@ -222,6 +222,67 @@ MDNode *MetadataBuilder::BuildDescriptorTableClause(
});
}
+std::optional
+ResourceRange::getOverlapping(const RangeInfo &Info) const {
+ IMap::const_iterator Interval = Intervals.find(Info.LowerBound);
+ if (!Interval.valid() || Info.UpperBound < Interval.start())
+return std::nullopt;
+ return Interval.value();
+}
+
+const RangeInfo *ResourceRange::lookup(uint32_t X) const {
+ return Intervals.lookup(X, nullptr);
+}
+
+std::optional ResourceRange::insert(const RangeInfo &Info) {
+ uint32_t LowerBound = Info.LowerBound;
+ uint32_t UpperBound = Info.UpperBound;
+
+ std::optional Res = std::nullopt;
+ IMap::iterator Interval = Intervals.begin();
+
+ while (true) {
+if (UpperBound < LowerBound)
+ break;
+
+Interval.advanceTo(LowerBound);
+if (!Interval.valid()) // No interval found
+ break;
+
+// Let Interval = [x;y] and [LowerBound;UpperBound] = [a;b] and note that
+// a <= y implicitly from Intervals.find(LowerBound)
+if (UpperBound < Interval.start())
+ break; // found interval does not overlap with inserted one
+
+if (!Res.has_value()) // Update to be the first found intersection
+ Res = Interval.value();
+
+if (Interval.start() <= LowerBound && UpperBound <= Interval.stop()) {
+ // x <= a <= b <= y implies that [a;b] is covered by [x;y]
+ // -> so we don't need to insert this, report an overlap
+ return Res;
+} else if (LowerBound <= Interval.start() &&
+ Interval.stop() <= UpperBound) {
+ // a <= x <= y <= b implies that [x;y] is covered by [a;b]
+ // -> so remove the existing interval that we will cover with the
+ // overwrite
+ Interval.erase();
+} else if (LowerBound < Interval.start() && UpperBound <= Interval.stop())
{
+ // a < x <= b <= y implies that [a; x] is not covered but [x;b] is
+ // -> so set b = x - 1 such that [a;x-1] is now the interval to insert
+ UpperBound = Interval.start() - 1;
+} else if (Interval.start() <= LowerBound && Interval.stop() < UpperBound)
{
+ // a < x <= b <= y implies that [y; b] is not covered but [a;y] is
Icohedron wrote:
```suggestion
// x <= a <= y < b implies that [y; b] is not covered but [a;y] is
```
https://github.com/llvm/llvm-project/pull/140957
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [HLSL][RootSignature] Implement `ResourceRange` as an `IntervalMap` (PR #140957)
@@ -0,0 +1,177 @@
+//===-- HLSLRootSignatureRangeTest.cpp - RootSignature Range tests
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "gtest/gtest.h"
+
+using namespace llvm::hlsl::rootsig;
+
+namespace {
+
+TEST(HLSLRootSignatureTest, NoOverlappingInsertTests) {
+ // Ensures that there is never a reported overlap
+ ResourceRange::IMap::Allocator Allocator;
+ ResourceRange Range(Allocator);
+
+ RangeInfo A;
+ A.LowerBound = 0;
+ A.UpperBound = 3;
+ EXPECT_EQ(Range.insert(A), std::nullopt);
+
+ RangeInfo B;
+ B.LowerBound = 4;
+ B.UpperBound = 7;
+ EXPECT_EQ(Range.insert(B), std::nullopt);
+
+ RangeInfo C;
+ C.LowerBound = 10;
+ C.UpperBound = RangeInfo::Unbounded;
+ EXPECT_EQ(Range.insert(C), std::nullopt);
+
+ // A = [0;3]
+ EXPECT_EQ(Range.lookup(0), &A);
+ EXPECT_EQ(Range.lookup(2), &A);
+ EXPECT_EQ(Range.lookup(3), &A);
+
+ // B = [4;7]
+ EXPECT_EQ(Range.lookup(4), &B);
+ EXPECT_EQ(Range.lookup(5), &B);
+ EXPECT_EQ(Range.lookup(7), &B);
+
+ EXPECT_EQ(Range.lookup(8), nullptr);
+ EXPECT_EQ(Range.lookup(9), nullptr);
+
+ // C = [10;unbounded]
+ EXPECT_EQ(Range.lookup(10), &C);
+ EXPECT_EQ(Range.lookup(42), &C);
+ EXPECT_EQ(Range.lookup(98237423), &C);
+ EXPECT_EQ(Range.lookup(RangeInfo::Unbounded), &C);
+}
+
+TEST(HLSLRootSignatureTest, SingleOverlappingInsertTests) {
+ // Ensures that we correctly report an overlap when we insert a range that
+ // overlaps with one other range but does not cover (replace) it
+ ResourceRange::IMap::Allocator Allocator;
+ ResourceRange Range(Allocator);
+
+ RangeInfo A;
+ A.LowerBound = 1;
+ A.UpperBound = 5;
+ EXPECT_EQ(Range.insert(A), std::nullopt);
+
+ RangeInfo B;
+ B.LowerBound = 0;
+ B.UpperBound = 2;
+ EXPECT_EQ(Range.insert(B).value(), &A);
+
+ RangeInfo C;
+ C.LowerBound = 4;
+ C.UpperBound = RangeInfo::Unbounded;
+ EXPECT_EQ(Range.insert(C).value(), &A);
+
+ // A = [1;5]
+ EXPECT_EQ(Range.lookup(1), &A);
+ EXPECT_EQ(Range.lookup(2), &A);
+ EXPECT_EQ(Range.lookup(3), &A);
+ EXPECT_EQ(Range.lookup(4), &A);
+ EXPECT_EQ(Range.lookup(5), &A);
+
+ // B = [0;0]
+ EXPECT_EQ(Range.lookup(0), &B);
+
+ // C = [6; unbounded]
Icohedron wrote:
```suggestion
// C = [6;unbounded]
```
https://github.com/llvm/llvm-project/pull/140957
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
@@ -821,22 +873,110 @@ std::optional
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)) {
// 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;
}
return Val.getExtValue();
}
+std::optional 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 std::nullopt; // Error has already been reported so just return
+
+ assert(Literal.isIntegerLiteral() &&
+ "NumSpelling can only consist of digits");
+
+ llvm::APSInt Val = llvm::APSInt(32, true);
+ if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
Icohedron wrote:
nvm. I see why
https://github.com/llvm/llvm-project/pull/140181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
https://github.com/Icohedron edited https://github.com/llvm/llvm-project/pull/140181 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
@@ -821,22 +873,110 @@ std::optional
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)) {
// 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;
}
return Val.getExtValue();
}
+std::optional 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 std::nullopt; // Error has already been reported so just return
+
+ assert(Literal.isIntegerLiteral() &&
+ "NumSpelling can only consist of digits");
+
+ llvm::APSInt Val = llvm::APSInt(32, true);
+ if (Literal.GetIntegerValue(Val) || INT32_MAX < Val.getExtValue()) {
Icohedron wrote:
But this code wouldn't allow INT32_MIN to be parsed because `abs(INT32_MIN) >
INT32_MAX`
https://github.com/llvm/llvm-project/pull/140181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error handling and validation in root signature parsing (PR #144577)
https://github.com/Icohedron approved this pull request. https://github.com/llvm/llvm-project/pull/144577 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error accumulation in root signature parsing (PR #144465)
https://github.com/Icohedron approved this pull request. Looks fine to me https://github.com/llvm/llvm-project/pull/144465 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [NFC][HLSL] Move resource range logic from `SemaHLSL` to `RootSignatureValidations` (PR #147117)
https://github.com/Icohedron approved this pull request. https://github.com/llvm/llvm-project/pull/147117 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error handling and validation in root signature parsing (PR #144577)
https://github.com/Icohedron dismissed https://github.com/llvm/llvm-project/pull/144577 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (PR #147800)
https://github.com/Icohedron edited https://github.com/llvm/llvm-project/pull/147800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (PR #147800)
https://github.com/Icohedron edited https://github.com/llvm/llvm-project/pull/147800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (PR #147800)
https://github.com/Icohedron approved this pull request. Seems fine to me. Could use a second approval to make sure https://github.com/llvm/llvm-project/pull/147800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (PR #147800)
@@ -17,24 +17,89 @@ void bad_root_signature_2() {}
[RootSignature(""), RootSignature("")] // expected-warning {{attribute
'RootSignature' is already applied}}
void bad_root_signature_3() {}
-[RootSignature("DescriptorTable(), invalid")] // expected-error {{expected end
of stream to denote end of parameters, or, another valid parameter of
RootSignature}}
+// expected-error@+1 {{invalid parameter of RootSignature}}
+[RootSignature("DescriptorTable(), invalid")]
void bad_root_signature_4() {}
-// expected-error@+1 {{expected ')' to denote end of parameters, or, another
valid parameter of RootConstants}}
-[RootSignature("RootConstants(b0, num32BitConstants = 1, invalid)")]
+// expected-error@+1 {{expected ')' or ','}}
Icohedron wrote:
nit: Perhaps check for column number to see where a ')' or ',' is expected?
https://github.com/llvm/llvm-project/pull/147800
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
