Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shader Model 6.8 #5982

Merged
merged 91 commits into from
Nov 9, 2023
Merged

Shader Model 6.8 #5982

merged 91 commits into from
Nov 9, 2023

Conversation

hekota
Copy link
Member

@hekota hekota commented Nov 6, 2023

Shader Model 6.8 is still a work in progress.

New features include:

Work Graphs (experimental)
Work Graphs define a system of shader nodes that feed into each other to enable tailored GPU work creation.

Wave Matrix (experimental)
Wave Matrices are type abstractions of hardware support for higher bandwidth matrix multiplications, useful in machine learning and image processing. Previously called Wave Matrix Multiply and Accumulate or WaveMMA for short.

Orthogonal Comparison Sampling
This is an addition to the existing sampling methods (SampleCmp, SampleCmpLevel) with the difference between Orthogonal Comparison Sampling and the existing methods is how the LOD is computed.

Comparison sampling prior to Shader Model 6.8 has supported the following ops: SampleCmp with implicit LOD and SampleCmpLevel with explicit LOD as of shader model 6.7, prior to that only SampleCmpLevelZero was supported.

Methods added in Shader Model 6.8 are SampleCmpBias and SampleCmpGrad as well as requiring CalculateLevelOfDetail[Unclamped] to reference a non-comparison sampler object.

Extended Command Information
New system-value semantics SV_StartVertexLocation and SV_StartInstanceLocation.

hekota and others added 30 commits June 20, 2023 21:50
Add Shader Model 6.8 features in preview state: Work Graphs and Wave Matrix.

This code is considered preview ready, with more work planned before retail shaders can be compiled to Shader Model 6.8.
…version test

Replace incorrect sizeof() with strlen() in compiler version test

This would produce a failure only on x86 because it sizeof() gave the size
of a pointer, which just so happens to be the hard coded size of the hash
on x64. On x86, it would advance only 4 bytes, and fail the comparison.

