Remove:
* DescriptorType enum - this almost exactly shadowed the ResourceClass
enum
* ClauseType aliased ResourceClass
Although these were introduced to make the HLSL root signature handling
code a bit cleaner, they were ultimately causing confusion as they
appeared to be unique enums that needed to be converted between each
other.
Closes#153890
This pr implements returning multiple parsing errors at the granularity
of a `RootElement`
This is achieved by adding a new interface onto `RootSignatureParser`,
namely, `skipUntilExpectedToken`. This will be used to consume all the
intermediate tokens between when an error has occurred and when the next
`RootElement` begins.
At this granularity, the implementation is somewhat straight forward, as
we can just implement this `skip` function when we return from a
`parse[RootElement]` method and continue in the main `parse` loop. With
the exception that the `parseDescriptorTable` will also have to skip
ahead to the next expected closing `')'`.
If we want to provide any finer granularity, then the skip logic becomes
significantly more complicated. Skipping to the next root element will
provide a good ratio of user experience benefit to complexity of
implementation.
For more context see linked issue.
- Updates `HLSLRootSignatureParser` with a `skipUntilExpectedToken` and `skipUntilClosedParen`
interface
- Updates the `parse` loops to use the skip interface when an error is
found on parsing a root element
- Updates `parseDescriptorTable` to skip ahead to the next `')'` if it
was inside a clause
- Adds test-case to demonstrate multiple error being reported
Resolves: https://github.com/llvm/llvm-project/issues/145818
This pr audits the diagnostics produced in `RootSignatureParser`
diagnostics.
First, it has been noted more than once that the current
`diag::err_hlsl_unexpected_end_of_params` is not direct and can be
misleading. For instance,
[here](https://github.com/llvm/llvm-project/pull/147350#discussion_r2193717272)
and
[here](https://github.com/llvm/llvm-project/pull/145827#discussion_r2169406679).
This pr address this by removing this diagnostic and replacing it with a
more direct `diag::err_expected_either`. However, doing so removes the
nuance between the case where it is a missing comma and when it was an
invalid parameter.
Hence, we introduce the `diag::err_hlsl_invalid_token` for the latter
case, which does so in a direct way. Further, we can apply the same
diagnostic to the parsing of parameter values.
As part of this, we see that there was a test gap in testing the
diagnostics produced from `diag::err_expected_after` and for the parsing
of enum/flag values. As such, these are also addressed here to provide
sufficient unit/sema test coverage.
- Removes all uses of `diag::err_hlsl_unexpected_end_of_params` in
`RootSigantureParser`
- Introduce `diag::err_hlsl_invalid_token` to provide a direct
diagnostic
- In each of these cases, replace the diagnostic with either a
`diag::err_hlsl_invalid_token` or `diag::err_expected_either`
accordingly
- Update `HLSLRootSignatureParserTest` to account for diagnostic changes
- Increase test coverage of `HLSLRootSignatureParserTest` for enum/flag
diagnostics
- Increase test coverage of `RootSignatures-err` for enum/flag
diagnostics
- Small fix-up of the `diag::err_expected_after` and add test to
demonstrate usage
Resolves: https://github.com/llvm/llvm-project/issues/147799
At the moment, when we report diagnostics from `SemaHLSL` we only
provide the source location of the root signature attr. This allows for
significantly less helpful diagnostics (for eg. reporting resource range
overlaps).
This pr implements a way to retain the source location of a root element
when it is parsed, so that we can output the `SourceLocation` of each
root element that causes the overlap in the diagnostics during semantic
analysis.
This pr defines a wrapper struct `clang::hlsl::RootSignatureElement` in
`SemaHLSL` that will contain the underlying `RootElement` and can hold
any additional diagnostic information. This struct will be what is used
in `HLSLRootSignatureParser` and in `SemaHLSL`. Then the diagnostic
information will be stripped and the underlying element will be stored
in the `RootSignatureDecl`.
For the reporting of diagnostics, we can now use the retained
`SourceLocation` of each `RootElement` when reporting the range overlap,
and we can add a `note` diagnostic to highlight the other root element
as well.
- Defines `RootSignatureElement` in the `hlsl` namespace in `SemaHLSL`
(defined in `SemaHLSL` because `Parse` has a dependency on `Sema`)
- Updates parsing logic to construct `RootSignatureElement`s and retain
the source loction in `ParseHLSLRootSignature`
- Updates `SemaHLSL` when it constructs the `RootSignatureDecl` to take
the new `RootSignatureElement` and store the underlying `RootElement`
- Updates the current tests to ensure the new `note` diagnostic is
produced and that the `SourceLocation` is seen
- Slight update to the `RootSignatureValidations` api to ensure the
caller sorts and owns the memory of the passed in `RangeInfo`
- Adds a test to demonstrate the `SourceLocation` of both elements being
correctly pointed out
Resolves: https://github.com/llvm/llvm-project/issues/145819
This pr fixes a bug that allows parameters to be specified without an
intermediate comma.
After this pr, we will correctly produce a diagnostic for (eg):
```
RootFlags(0) CBV(b0)
```
This pr updates the problematic code pattern containing a chain of 'if'
statements to a chain of 'else if' statements, to prevent parsing of an
element before checking for a comma.
This pr also does 2 small updates, while in the region:
1. Simplify the `do` loop that these `if` statements are contained in.
This helps code readability and makes it easier to improve the
diagnostics further
2. Moves the `consumeExpectedToken` function calls to be right after the
`parse.*Params` invocation. This will ensure that the comma or invalid
token error is presented before a "missed mandatory param" diagnostic.
- Updates all occurrences of the if chains with an else-if chain
- Simplifies the surrounding `do` loop to be an easier to understand
`while` loop
- Moves the `consumeExpectedToken` diagnostic right after the loop so
that the missing comma diagnostic is produce before checking for any
missed mandatory arguments
- Adds unit tests for this scenario
- Small fix to the diagnostic of `RootDescriptors` to use their
respective `Token` instead of `RootConstants`
Resolves: https://github.com/llvm/llvm-project/issues/147337
The `SourceLocation` of a `RootSignatureToken` is incorrectly set to be
the "offset" into the concatenated string that denotes the
rootsignature. This causes an issue when the `StringLiteral` is a
multi-line expansion macro, since the offset will not account for the
characters between `StringLiteral` tokens.
This pr resolves this by retaining the `SourceLocation` information that
is kept in `StringLiteral` and then converting the offset in the
concatenated string into the proper `SourceLocation` using the
`StringLiteral::getLocationOfByte` interface. To do so, we will need to
adjust the `RootSignatureToken` to only hold its offset into the root
signature string. Then when the parser will use the token, it will need
to compute its actual `SourceLocation`.
See linked issue for more context.
For example:
```
#define DemoRootSignature \
"CBV(b0)," \
"RootConstants(num32BitConstants = 3, b0, invalid)"
expected caret location ---------------^
actual caret location ------------^
```
The caret points 5 characters early because the current offset did not
account for the characters:
```
'"' ' ' '\' ' ' '"'
1 2 3 4 5
```
- Updates `RootSignatureParser` to retain `SourceLocation` information
by retaining the `StringLiteral` and passing the underlying `StringRef`
to the `Lexer`
- Updates `RootSignatureLexer` so that the constructed tokens only
reflect an offset into the `StringRef`
- Updates `RootSignatureParser` to directly construct its used `Lexer`
so that the `StringLiteral` is directly tied with the string used in the
`RootSignatureLexer`
- Updates `RootSignatureParser` to use
`StringLiteral::getLocationOfByte` to get the actual token location for
diagnostics
- Updates `ParseHLSLRootSignatureTest` to construct a phony
`AST`/`StringLiteral` for the test cases
- Adds a test to `RootSignature-err.hlsl` showing that the
`SourceLocation` is correctly set for diagnostics in a multi-line macro
expansion
Resolves: https://github.com/llvm/llvm-project/issues/146967
This pr updates `setDefaultFlags` in `HLSLRootSignature.h` to account
for which version it should initialize the default flag values for.
- Updates `setDefaultFlags` with a `Version` argument and initializes
them to be compliant as described
[here](https://github.com/llvm/wg-hlsl/pull/297).
- Updates `RootSignatureParser` to retain the `Version` and pass this
into `setDefaultFlags`
- Updates all uses of `setDefaultFlags` in test-cases
- Adds some new unit testing to ensure behaviour is as expected and that
the Parser correctly passes down the version
Resolves https://github.com/llvm/llvm-project/issues/145820.
This pr removes the redundancy of having the same enums defined in both
the front-end and back-end of handling root signatures. Since there are
many more uses of the enum in the front-end of the code, we will adhere
to the naming conventions used in the front-end, to minimize the diff.
The macros in `DXContainerConstants.def` are also touched-up to be
consistent and to have each macro name follow its respective definition
in d3d12.h and searchable by name
[here](https://learn.microsoft.com/en-us/windows/win32/api/d3d12/).
Additionally, the many `getEnumNames` are moved to `DXContainer` from
`HLSLRootSignatureUtils` as they we will want them to be exposed
publicly anyways.
Changes for each enum follow the pattern of a commit that will make the
enum definition in `DXContainer` adhere to above listed naming
conventions, followed by a commit to actually use that enum in the
front-end.
Resolves https://github.com/llvm/llvm-project/issues/145815
- defines in-memory reprsentation of `comparisonFunc` and `borderColor`
- defines parsing of the `ComparisonFunc` and `StaticBorderColor` enum
- integrates parsing of these number parameters with their respective
`parseComparisonFunc` and `parseStaticBorderColor`
- adds basic unit tests to demonstrate setting functionality
Part 6 of https://github.com/llvm/llvm-project/issues/126574
The current naming of the `enum class Filter` and the Filter struct
member causes ambiguity.
This change will be reverted to be addressed by renaming the variable.
Reverts llvm/llvm-project#140294
- defines in-memory reprsentation of `filter`
- defines parsing of the `Filter` enum
- integrates parsing of these number parameters with their respective,
`parseFilter`
- adds basic unit tests to demonstrate setting functionality
Part 5 of https://github.com/llvm/llvm-project/issues/126574
- defines in-memory reprsentation of `address[U|V|W]`
- defines parsing of the `TextureAddressMode` enum
- integrates parsing of these number parameters with their respective,
`parseTextureAddressMode`
- adds basic unit tests to demonstrate setting functionality
Part 4 of https://github.com/llvm/llvm-project/issues/126574
- defines in-memory reprsentation of `maxAnisotropy`, `minLOD` and
`maxLOD`
- integrates parsing of these number parameters with their respective,
`parseUInt` and `parseFloat` respectively
- adds basic unit tests to demonstrate setting functionality
Part 3 of https://github.com/llvm/llvm-project/issues/126574
- defines in-memory representaiton of MipLODBias to allow for testing of
a float parameter
- defines `handleInt` and `handleFloat` to handle converting a token's
`NumSpelling` into a valid float
- plugs this into `parseFloatParam` to fill in the MipLODBias param
The parsing of floats is required to match the behaviour of DXC. This
behaviour is outlined as follows:
- if the number is an integer then convert it using `_atoi64`, check for
overflow and static_cast this to a float
- if the number is a float then convert it using `strtod`, check for
float overflow and static_cast this to a float, this will implicitly
also check for double over/underflow and if the string is malformed then
it will return an error
This pr matches this behaviour by parsing as, uint/int accordingly and
then casting, or, by using the correct APFloat semantics/rounding mode
with `NumericLiteralParser`.
- adds testing of error diagnostics and valid float param values to
demonstrate functionality
Part 2 of https://github.com/llvm/llvm-project/issues/126574
- define StaticSampler in-memory representation
- implement the infastructure for parsing parameters of StaticSampler
- define and implement parsing of the `s` reg to demonstrate
functionality
- add unit tests
First part of https://github.com/llvm/llvm-project/issues/126574
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
- defines RootDescriptorFlags in-memory representation
- defines parseRootDescriptorFlags to be DXC compatible. This is why we
support multiple `|` flags even though validation will assert that only one
flag is set
- add unit tests to demonstrate functionality
Final part of and resolves
https://github.com/llvm/llvm-project/issues/126577
- define in-memory representation of optional non-flag parameters to
`RootDescriptor`
- fill in data to parse these params in `parseRootDescriptorParams`
- add unit tests
Part 3 of https://github.com/llvm/llvm-project/issues/126577
- defines the `parseRootDescriptorParams` infrastructure for parsing the
params of a `RootDescriptor`
- defines the register type to illustrate use
- add tests to demonstrate functionality
Part 2 of https://github.com/llvm/llvm-project/issues/126577
- defines the `RootFlags` in-memory enum
- defines `parseRootFlags` to parse the various flag enums into a single
`uint32_t`
- adds corresponding unit tests
- improves the diagnostic message for when we provide a non-zero integer
value to the flags
Resolves https://github.com/llvm/llvm-project/issues/126575
- extends `parseRootConstantParams` and the struct to include the
optional parameters of a RootConstant
- adds corresponding unit tests
Part three of and resolves
https://github.com/llvm/llvm-project/issues/126576
- defines the `parseRootConstantParams` function and adds handling for
the mandatory arguments of `num32BitConstants` and `bReg`
- adds corresponding unit tests
Part two of implementing #126576
- defines the empty RootConstants in-memory struct
- adds test harness for testing it
- adds missing parameter keywords to the lexer (`RootConstants`,
`num32BitConstants`)
First part of implementing:
https://github.com/llvm/llvm-project/issues/126576
- defines the special values for `DESCRIPTOR_RANGE_OFFSET_APPEND` and
`unbounded` for the `offset` and `numDescriptors` parameters
respectively
- adds these parmaters to the `DescriptorClause` struct and the params
struct
- plugs in parsing of `numDescriptors` and `offset` into
`parseDescriptorTableClauseParams`
- defines the `unbounded` enum keyword for the lexer to expose to the
parser
- adds corresponding unit tests
Part 5 of #126569
- Defines `parseDescriptorRangeFlags` to establish a pattern of how
flags will be parsed
- Add corresponding unit tests
Part four of implementing #126569
- Defines `parseShaderVisiblity` to establish how single enums will be
parsed
- Adds unit testing of the visiblity enum
Part three of implementing #126569
- It was determined to define the parsing methods much more inline with
a recursive descent parser to follow the EBNF notation better
- As part of this change, we decided to go with a calling convention to
the parse.* methods of returning an optional rather than a bool and a
reference to the parsed struct
This is a clean-up task from
https://github.com/llvm/llvm-project/pull/133800
This pr relands #133800.
It addresses the compilation error of using a shadowed name `Register`
for both the struct name and the data member holding this type:
`Register Register`. It resolves the issues my renaming the data members
called `Register` to `Reg`.
This issue was not caught as the current pre-merge checks do not include
a build of `llvm;clang` using the gcc/g++ compilers and this is not
erroneous with clang/clang++.
Second part of #126569
---------
Co-authored-by: Finn Plummer <finnplummer@microsoft.com>
- Defines `ParseDescriptorTableClauseParams` to establish the pattern of
how we will parse parameters in root signatures. Namely, to use
recursive descent parsing in a way that follows closely to the EBNF
notation definition in the root signature spec.
- Implements parsing of two param types: `UInt32` and `Register` to
demonstrate the parsing implementation and allow for unit testing
- Changes the calling convention to use `std::optional` return values
instead of boolean error returns and parameters by reference
Part two of implementing:
https://github.com/llvm/llvm-project/issues/126569
---------
Co-authored-by: Finn Plummer <finnplummer@microsoft.com>
- when developing the RootSignatureLexer library, we are creating new
files so we should set the standard to adhere to the coding conventions
for function naming
- this was missed in the initial review but caught in the review of the
parser pr
[here](https://github.com/llvm/llvm-project/pull/133302#discussion_r2017632092)
Co-authored-by: Finn Plummer <finnplummer@microsoft.com>
This pr relands https://github.com/llvm/llvm-project/pull/133302.
It resolves two issues:
- Linking error during build,
[here](https://github.com/llvm/llvm-project/pull/133302#issuecomment-2767259848).
There was a missing dependency for `clangLex` for the
`ParseHLSLRootSignatureTest.cpp` unit testing. This library was added to
the dependencies to resolve the error. It wasn't caught previously as
the library was transitively linked in most build environments
- Warning of unused declaration,
[here](https://github.com/llvm/llvm-project/pull/133302#issuecomment-2767091368).
There was a usability line in `LexHLSLRootSignature.h` of the form
`using TokenKind = enum RootSignatureToken::Kind` which causes this
error. The declaration is removed from the header file to be used
locally in the `.cpp` files that use it.
Notably, the original pr would also exposed `clang::hlsl::TokenKind` to
everywhere it was included, which had a name clash with
`tok::TokenKind`. This is another motivation to change to the proposed
resolution.
---------
Co-authored-by: Finn Plummer <finnplummer@microsoft.com>
Reverts llvm/llvm-project#133302
Reverting to inspect build failures that were introduced from use of the
`clang::Preprocessor` in unit testing, as well as, the warning about an
unused declaration. See linked issue for failures.
- defines the Parser class and an initial set of helper methods to
support consuming tokens. functionality is demonstrated through a simple
empty descriptor table test case
- defines an initial in-memory representation of a DescriptorTable
- implements a test harness that will be used to validate the correct
diagnostics are generated. it will construct a dummy pre-processor with
diagnostics consumer to do so
Implements the first part of
https://github.com/llvm/llvm-project/issues/126569
