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
Handles clang::DiagnosticsEngine and clang::DiagnosticIDs.
For DiagnosticIDs, this mostly migrates from `new DiagnosticIDs` to
convenience method `DiagnosticIDs::create()`.
Part of cleanup https://github.com/llvm/llvm-project/issues/151026
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
This relands #141130.
The initial commit uncovered that we are missing the correct linking of
FrontendHLSL into clang/lib/Parse and clang/lib/unittests/Parse.
This change addreses this by linking them accordingly.
It was also checked and ensured that the LexHLSLRootSignature libraries
do not depend on FrontendHLSL and so we are not required to link there.
Resolves: #138190 and #138192
- 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
- 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
This reverts commit e2a885537f11f8d9ced1c80c2c90069ab5adeb1d. Build failures were fixed right away and reverting the original commit without the fixes breaks the build again.
The `DiagnosticOptions` class is currently intrusively
reference-counted, which makes reasoning about its lifetime very
difficult in some cases. For example, `CompilerInvocation` owns the
`DiagnosticOptions` instance (wrapped in `llvm::IntrusiveRefCntPtr`) and
only exposes an accessor returning `DiagnosticOptions &`. One would
think this gives `CompilerInvocation` exclusive ownership of the object,
but that's not the case:
```c++
void shareOwnership(CompilerInvocation &CI) {
llvm::IntrusiveRefCntPtr<DiagnosticOptions> CoOwner = &CI.getDiagnosticOptions();
// ...
}
```
This is a perfectly valid pattern that is being actually used in the
codebase.
I would like to ensure the ownership of `DiagnosticOptions` by
`CompilerInvocation` is guaranteed to be exclusive. This can be
leveraged for a copy-on-write optimization later on. This PR changes
usages of `DiagnosticOptions` across `clang`, `clang-tools-extra` and
`lldb` to not be intrusively reference-counted.
- 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
This reduces the size of the clang/unittests build directory by 64% and
my overall build dir size by 5%. Static linking is the real driving
factor here, but even if the default build configuration used shared
libraries, I don't see why we should be building so many unit test
binaries.
To make the project more approachable for new contributors, I'm
attempting to make the build a bit less resource-intensive. Build
directory size is a common complaint, and this is low-hanging fruit.
I've noticed that incremental builds leave behind the old, stale gtest binaries, and lit will keep running them. This mostly doesn't matter unless they use shared libraries, which will eventually stop working after successive builds. You can clean up the old test binaries with this command in the build directory:
$ find tools/clang/unittests/ -iname '*Tests' -type f | xargs rm
... or you can simply clean the build directory in a more holistic way.
---------
Co-authored-by: Petr Hosek <phosek@google.com>
This PR hides the reference-counted pointer that holds `TargetOptions`
from the public API of `CompilerInvocation`. This gives
`CompilerInvocation` an exclusive control over the lifetime of this
member, which will eventually be leveraged to implement a copy-on-write
behavior.
There are two clients that currently share ownership of that pointer:
* `TargetInfo` - This was refactored to hold a non-owning reference to
`TargetOptions`. The options object is typically owned by the
`CompilerInvocation` or by the new `CompilerInstance::AuxTargetOpts` for
the auxiliary target. This needed a bit of care in `ASTUnit::Parse()` to
keep the `CompilerInvocation` alive.
* `clangd::PreambleData` - This was refactored to exclusively own the
`TargetOptions` that get moved out of the `CompilerInvocation`.
- 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
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>
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