This also checks to see if the VersionStringListSizeInBytes is > 2 more than the hash size, because both null-terminators are always added, so it will always be at least 2 more, even with no CustomVersionString.
The versioning is broken if the extra space in there.
Test relies on validator version 1.8, but doesn't specify this version
requirement. Added requirement.
GCC failed when faced with a variable of both type and name
RecordDispatchGrid. Adding struct to the type to make it clear
eliminates the error
…ctionProps (microsoft#5335)

A recent change from a recent PR includes changes to how the MDVals data
structure inside of EmitDxilFunctionProps gets elements (push_back
instead of index assignment).
However, there was no accompanying test for these changes, because
HLSLFileCheck\hlsl\workgraph\called_function_arg_nodeoutput.hlsl
exercised this code path and failed without the MDVals changes.
This PR adds some extra context to the above test so that the extra
purpose the above test serves isn't lost with time.

The test passes with the MDVals pushback changes, and fails with the
original valIdx index assignments.
Globals.h defines things like `IFC` macros that leak into callers. All
that was really needed here was a definition of `std::string`.
This will avoid test fail when forget to add git usr bin to PATH.

Port from
llvm/llvm-project@0f1f13f
This is for case where git usr bin is not in winreg.
…ccepting malformed handle arguments (microsoft#5399)

Except for Output Complete, instructions should not accept handle
arguments that are undefs / zeroinitializers, and should emit a
diagnostic when this happens. This PR adds tests to specifically
exercise the cases where a call instruction :

1.  receives an undef argument
2.  receives a zeroinitializer argument
3.  receives a handle argument that is invalid

And combines these cases with all different types of handle arguments
(resource handle, node handle, and node record handle)
This PR intends to at least implement the first work item in
[microsoft#5356](microsoft#5356)
…cy (microsoft#5336)

This test used the ShaderOpArithTable.xml in a weird way that breaks the
way we map to the HLK test. Use of ShaderOpArithTable.xml in this test
was unnecessary, since it wasn't using more than one row with different
parameter sets to define data-driven test cases.

This change simplifies things and gets rid of this dependency. The
impact is that the shaders are now defined in ShaderOpArith.xml in place
of the dummy shaders that used to be there, instead of
ShaderOpArithTable.xml. The shader text and target are used as defined
from ShaderOpArith.xml, instead of overriding those values with ones
from ShaderOpArithTable.xml.

The only changes to the shader content during the move is in whitespace:
indentation changed to be consistent with target file, trailing
whitespace removed.
This will avoid test fail when forget to add git usr bin to PATH.
If git usr bin already on PATH, it will just use it.

Port from

llvm/llvm-project@0f1f13f
Issue 5372 highlighted obsolete code left over from an earlier version
of work graph implementation. After investigation it is confirmed that
none of the changes there are currently required, so this change reverts
to the previous version of the function, and also updates the related
test.

Fixes microsoft#5372

Co-authored-by: Tim Corringham <[email protected]>
Diagnostics for invalid shader attributes to shader functions were
appearing too late. This PR moves the emission of these diagnostics
earlier so the verifier can catch them, and adds some tests to accompany
the changes.
This PR is one of a few that will help implement this task:
microsoft#5368
…osoft#5484)

This PR is one of several more PR's that move diagnostic emission
earlier for specific errors, so that the verifier can catch these
errors. This PR focuses on the MaxRecord* attributes applied on
parameters. Specifically, there are 2 behaviors that this PR will allow
DXC to detect earlier:

1. Prevent more than one of {MaxRecord, MaxRecordsSharedWith} from being
applied to a single parameter
2. Prevent MaxRecordsSharedWith from referencing the same parameter name
that the attribute is being applied to.

Some tests have been merged or deleted to consolidate these verifier
tests. There is still some behavior that cannot be tested at this time
(verifying that MaxRecordsSharedWith doesn't reference an invalid
parameter name), which will be addressed in the future. It's likely that
this location is too early to properly validate this behavior, and the
tests for this behavior may need to be left at the same location.

This PR is one of a few that will help implement this task:
microsoft#5368
Some D3DReflect tests compare size values literally, when the size will
change depending on the validator version.
This PR changes the literal integer in the checkline to a regex, so that
the size value changing between validators won't break the test.
Fixes  microsoft#5386
The wave matrix tests had a remaining __debug_break() call left in the
code.
There was also a function that risked returning the wrong comparison
value due to underflow behavior.
This PR improves the stability of the wave matrix tests by removing the
__debug_break() call and preventing underflow from happening.
Fixes microsoft#5498
Fixes microsoft#5369
Update both the verifier and validator tests for the node and compute
compatibility errors.

The front-end error diagnostics and the validator checks for node and
compute incompatibilities are already present in the preview branch.
This updates both the verifier and validator test cases to make them
more comprehensive.

Fixes microsoft#5346

---------

Co-authored-by: Tim Corringham <[email protected]>
…ft#5577)

Move work graph related diagnostics from CodeGen to Sema in order to
provide earlier detection of invalid code, which will help ensure that
the generated AST is valid.

Also replace tests for these with verifier tests.

Fixes microsoft#5352

---------

Co-authored-by: Tim Corringham <[email protected]>
Co-authored-by: Greg Roth <[email protected]>
(cherry picked from commit 327e582)

Co-authored-by: Tim Corringham <[email protected]>
IsHLSLNodeIOType is supposed to capture all node object types, but was
missing output record and output array types.

TranslateHLCastHandleToRes had LLVM_FALLTHROUGH in for two cases where
it made no sense to fall through to the next case.

MergeGepUse is needed after lowering GetRecordPtr to collapse GEP chains
that could result in intermediate multi-dimension array pointers from
GEPs, which is considered an invalid DXIL type.

Updated some tests for -Od compilations. Added a test for the multi-dim
GEP case.

Some CHECKs needed specialization for -Od:
- align 4 added to load/store only with optimizations enabled
- no CSE of index/annotate node handle
- also, use CHECK-LABEL to make failure investigations easier

Fixes microsoft#5597
The entry diagnostics in SemaDXR in SM 6.8 had been expanded to include
diagnostics for entry functions that are not related to ray tracing -
particulary node diagnostics.
This change relocates the diagnostics unrelated to ray tracing to
SemaHLSL.

Fixes microsoft#5528

Co-authored-by: Tim Corringham <[email protected]>
The team has come to a decision that no inferences should be made on
function declarations that have a numthreads attribute (specifically,
inferences on whether or not the function represents a shader, or a
compute shader, even).
However, some tests exist that were written with the expectation that
they would be treated as a compute shader, and they lack the compute
shader attribute. This PR addresses the problem by correctly adding the
compute shader attribute to the function declarations in these tests.
The PR is necessary so that these tests won't fail when the inference
behavior is removed.
Fixes microsoft#5623
EmitInstrNote reused most of the code from EmitInstrErrorMsg. This
consolidates both functions into one that shares most of the code. In
addition, it no longer makes the note look like an additional error in
the output and adds testing for that note. Relevant tests are modified
to include the notes

Fixes microsoft#5613
Conflicts primarily from differences in how the compiler version
mismatch on linking was implemented, some additional basic types added
in one place and the other, and some incidental clang-formatting along
the way in main
…tion (microsoft#5679)

There was a recent spec change on work-graphs that disallows the
existence of node + compute attributes on shader declarations. Because
of this old behavior, a lot of code worked around the possibility that 2
shader attributes could exist for one declaration. With this old
behavior disallowed, we no longer need to account for this possibility.
The PR now adds a diagnostic preventing multiple attributes from
existing. It will also change all the tests that worked with shaders
with multiple shader attributes, and either remove them, or update them
so that only one attribute is used.
Fixes microsoft#5601
Elimination of incorrect output complete calls was located in an awkward
spot. The code has been moved over to DCE.
Fixes microsoft#5363
…on (microsoft#5627)

This PR will automatically add the target profile shader stage as a
HLSLShader attribute to the function declaration that represents the
entry point. If the entry point doesn't exist (in the case of library
shaders), then this behavior won't happen.
Fixes microsoft#5626
microsoft#5679 removed some code in the CodeGen pass that allowed node + compute,
and some tests were updated.
In this follow up PR, code is changed in the Sema pass that used to
allow validation exemptions for node + compute. The appropriate tests
were updated. This is an improvement to microsoft#5601

Fixes microsoft#5601
python3kgae and others added 12 commits November 1, 2023 23:24
…#5953)

verify that the input record field with SV_DispatchGrid semantic is a
valid type: 32 or 16-bit uint scalar or array/vector with up to 3
components.

Fixes microsoft#5845

---------

Co-authored-by: Chris B <[email protected]>
… shaders (microsoft#5858)

Previously, there would be no recursion validation performed on any
function declarations in library shaders. This PR adds code that
validates that no function declarations that are found inside library
shaders are recursive. The diagnostics aren't repeated, since there's
logic to prevent the same diagnostic from being emitted by 2 similar
ancestors of the same recursive function. The PR also adjusts recursion
detection for shaders in non-library cases, and improves the original
diagnostic by adding information about the name of the function that is
recursive. The PR is meant to complete the work on issue microsoft#5789. However,
more work needs to be done for full accuracy.
The PR checks *all* function declarations in the translation unit for
the library shader case, when it should really only be checking a subset
of function declarations. This issue is filed here: microsoft#5857
Fixes microsoft#5789
Based on PR microsoft#5825: Update Barrier intrinsic to merge ACCESS and SYNC flags
Merging main @ 5f87376 to staging-sm-6.8.
Moving some tests from CodeGenDXIL to HLSLFileCheck because they will be properly gated on validator version here. These can be moved back once we have support for this in lit shell test.

Added %dxilver for %dxv tests requiring new validator version.

Moved `DXILValidation/Metadata` tests to `HLSLFileCheck/validation/Metadata` to allow %dxilver.  Needed to add RUN line for the HLSL file there.

Added -select-validator internal to lit shell test that checks diagnostics, thus does not run through validation, and shouldn't be limited by external validator version.

Updated version requirement in ValidationTest::AtomicsInvalidDests because the message was changed in the latest validator version.
Copy link
Contributor

github-actions bot commented Nov 6, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@hekota hekota marked this pull request as ready for review November 6, 2023 19:32
@python3kgae
Copy link
Contributor

Shall we add SV_BaseVertexLocation and SV_StartInstanceLocation to the feature list?

include/dxc/DxilContainer/RDAT_LibraryTypes.inl Outdated Show resolved Hide resolved
include/dxc/DxilContainer/RDAT_SubobjectTypes.inl Outdated Show resolved Hide resolved
include/dxc/HLSL/HLOperations.h Show resolved Hide resolved
include/dxc/Support/HLSLOptions.td Show resolved Hide resolved
Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnostics that I've flagged in CGHLSLMS.cpp are also lacking test coverage. So we really need to do something about that.

lib/DXIL/DxilUtil.cpp Show resolved Hide resolved
lib/HLSL/DxilValidation.cpp Show resolved Hide resolved
lib/HLSL/DxilValidation.cpp Show resolved Hide resolved
test/DXILValidation/lit.local.cfg Outdated Show resolved Hide resolved
test/lit.cfg Outdated Show resolved Hide resolved
tools/clang/lib/Sema/SemaHLSL.cpp Show resolved Hide resolved
tools/clang/lib/Sema/SemaHLSL.cpp Show resolved Hide resolved
tools/clang/unittests/HLSLExec/ExecutionTest.cpp Outdated Show resolved Hide resolved
- remove unused "class." parsing
- use llvm_unreachable instead of DXASSERT
- remove priority 2 from WaveMatrix execution tests
- use StringSwitch in validator arg parsing
- remove unused lit configs
Copy link
Member

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A typo and a response. It seems I can't make a single comment when I have a review pendding

utils/hct/hctdb.py Outdated Show resolved Hide resolved
@hekota hekota merged commit ceff9b8 into microsoft:main Nov 9, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants