Ensure that the validator rejects stores to objects of types
`OpTypeImage`, `OpTypeSampler`, `OpTypeSampledImage`,
`OpTypeAccelerationStructureKHR`, and arrays of these types, according
to `VUID-StandaloneSpirv-OpTypeImage-06924`.
Guard the check behind the before_hlsl_legalization option, as
sometimes we may have temporaries or local variables that are expected
to get optimized away.
Fixes#4796
Change-Id: Ie035c01c5f94e7bdfc16b5c6c85705f302b7bda3
Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
* spirv-opt: fix crash in function declarations
Function declarations contain no blocks, so bail before segfaulting
in function optimization passes that operate on blocks.
Fixes#5795
* spirv-opt: add test for optimizing declarations
* Validate Stride operand to OpCooperativeMatrix{Load,Store}KHR
The specification requires the Stride operand for the RowMajorKHR and
ColumnMajorKHR layouts.
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Change-Id: I51084b9b8dedebf9cab7ae25334ee56b75ef0126
* Update source/val/validate_memory.cpp
Co-authored-by: alan-baker <alanbaker@google.com>
* add test to exercise memory layout from spec constant and fix validation
Change-Id: I06d7308c4a2b62d26d69e88e03bfa009a7f8fff3
* format fixes
Change-Id: I9cbabec0ed2172dcd228cc385551cb7a5b79df1a
---------
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Co-authored-by: alan-baker <alanbaker@google.com>
We want to be able to recover when fix storage class is not able to fix
everything, and just leave the spir-v in an invalid state. The pass
should not fail because of that.
This removes some uses of the type manager. One use could not be
removed. Instead I had to update GenCopy to not use the type manager,
and be able to copy pointers.
Part of #5691
* linker: run dedup earlier
Otherwise `linkings_to_do` might end up with stale IDs.
* linker: allow linking functions with different pointer arguments
Since llvm-17 there are no typed pointers and hte SPIRV-LLVM-Translator
doesn't know the function signature of imported functions.
I'm investigating different ways of solving this problem and adding an
option to work around it inside `spirv-link` is one of those.
The code is almost complete, just I'm having troubles constructing the
bitcast to cast the pointer parameters to the final type.
Closes: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/2153
* test/linker: add tests to test the AllowPtrTypeMismatch feature
When a trying to mark store that use the same address as a load live, we
consider any use of the pointer in the store instruction enough to make
the store live. This is not correct. We should only mark the store as
live if it store to the pointer, and not storing the pointer to another
memory location.
This causes DCE to miss some dead code.
* properly handle the load and store cache control operand types
Without handling these operand types, disassembling a SPIR-V module that uses the cache control extension produces an invalid operand type error.
* add a round trip test for SPV_INTEL_cache_controls
* opt: split composite from array flattening
DXC has an option to flatten resource arrays. But when this option
is not used, the resource arrays should be kept as-is.
On the other hand, when a struct contains resources, we MUST flatten is
to be compliant with the Vulkan spec.
Because this pass flattens both types of resources, using a struct of
resources automatically implied flattening arrays.
By adding those 2 new settings, we decide if the pass flattens only one type
of resources, or both.
Note: the flatten_arrays flag only impacts resource arrays.
Arrays of composites containing resources are still flattened.
Since the API is considered stable, I added 2 new functions to create
passes with one flag or the other, and kept the original behavior as-is.
Related to https://github.com/microsoft/DirectXShaderCompiler/issues/6745
Signed-off-by: Nathan Gauër <brioche@google.com>
* add commandline options
Signed-off-by: Nathan Gauër <brioche@google.com>
* clang-format
Signed-off-by: Nathan Gauër <brioche@google.com>
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
* Support SPV_KHR_untyped_pointers
Covers:
- assembler
- disassembler
- validator
fix copyright
Validate OpTypeUntypedPointerKHR
* Disallow an untyped pointer in a typed pointer
* Validate capability requirements for untyped pointer
* Allow duplicate untyped pointer declarations
Add round trip tests
Validate OpUntypedVariableKHR
Validate untyped access chains
* Add a test for opcodes that generate untyped pointers
* simplify some checks for operands needing types
* validate OpUnypedAccessChainKHR, OpUntypedInBoundsAccessChainKHR,
OpUntypedPtrAccessChainKHR, OpUntypedInBoundsPtrAccessChainKHR
Unify variable validation
Validate OpCopyMemorySized
* Fix some opcode tests to accound for untyped pointers
* Add validation for OpCopyMemorySized for shaders and untyped pointers
* fix up tests
Validate pointer comparisons and bitcast
* Update more helpers
* Fix entry validation to allow OpUntypedVariableKHR
* Validate OpPtrEqual, OpPtrNotEqual and OpPtrDiff
* Validate OpBitcast
Validate atomics and untyped pointers
Make interface variable validation aware of untyped pointers
* Check OpUntypedVariableKHR in interface validation
More untyped pointer validation
* Validate interfaces more thoroughly
* Validate layouts for untyped pointer uses
* Improve capability checks for vulkan with OpTypeUntypedPointerKHR
* workgroup member explicit layout validation updates
More validation
* validate function arguments and parameters
* handle untyped pointer and variable in more places
Add a friendly assembly name for untyped pointers
Update OpCopyMemory validation and tests
Fix test for token update
Fixes for validation
* Allow typed pointers to contain untyped pointers
* Fix decoration validation
* add untyped pointer as a case for size and alignments
Fix interface validation
* Grabbed the wrong storage class operand for untyped variables
* Add ability to specify assembler options in validation tests
Add passthrough validation for OpUntypedArrayLengthKHR
More validation of untyped pointers
* Validate OpUntypedArrayLengthKHR
* Validate layout for OpLoad, OpStore, and OpUntypedArrayLengthKHR
Validation support for cooperative matrix and untyped pointers
* Allow untyped pointers for cooperative matrix KHR load and store
Updates to match spec
* Remove extra capability references
* Swap untyped variable data type and storage class operands
* update validation of variables
* update deps
---------
Co-authored-by: David Neto <dneto@google.com>
* In spirv-val allow format arg to printf to be an array of i8 in Generic space
Signed-off-by: Lu, John <john.lu@intel.com>
* Allow more addr spaces for printf format string
Signed-off-by: Lu, John <john.lu@intel.com>
* Update printf format arg testcase
Signed-off-by: Lu, John <john.lu@intel.com>
* Apply clang-format
Signed-off-by: Lu, John <john.lu@intel.com>
* Reorder code for clarity
Signed-off-by: Lu, John <john.lu@intel.com>
* Only allow other addr spaces if extension is seen
Signed-off-by: Lu, John <john.lu@intel.com>
* Add test to check printf format with extension
Signed-off-by: Lu, John <john.lu@intel.com>
* Add extension correctly
Signed-off-by: Lu, John <john.lu@intel.com>
---------
Signed-off-by: Lu, John <john.lu@intel.com>
This patch adds the optional FPEncoding operand that can be added to OpTypeFloat.
At the moment there is no usable operand, so support is limited to adding the entry.
Co-authored-by: Kévin Petit <kevin.petit@arm.com>
Co-authored-by: David Neto <dneto@google.com>
* Add knowledge of cooperative matrices
Some optimizations are not aware of cooperative matrices, and either do
nothing or assert. This commits fixes that up.
* Add int tests, and a handle a couple more cases.
* Add float tests, and a handle a couple more cases.
* Add NV coop matrix as well.
The folding rule `BitCastScalarOrVector` was incorrectly handling
bitcasting to unsigned integers smaller than 32-bits. It was simply
copying the entire 32-bit word containing the integer. This conflicts with the
requirement in section 2.2.1 of the SPIR-V spec which states that
unsigned numeric types with a bit width less than 32-bits must have the
high-order bits set to 0.
This change include a refactor of the bit extension code to be able to
test it better, and to use it in multiple files.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/6319.
With --nested-indent, the SPIR-V blocks are nested according to the
structured control flow. Each OpLabel is nested that much with the
contents of the block nested a little more. The blocks are separated by
a blank line for better visualization.
With --reorder-blocks, the SPIR-V blocks are reordered according to the
structured control flow. This is particularly useful with
--nested-indent.
Note that with --nested-indent, the disassembly does not exactly show
the binary as-is, and the instructions may be reordered.
This pass fixups the opcode used for OpExtInst instructions
to use OpExtInstWithForwardRefsKHR when it contains a forward
reference.
This pass is agnostic to the extension used, hence the validity
of the code depends of the validity of the usage:
If a forward reference is used on a non-semantic extended instruction,
the generated code will remain invalid, but the opcode will change.
What this pass guarantees is valid code won't become invalid.
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
Co-authored-by: Steven Perron <stevenperron@google.com>
PR #5648 added support for the GroupNonUniformPartitionedNV. But there
was an issue: the opcodes are enabled by multiple capabilities, and the
actual operand is what matters.
Added testing coverage and the implementation to correctly trim a few
NonUniform capabilities.
Signed-off-by: Nathan Gauër <brioche@google.com>
The KHR suffix was missing from the published SPIR-V extension.
This is now fixed, but requires some patches in SPIRV-Tools.
Signed-off-by: Nathan Gauër <brioche@google.com>
* val, core: add support for OpExtInstWithForwardRefs
This commit adds validation and support for
OpExtInstWithForwardRefs. This new instruction will be used
for non-semantic debug info, when forward references are
required.
For now, this commit only fixes the code to handle this new instruction,
and adds validation rules. But it does not add the pass to generate/fix
the OpExtInst instruction when forward references are in use.
Such pass would be useful for DXC or other tools, but I wanted to land
validation rules first.
This commit also bumps SPIRV-Headers to get this new opcode.
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
* Avoid use of type manager in extact->construct folding
When dealing with structs the type manager merge two different structs
into a single entry if they have all of the same decorations and
element types. This is because they hash to the same value in the hash
table. This can cause problems if you need to get the id of a type from
the type manager because you could get either one. In this case, it
returns the wrong one.
The fix avoids using the type manager in one place. I have not
looked closely at other places the type manager is used to make
sure it is used safely everywhere.
Fixes#5624
* Remove use of TypeManager::GetId
This removes a use of TypeManager::GetId by keeping the id around. This
avoid a potential problem if the type manager gets confused. These types
of bugs are hard to generate test cases for, so I do not have a test.
However, existing tests make sure that do not regress.
* Basic support for SPV_EXT_replicated_composites
Validation will follow as a separate PR (still need to write a test suite)
Change-Id: Ic95fa6ce39d32f5ac2787bc38dba2748c9cc58f7
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
* Update SPIRV-Headers
Change-Id: I6c0df248d99c13b49d78528d035a4222027c0232
---------
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
This removes the folding rules added in #4783 and #4808. They lead to
poor code generation on Adreno devices when 16-bit floating point values
were used. Since this change is transformation is suppose to be neutral,
there is no general reason to continue doing it.
I have talked to the owners of SwiftShader, and they do not mind if the
transform is removed. They were the ones the requested the change in the
first place.
Fixes#5658
The Scope operand of `OpReadClockKHR` was always validated using the
Vulkan environment rules, which only allow `Subgroup` or `Device`.
For the OpenCL environment, `Workgroup` is also a valid Scope, so
`Workgroup` should not be rejected in the universal environment.
Guard the existing Scope check behind `spvIsVulkanEnv` and add a new
Scope check for the OpenCL environment.
Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
* Check for matrix decorations on arrays of matrices
* MatrixStide, RowMajor and ColMajor can be applied to matrix or
arrays of matrix members
* Check that matrix stride satisfies alignment in arrays
Reject `OpCooperativeMatrixStoreKHR` with a `MakePointerVisibleKHR`
MemoryAccess operand, as `MakePointerVisibleKHR` is not supposed to be
used with store operations.
The `CoopMatKHRStoreMemoryAccessFail` test failed to catch this
because it used the helper function `GenCoopMatLoadStoreShader` which
generates `...NV` instead of `...KHR` instructions. Add a new helper
function to generate similar shaders for the KHR extension, as the NV
and KHR extensions have various subtle differences that makes
parameterizing the original helper function non-trivial.
Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
* Disallow duplicate decorations generally
* Only FuncParamAttr and UserSemantic can be applied to the same target
multiple times
* Unchecked: completely duplicate UserSemantic and FuncParamAttr
* Disallow duplicate execution modes generally
* Exceptions for float controls, float controls2 and some intel
execution modes
* Fix invalid fuzzer transforms
This fixes the problem reported in #5623 using the observation that if
we are re-building a type that already exists in the type pool, we
should just return that type.
This makes type rebuilding more efficient, and it also prevents the
type builder from getting itself into infinite recursion (as reported in
this issue).
In fixing this, I found a couple of other bugs in the type builder:
- When rebuilding an Array type, we were not re-building the element
type. This caused stale type references in the rebuilt type.
- This bug had not been caught by the test, because the test itself had
a bug in it: the test was rebuilding types on top of the same ID (the
ID counter was never incremented).
Initially, the bug in the test caused a failure with the new logic in
the builder because we now return types from the pool directly, which
causes a failure when two incompatible types are registered under the
same ID.
Fixing that issue in the test exposed another bug in the rebuilder: we
were not re-building the element type for Array types. This was causing
a stale type reference inside Array types which was later caught by the
type removal logic in the test.
