According to this section of the spec:
https://microsoft.github.io/DirectX-Specs/d3d/WorkGraphs.html#node-shader-system-values
There should be diagnostics announcing the unsupported use of certain
system value semantics in combination with certain supported launch
modes. There were previously no diagnostics for these incompatible
combinations.
This PR adds the diagnostics into Sema.
According to issue
https://github.com/microsoft/DirectXShaderCompiler/issues/5827, there is
no path for proper flattened signature construction. Parameters won't be
fully flattened at the time that Sema runs, so the diagnostics in this
PR only trigger on simple parameters with low dimensionality.
The team has agreed to leave the full implementation (which can address
higher-dimensional parameters) to the implementation of a proper
parameter parsing infrastructure, which will take place in the
modernization effort.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5827
---------
Co-authored-by: Greg Roth <grroth@microsoft.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
The SPIR-V path currently fails when a variable is passed at the index
to GetAttributeAtVertex. Not only should we not crash, but the variable
does not seem to be a problem for the SPIR-V validator. So we can simply
generate an access chain with a variable idx.
In the current implementation, a single OpAccessChain is generated for
all array subscripts and member accesses. This causes a problem if the
member access is an aliased variable, and we need to generate and
OpAccessChains for the member, a load, and then another OpAccessChain
to index into the aliased array.
This change cause an array subscript to be process as a single
OpAccessChian. Note that we still process array sub scripts with the
member access when it is a subnode of the member access. We will
probably need to fix that up later as well, but I want to keep the
change small.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/6110.
Mesh shader output variables may be decorated with the `nointerpolation`
attribute, which should be translated to a `Flat` decoration in the
SPIR-V backend.
Fixes#6250
This option is not enabled by default. When enabled, this option adds
the MaximallyReconvergesKHR execution mode to the module, and the
associated extension.
Related to #6222
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
One of the current test has an out of bounds memory access in a vector.
We access `scalars[0]` vector after all of the elements have been
removed. This is used to get the source location. The solution is to get
the source location earlier, and use the local variable.
No new test is needed because a current test exposed the problem on a
platform and build that asserts when the access happens.
These TODOs were intended for if we were to implement deep RDAT
validation rather than relying on the exact binary comparison.
This isn't happening for SM 6.8, so these are no longer
PRERELEASE-TODOs.
The comment in RDAT_Macros.inl didn't really belong there in the first
place, so I removed it.
Fixes#5699.
An "aliased" variable is on where the compiler has implicitly added an
extra level indirection. So when we access one of these variable, we
have to do an extra load.
When we do an array access on an aliased variable, we do the extra load
before doing the access chain, but we still treat the result of the
access chain as if it is an aliased value. This causes an extra load and
generates invalid spir-v.
The fix is to change `isReferencingNonAliasStructuredOrByteBuffer` so
that we do not distinguish between externally and internally visible
variable if we see an array subscript.
Fixes#6110
Vulkan allows SubpassInputs without an input attachment index specified,
so the error when it is missing has been removed and tests have been
updated.
As far as I can tell, there aren't earlier Vulkan or SPIR-V versions
where the presence of the the input attachment index was explicitly
required, so allowing it to be omitted in all cases.
Also added a test to verify that this fixes#2808 at the same time.
Fixes#6238, #2808
According to the Vulkan spec:
OpTypeImage must declare a scalar 32-bit float, 64-bit integer, or
32-bit integer type for the “Sampled Type” (RelaxedPrecision can be
applied to a sampling instruction and to the variable holding the result
of a sampling instruction)
We want to avoid translating a true HLSL 16-bit floating point texture
object to a 32-bit texture object with relaxed precision, as this will
break backwards compatibility if Vulkan does support 16-bit texture
types in the future.
This change adds an error message in cases where the code generated
would otherwise fail validation for the above reasons.
Closes#4372
When not updating the in-tree sources, the compare step would always run
because the comparison was part of the target. This prevents that by
having the comparison be its own custom command that produces an output
and the target depends on the "output" of the comparison step.
- This changes the FeatureManager constructor, so that EXT_mesh_shader
is still the default choice for mesh shaders when SPIR-V 1.4 or above is
supported, but is not forced even when NV_mesh_shader is requested.
Before, it was impossible to choose NV_mesh_shader in that case.
Fixes#6193
Two related sets of changes:
1) Modify the value-to-declare pass to tolerate bitfields, which are
detected by their habit of being smaller than the type in which they are
contained. The pass produces dbg.declare statements that map each HLSL
variable to some storage. With bitfields, that mapping becomes
many-to-one, because many bitfields can be stored in the same basic
type. Most of the changes are to preserve the size of the bitfield as
the code descends the type hierarchy into the underlying basic type,
which is what motivates these new "type overrride" parameters to some
functions. Complications to watch out for are min16 types, which may or
may not be their own purpose-made (16-bit) types depending on whether or
not they are enabled.
2) Modify the PIX debug-info API to return enough data to PIX to allow
it to extract (by bit shifting) a bitfield from the underlying type.
This bit is also about preserving knowledge of the field's size as
opposed to its containing type's size, hence the new dxcapi
interface/method.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This PR improves the experience for debugging tools that consume SPIR-V
produced by passing in-memory strings to dxcompiler.dll. When dxc.exe is
used, OpSource and OpDebugSource are populated by reading original
source from the filesystem. dxcompiler.dll users instead find their
input string is preprocessed, which is confusing behavior.
Contrary to the leading comment in the code this change modifies,
debugging tools do not want preprocessed source. Such tools show and
allow editing of include files. If everything is pasted into a single
source string, finding what to edit is difficult. Editing the
preprocessed source has the additional complication that a user would
have to edit also the #line markers, which is hard to get right
manually. In the preprocessed source, all comments are lost, which makes
it hard to read the code. Finally in preprocessed source, all macros and
code under conditional compilation are lost. That will make it
impossible to modify those parts in an edit.
Co-authored-by: Steve Urquhart <stevur01@ntd.nintendo.com>
Fix#6230
The literal type visitor is asserting that the AST result type of an
OpLoad should not be literal. The following expression was encountering
this assertion.
float_vec_4 *= condition ? -1.0 : 1.0;
This patch visits OpVectorTimesScalar operations to change the AST
result type of an instruction from a literal type to a non-literal type
where possible.
Also, fix some mispellings I encountered while navigating the code.
When the env isn't valid, we emit an error. But instead of returning, we
continued and still tried to load the optional value, which triggered an
assert.
Signed-off-by: Nathan Gauër <brioche@google.com>
The implementation of flat conversion in the spir-v codegen is very
adhoc. This changes the implemetation to match what is done in DXIL. For
now I have only reimplemented to catch all case. I will leave the
special cases as they seem to be correct for what they do, and will
generate less verbose code.
Fixes#4906
---------
Co-authored-by: Nathan Gauër <github@keenuts.net>
The VulkanMemoryModelDeviceScope capability is now covered by the
SPIRV-Tools capability trimming pass, so it should be added here
unconditionally now.
Fixes#6066
We recently removed the FileTest infrstructure because we were able to
replace all of the tests with LIT tests. However, we now have to test
the a case were the source code is in memory and not in a file (see
`FileTest`, buti specializes it so that it cannot be given a file name,
and
remove the uses of EFFCEE.
One test is added as an example on how to use a REGEX matcher to check
the assemble output.
Other options we considers:
1. Adding new infrastructure based on the code in HLSL/CompilerTest.cpp.
However that was more complex that we need. I wanted a simple
interface so that it is easy for people to add new tests. I also wanted
to make it spir-v specific.
2. Modifying the DXC executable to not pass along the file name. This
was rejected because it would place an extra option, just for
testing, the public facing executable.
3. Have the dxc executable accept `-` at the input file to have it read
the input file from stdin. Similar to 2. Rejected for similar
reasons.
This moves functions for evaluating constant expressions and translating
LLVM AP types to SPIR-V constants into a new class. These functions need
to be used from LowerTypeVisitor in #6156, so I am factoring them out to
avoid duplication.
The D3D12_EXPERIMENTAL_WAVE_MATRIX define will be removed in the future
and replaced with WinSDK version that includes runtime support for
WaveMatrix.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Internal pipeline issues revealed that certain launch type tests that
were recently merged were running with older validators.
This PR ensures that the tests run with validator version 1.8.
When asserts are disabled (LLVM_ENABLE_ASSERTIONS=0), DXASSERT* macros
are defined to nothing, resulting in no statements followed by a
semicolon. This results in:
warning C4390: ';': empty controlled statement found; is this the
intent?
Which fails because warnings are treated as errors.
Fix by adding the standard "do {} while(0)". Also opportunistically
remove the extra semicolon for non-WIN32 builds.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
#5827 first requires validation at the DXIL level. The validation will
check for functions that have incompatible node launch types, and
intrinsics that represent certain system values.
This PR implements this validation and adds some tests that exercise
this validation.
Fixes#6104
Added DiagnoseUsedHLSLMethodCall which only report error when the
MethodCall is used.
DiagnoseUsedHLSLMethodCall is used in DiagnoseTranslationUnit.
In DiagnoseTranslationUnit, traverse all visited functions in call
graph, diagnose illegal intrinsic call inside these functions.
Shader kind is also checked in DiagnoseUsedHLSLMethodCall.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5855
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
The init of the dia symbol manager presents an unneccessary performance
hit for WinPix. It's quite simple to defer the init, so here we are.
(This dia implementation was originally added specifically for WinPix,
but was then superseded by a simpler API. It's entirely possible that
there are no other clients of the dia interface at all, but it has been
published, so we're probably bound to maintain it.)
(Note that there are other deferment opportunities here, but the larger
pending work is to disentangle the dia implementation from that of the
new "IDxcPix*" API that WinPix uses, which would obviate deferment
altogether, but that's way out of scope for me for now.)
PSV0 MaximumExpectedWaveLaneCount was incorrectly set to 0 for
non-range.
Create struct for WaveSize in DxilFunctionProps.h.
Centralize encoding and validation logic there.
Use validation logic in both SemaHLSL and DxilValidation.
Remove test requiring newer shader model in CodeGenHLSL.
Add comprehensive test for compute and node, cs and lib targets, SM 6.6
and 6.8, testing ast, metadata and RDAT.
Add PSV0 tests to catch incorrect runtime data.
Update validation rules and test for more cases.
The build is failing due to clang-format modifying these lines. I've
changed them to what clang-format wants even though it's a bit odd, and
updated the CMake script to make the error message more clear when this
type of failure occurs.
The AppVeyor subscription has now ended, so latest builds are failing
with timeouts and downloads of last-known-good are failing with download
size limits. This change removes the AppVeyor run scripts and
documentation references.
Follow-up to add new links for downloading latest build artifacts in
Issue #6175
Wave Size should be able to take a range of possible wavesizes. This PR
aims to implement what is described in this spec:
https://github.com/microsoft/hlsl-specs/pull/149/files
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
WaveMatrix is moved out of SM 6.8, so the requirement in the test needs
to be updated to SM 6.9 to exclude the test from execution until at
least experimental SM 6.9 is supported by the device.
New feature flag ShaderFeatureInfo_SampleCmpGradientOrBias = 0x80000000
and
ShaderFeatureInfo_ExtendedCommandInfo = 0x100000000 were added.
They will be set when SampleCmpBias/Grad startInstance/VertexLocation
exists.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
The tests are for an error message that originates from Sema HLSL, it is
a not a spirv-specific code. The `-spirv` test case does not bring any
additional value. And since these tests are not marked spirv-only (as
they should not be), they are failing when the compiler is built without
SPIRV support.
Joshua Batista
Xiang Li
Steven Perron
Natalie Chouinard
Nathan Gauër
Tex Riddell
Chris B
rj12122
Jeff Noyle
Steve Urquhart
Jeremy Hayes
Helena Kotas
Cassandra Beckley
dependabot[bot]
Antonio Maiorano