Add the VulkanMemoryModelDeviceScope capability to the capability
trimming pass. According the the spec, "If the Vulkan memory model is
declared and any instruction uses Device scope, the
VulkanMemoryModelDeviceScope capability must be declared." Since this
case, based on the type of an operand, is not covered by the JSON
grammar, it is added explicitly.
The optimizer is able to preserve the interface variables of the
shaders, but that feature has not been exposed to the command line
tool.
This commit adds an option `--preserve-interface` to spirv-opt that will
cause all calls to ADCE to leave the input and output variables, even if
the variable is unused. It will apply regardless of where the option
appears on the command line.
Fixes#5522
Certain versions of GCC warn about these variables being potentially uninitialized when used.
I believe this is a false-positive, but zero-init'ing them is a safe way to fix this.
The PhysicalStorageBufferAddresses capability can now be
trimmed. From the spec, it seems any instruction enabled by this
required some operand to have the PhysicalStorageBuffer storage class.
This means checking the storage class is enough.
Now, because the pass uses the grammar, we don't need to add any
new logic.
Signed-off-by: Nathan Gauër <brioche@google.com>
The StorageImageReadWithoutFormat capability is only required when
an image type with the format set to Unknown is used with some specific
OpImageRead or OpImageSparseRead instructions.
This patch adds the required code to the capability trimming pass to
remove the StorageImageReadWithoutFormat capability when not required.
Signed-off-by: Nathan Gauër <brioche@google.com>
The function that get the number of elements in a composite variable
returns an incorrect values for the arrays. This is fixed, so that it
returns the correct number of elements for arrays where the number of
elements is represented as a 32-bit integer and is known at compile
time.
Fixes#4953
* Remove references to __FILE__
Uses of `__FILE__` leak the directory structure of the machine used to
build because it adds a string to the string table with the full path
name. I've removed the uses that show up in the release builds.
Fixes#5416
The SPIR-V specification allows any scalar integer type as an index. DXC
usually emits indexes as 32-bit integer types, however, in some cases it
is possible to make it emit 64-bit indexes instead (as in
https://github.com/microsoft/DirectXShaderCompiler/issues/5638).
* Add ComputeDerivativeGroup*NV capabilities to trim capabilities pass.
* Add SPV_NV_compute_shader_derivatives to allow lists
No tests needed for this. The code path is well tested. Just adding new
data.
spirv-link requires that memory models match between its input files.
Ensure this is the case by always returning SuccessWithChange and
always changing the memory model to match the instrumentation
code in Vulkan-ValidationLayers.
Also, disable the DCE pass in the --inst-* command line options, since
it will only work after linking.
Add a new legalization pass to dedupe invocation interlock instructions
DXC will be adding support for HLSL's rasterizer ordered views by using
the SPV_EXT_fragment_shader_interlock_extension. That extension
stipulates that if an entry point has an interlock ordering execution
mode, it must dynamically execute OpBeginInvocationInterlockEXT and
OpEndInvocationInterlockEXT, in that order, exactly once. This would be
difficult to determine in DXC's SPIR-V backend, so instead we will emit
these instructions potentially multiple times, and use this legalization
pass to ensure that the final SPIR-V follows the specification.
This PR uses data-flow analysis to determine where to place begin and
end instructions; in essence, determining whether a block contains or is
preceded by a begin instruction is similar to a specialized case of a
reaching definitions analysis, where we have only a single definition,
such as `bool has_begun = false`. For this simpler case, we can compute
the set of blocks using BFS to determine the reachability of the begin
instruction.
We need to do this for both begin and end instructions, so I have
generalized portions of the code to run both forward and backward over
the CFG for each respective case.
These functions are getting far too complicated to code in SPIRV-Tools
C++. Replace them with import stubs so that the real implementations
can live in Vulkan-ValidationLayers where they belong.
VVL will need to define these functions in spirv and link them to the
instrumented version of the user's shader.
From here on out, VVL can redefine the functions and any data they use
without updating SPIRV-Tools. Changing the function declarations will
still require both VVL and SPIRV-Tools to be updated in lock step.
From the Capability's text in the SPIRV spec:
```
An MS operand in OpTypeImage indicates multisampled, used with an
OpTypeImage having Sampled == 2 and Arrayed == 1.
```
Adding this logic to the capability trimming pass.
Adds the RayTracingKHR and RayQueryKHR capabilities to
the supported capabilities list (this includes the linked extension).
(NV and KHR capabilities/extensions shared the same IDs, so it also
works for NV flavors of those).
Some operands are not simple values, but bitmasks.
The lookup in the table for required decomposing the mask into
single values.
This commit adds support for such operands, like MinLod|Offset.
* opt: fix StorageInputOutput16 trimming.
While integrating this pass into DXC, I found a lot of missing
cases. This PR fixes a few issues centered around this capability
while laying out fondations for more fixes.
1. The grammar can define extensions in operand & opcode tables.
- opcode can rely on common capabilities, but require a new
extension.
- opcode can also rely on a capability which requires an extension.
Sometimes, the extension is listed twice, in the opcode, and
capability. But this redundancy is not guaranteed.
2. minVersion check. The condition was flipped: we added the extension
when the minVersion was less than current.
Didn't noticed the issue as I only tests on the default env.
3. Capability/Extension instructions were not ignored.
- `OpCapability Foo` will require the `Foo` capability.
- it doesn't mean the module requires the `Foo` capability.
Same for extensions.
This commit adds disabled tests, for fixes which are too large to
be brought into this already large PR.
Multiple calls to this function were causing vkCreateGraphicsPipelines
to be 3x slower on some driver. I suspect this was because each
call had to be inlined separately which bloated the code and caused
more work in the driver's SPIRV -> native instruction compilation.
This commit adds a new optimization which tries to remove unnecessary
capabilities from a SPIR-V module.
When compiling a SPIR-V module, you may have some dead-code using
features gated by a capability.
DCE will remove this code, but the capability will remain. This means
your module would still require some capability, even if it doesn't
require it. Calling this pass on your module would remove obsolete
capabilities.
This pass wouldn't be enabled by default, and would only be usable
from the API (at least for now).
NOTE: this commit only adds the basic skeleton/structure, and
doesn't mark as supported many capabilities it could support.
I'll add them as supported as I write tests.
Signed-off-by: Nathan Gauër <brioche@google.com>
The code currently tries to get the value of the floating point constant
to see if it is -0.0. However, we are not able to get the value for
16-bit floating point value, and we hit an assert.
To avoid this, we add an early check for the width to make sure it is
either 32 or 64.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5413.
GetCapabilities returned a const*, and GetExtensions did not exist.
This commit adds GetExtensions, and changes the return value to
be a const&.
This commit also removes the overload to GetCapabilities which returns
a mutable set, as it is unused.
Signed-off-by: Nathan Gauër <brioche@google.com>
When using PhysicalStorageBuffer it is possible for a function to
return a pointer type. This was not being handled correctly in
`GetLoadedVariablesFromFunctionCall` in the DCE pass because
`IsPtr` returns the wrong result.
Fixes#5270.
* NFC: makes the FeatureManager immutable for users
The FeatureManager contains some internal state, like
a set of capabilities and extensions. Those are derived
from the module.
Before this commit, the FeatureManager exposed Remove* functions
which could unsync the reported extensions/capabilities from
the truth: the module.
The only valid usecase to remove items directly from the FeatureManager
is by the context itself, when an instruction is killed:
instead of running the whole an analysis, we remove the single outdated
item.
The was 2 users who mutated its state:
- one to invalidate the manager. Moved to call a reset function.
- one who removed an extension from the feature manager after removing
it from the module. This logic has been moved to the context, who
now handles the extension removal itself.
Signed-off-by: Nathan Gauër <brioche@google.com>
* clang-format
* add RemoveCapability since the fuzztests are using it
* add tests
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
This avoids errors like this from instrumenting vertex shaders:
error: 165: Expected Constituents to be scalars or vectors of the
same type as Result Type components
%195 = OpCompositeConstruct %v4uint %uint_0 %191 %194 %uint_0
This commit adds forward iterator, and renames functions to
it matches the std::unordered_set/std::set better.
This goes against the SPIR-V coding style, but might be better in
the long run, especially when this set is used along real STL
sets.
(Right now, they are not compatible, and requires 2 syntaxes).
This container could in theory handle bidirectional
iterator, but for now, only forward seemed required for
our use-cases.
Signed-off-by: Nathan Gauër <brioche@google.com>
* SPV_KHR_cooperative_matrix
* Update DEPS with headers
* Update according to review recommendations
* Bugfix and formatting
* Formatting missed or damaged by VS2022
Simplify what we add to user code by moving most of it into a function
that checks both that the descriptor index is in bounds and the
initialization state. Move error logging into this function as
well.
Remove many options to turn off parts of the instrumentation,
because there were far too many permutations to keep working and
test properly.
Combine Buffer and TexBuffer error checking. This requires that VVL
set the length of TexBuffers in the descriptor input state, rather
than relying on the instrumentation code to call OpImageQuerySize.
Since the error log includes the descriptor set and binding numbers
we can use a single OOB error code rather than having 4 per-type
error codes.
Since the error codes are getting renumbered, make them start at 1
rather than 0 so it is easier to determine if the error code was
actually set by the instrumentation.
The length of a uvec3 was assumed to be 16 bytes, but it is 12.
Sometimes the stride might be 16 bytes though, which is probably the
source of the confusion.
Redo structure length to be the offset + length of the last member.
Add tests to cover arrays of uvec3s and uvec3 struct members.
Fixes https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5691
* Check if const is zero before getting components.
Two folding rules try to cast a constant to a MatrixConstant before
checking if it is a Null constant. This leads to the null pointer being
dereferneced. The solution is to move the check for zero earlier.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5063
We want to be able to apply scalar replacement on variables that have
the AliasPointer and RestrictPointer decorations.
This exposed a bug that needs to be fixed as well.
Scalar replacement sometimes uses the type manager to get the type id for the
variables it is creating. The variable type is a pointer to a pointee
type. Currently, scalar replacement uses the type manager when only if
the pointee type has to be unique in the module. This is done to try to avoid the case where two type hash to the same
value in the type manager, and it returns the wrong one.
However, this check is not the correct check. Pointer types still have to be
unique in the spir-v module. However, two unique pointer types can hash
to the same value if their pointee types are isomorphic. For example,
%s1 = OpTypeStruct %int
%s2 = OpTypeStruct %int
; %p1 and %p2 will hash to the same value even though they are still
; considered "unique".
%p1 = OpTypePointer Function %s1
%p2 = OpTypePointer Function %s2
To fix this, we now use FindPointerToType, and we modified TypeManager::IsUnique to refer to the whether or not a type will hash to a unique value and say that pointers are not unique.
Fixes#5196
Split per-DescriptorSet state into separate memory blocks
which are accessed via an array of buffer device addresses.
This is being done to make it easier to update state for a
single DescriptorSet without rebuilding the old giant flat
buffer.
The new data format is documented as comments in
include/spirv-tools/instrument.hpp
The control barrier instruction was allowed in a limiteted set of shader types.
Part of the HLSL legalization, we use to remove the instructions when it was is
a shader in which it was not allowed. As of spv1.3 that restriction is not long
there.
This change modifies replaced invalid opc to no longer remove it.
Fixes#4999.
GetExtractOperandsForElementOfCompositeConstruct() states "Returns the
empty vector if |result_index| is out-of-bounds", but violates that
contract for non-vector result types.
Adding a new type instruction required making the same change in two
places.
Also remove IsCompileTimeConstantInst that was used in a single place
and replace its use by an expression that better conveys the intent.
Change-Id: I49330b74bd34a35db6369c438c053224805c18e0
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
* Fix null pointer in FoldInsertWithConstants.
Struct types are not supported in constant folding yet.
* Added 'Test case 16' to fold_test.
Tests OpCompositeInsert not to be folded on a struct type.
-Make more use of InstructionBuilder instruction helper methods
-Use MakeUnique<>() rather than new
-Add InstrumentPass::GenReadFunctionCall() which optimizes function
calls in a loop with constant arguments and no side effects.
This is a prepatory change for future work on the instrumentation
code which will add more generated functions.
Avoid using OpConstantNull with types that do not allow it.
Update existing tests for slight changes in code generation.
Add new tests based on the Vulkan Validation layer test case
that exposed this problem.
This can cause interface incompatibility and should only be done
if ADCE has been applied to the following shader in the pipeline.
For this reason this capability is not available through the CLI
but rather only non-default through the API. This functionality is
intended as part of a larger cross-shader dead code elimination
sequence.
Constexpr guaranteed no runtime init in addition to const semantics.
Moving all opt/ to constexpr.
Moving all compile-unit statics to anonymous namespaces to uniformize
the method used (anonymous namespace vs static has the same behavior
here AFAIK).
Signed-off-by: Nathan Gauër <brioche@google.com>
Safe version will only optimize vertex shaders. All other shaders will
succeed without change.
Change --eliminate-dead-input-components to use new safe version.
Unsafe version (allowing non-vertex shaders) currently only available
through API. Should only be used in combination with other optimizations
to keep interfaces consistent. See optimizer.hpp for more details.
Add a flags field at the first offset within this buffer.
Define flags to allow buffer OOB checking to be enabled or
disabled at run time. This is to support VK_EXT_pipeline_robustnes.
This pass eliminates components of output variables that are not stored
to. Currently this just eliminates trailing components of arrays and
structs, all of which are dead.
WARNING: This pass is not designed to be a standalone pass as it can
cause interface incompatibiliies with the following shader in the
pipeline. See the comment in optimizer.hpp for best usage. This pass is
currently available only through the API; it is not available in the CLI.
This commit also fixes a bug in CreateDecoration() which is part of the
system of generating SPIR-V from the Type manager.
This adds two passes to accomplish this: one pass to analyze a shader
to determine the input slots that are live. The second pass is run on
the preceding shader to eliminate any stores to output slots that are
not consumed by the following shader.
These passes support vert, tesc, tese, geom, and frag shaders.
These passes are currently only available through the API.
These passes together with dead code elimination, and elimination of
dead input and output components and variables (WIP), will allow users
to do dead code elimination across shader boundaries.
Fixes#4918
* Prevent block merging from producing an invalid case construct by
merging a switch target/default with another construct's merge or
continue block
* This is to satisfy the structural dominance requirement between the
switch header and the case constructs
* Support Narrow Types in BitCast Folding Rule
This change adds support for narrow types in the BitCastScalarOrVector
folding rule. According to Section 2.2.1 of the SPIR-V spec, types that
are narrower than 32 bits are automatically either sign extended, or
zero extended depending on the type. With that guaranteed, we should
be able to use the first 32-bit word of any narrow type for the folding
logic without performing any special conversions.
In order to reduce code duplication, this change moves the
GetU32BitValue and GetU64BitValue functions from IntConstant to
ScalarConstant. Without this move, we would have needed an identical
version of GetU32BitValue on FloatConstant.
* Add Tests for 16-bit BitCast Folding
This change adds several new test cases to the
IntegerInstructionFoldingTest which trigger the 16-bit BitCast logic.
The logic for half types was also added to the integer case since we
can't easily validate half float types in C++ code. It's easier to
validate them as unsigned integers instead. Pllus this also allows us
to verify the SPIR-V constant sign extension logic too.
* Add 8-Bit Folding Test Cases
This change adds a couple more test cases to the integer instruction
folding test suite in order to ensure that the BitCast logic also
works correctly with the Int8 shader capability.
This was spotted in the Validation Layers where OpSpecConstantOp %x CompositeExtract %y 0 was being folded to a constant, but anything that was using it wasn't recognizing it as a constant, the simple fix was to add a const_mgr->MapInst(new_const_inst); so the next instruction knew it was a const
