Extends the pass for removing unused instructions so that it can
remove global declarations (such as types and variables) that are only
used by decorations with which they are intimately connected, such as
descriptor set and binding decorations.
* Do merge return if the return is not at the end of the function.
We will remove the code in inlining to handle a return in the middle of
a function. To inline those functions, we need to run merge return to
move the return to the end of the function.
Generalizes the IsReadOnlyVariable() method, and related methods, so
that they can be used to ask whether pointer result ids are read-only.
Fixes#3324.
Many high-level languages like HLSL and GLSL generate termination
instructions such as return and branch from the actual part of the
high-level language code like return and if statements. This commit lets
IrLoader set `DebugScope` for termination instructions.
The outliner would outline regions ending with a loop header, making
the block containing the call to the outlined function serve as the
loop header. This, however, is incorrect in general, since the whole
outlined function -- rather than just the exit block for the region --
would end up getting called every time the loop would iterate.
This change restricts the outliner so that the last block in a region
cannot be a loop header.
We need an analysis for OpenCL.DebugInfo.100 extension instructions such
as a map between function id and its DebugFunction. This commit add an
analysis for it.
It has been resolved that statically out-of-bounds accesses are not
invalid in SPIR-V (they lead to undefind behaviour at runtime but
should not cause a module to be rejected during validation). This
change tolerates such accesses in donated code, clamping them in-bound
as part of making a function live-safe.
The Sample argument of OpImageTexelPointer is sometimes required to be
a zero constant. It thus cannot be replaced with a synonym in
general. This change avoids replacing this argument with a synonym.
The fact manager maintains an equivalence relation on data descriptors
that tracks when one data descriptor could be used in place of
another. An algorithm to compute the closure of such facts allows
deducing new synonym facts from existing facts. E.g., for two 2D
vectors u and v it is known that u.x is synonymous with v.x and u.y is
synonymous with v.y, it can be deduced that u and v are synonymous.
The closure computation algorithm is very expensive if we get large
equivalence relations.
This change addresses this in three ways:
- The size of equivalence relations is reduced by limiting the extent
to which the components of a composite are recursively noted as
being equivalent, so that when we have large synonymous arrays we do
not record all array elements as being pairwise equivalent.
- When computing the closure of facts, equivalence classes above a
certain size are simply skipped (which can lead to missed facts)
- The closure computation is performed less frequently - it is invoked
explicitly before fuzzer passes that will benefit from data synonym
facts. A new transformation is used to control its invocation, so
that fuzzing and replaying do not get out of sync.
The change also tidies up the order in which some getters are declared
in FuzzerContext.
Adds an extra condition on when a region can be outlined to avoid the
case where a region ends with a loop head but such that the loop's
continue target is in the region. (Outlining such a region would mean
that the loop merge is in the original function and the continue target
in the outlined function.)
The function outliner uses a struct to return ids that a region
generates and that are used outside that region. If these ids have
pointer type this would result in a struct with pointer members, which
leads to illegal loading from non-logical pointers if logical
addressing is used. This change bans that outlining possibility.
Provides support for runtime arrays in the code that traverses
composite types when checking applicability of transformations that
replace ids with synonyms.
Demotes the image storage class to Private during donation. Also
fixes an issue where instructions that depended on non-donated global
values would not be handled properly.
The SPIR-V data rules say that all uses of an OpSampledImage
instruction must be in the same block as the instruction, and highly
restrict those instructions that can consume the result id of an
OpSampledImage.
This adapts the transformations that split blocks and create synonyms
to avoid separating an OpSampledImage use from its definition, and to
avoid synonym-creation instructions such as OpCopyObject consuming an
OpSampledImage result id.
* Preserve debug info in eliminate-dead-functions
The elimination of dead functions makes OpFunction operand of
DebugFunction invalid. This commit replaces the operand with
DebugInfoNone.
* Handle more cases in dead member elim
- Rewrite composite insert and extract operations on SpecConstnatOp.
- Leaves assert for Access chain instructions, which are only allowed
for kernels.
- Other operations do not require any extra code will no longer cause an
assert.
Fixes#3284.
Fixes#3282.
The management of equation facts suffered from two problems:
(1) The processing of an equation fact required the data descriptors
used in the equation to be in canonical form. However, during
fact processing it can be deduced that certain data descriptors
are equivalent, causing their equivalence classes to be merged,
and that could cause previously canonical data descriptors to no
longer be canonical.
(2) Related to this, if id equations were known about a canonical data
descriptor dd1, and other id equations known about a different
canonical data descriptor dd2, the equation facts about these data
descriptors were not being merged in the event that dd1 and dd2
were deduced to be equivalent.
This changes solves (1) by not requiring equation facts to be in
canonical form while processing them, but instead always checking
whether (not necessary canonical) data descriptors are equivalent when
looking for corollaries of equation facts, rather than comparing them
using ==.
Problem (2) is solved by adding logic to merge sets of equations when
data descriptors are made equivalent.
In addition, the change also requires elements to be registered in an
equivalence relation before they can be made equivalent, rather than
being added (if not already present) at the point of being made
equivalent.
This change increases the extent to which arbitrary SPIR-V can be used
by the fuzzer pass that donates modules. It handles the case where
various ingredients (such as types, variables and particular
instructions) cannot be donated by omitting them, and then either
omitting their dependencies or replacing their dependencies with
alternative instructions.
The change pays particular attention to allowing code that manipulates
image types to be handled (by skipping anything image-specific).
(1) Runtime arrays are turned into fixed-size arrays, by turning
OpTypeRuntimeArray into OpTypeArray and uses of OpArrayLength into
uses of the constant used for the length of the fixed-size array.
(2) Atomic instructions are not donated, and uses of their results are
replaced with uses of constants of the result type.
The fuzzer pass that constructs composites had an issue where it would
regard isomorphic but distinct structs (similarly arrays) as being
interchangeable when constructing composites. This change fixes the
problem by relying less on the type manager.
Some transformations (e.g. TransformationAddFunction) rely on running
the validator to decide whether the transformation is applicable. A
recent change allowed spirv-fuzz to take validator options, to cater
for the case where a module should be considered valid under
particular conditions. However, validation during the checking of
transformations had no access to these validator options.
This change introduced TransformationContext, which currently consists
of a fact manager and a set of validator options, but could in the
future have other fields corresponding to other objects that it is
useful to have access to when applying transformations. Now, instead
of checking and applying transformations in the context of a
FactManager, a TransformationContext is used. This gives access to
the fact manager as before, and also access to the validator options
when they are needed.
When DebugScope is given in SPIR-V, each instruction following the
DebugScope is from the lexical scope pointed by the DebugScope in
the high level language. We add DebugScope struction to keep the
scope information in Instruction class. When ir_loader loads
DebugScope/DebugNoScope, it keeps the scope information in
|last_dbg_scope_| and lets following instructions have that scope
information.
In terms of DebugDeclare/DebugValue, if it is in a function body
but outside of a basic block, we keep it in |debug_insts_in_header_|
of Function class. If it is in a basic block, we keep it as a normal
instruction i.e., in a instruction list of BasicBlock.
Create a pass to instrument OpDebugPrintf instructions. This pass replaces all OpDebugPrintf instructions with instructions to write a record containing the string id and the all specified values into a special printf output buffer (if space allows). This pass is designed to support the printf validation in the Vulkan validation layers.
Fixes#3210
In this PR, the classes that represent the toggle access chain
instruction transformation and fuzzer pass were implemented. This
transformation toggles the instructions OpAccessChain and
OpInBoundsAccessChain between them.
Fixes#3193.
This introduces a new fuzzer pass to add instructions to the module
that define equations, and support in the fact manager for recording
equation facts and deducing synonym facts from equation facts.
Initially the only equations that are supported involve OpIAdd,
OpISub, OpSNegate and OpLogicalNot, but there is scope for adding
support for equations over various other operators.
According to SPV_AMD_shader_image_load_store_lod spec, Lod operand is
valid with OpImageRead, OpImageWrite, or OpImageSparseRead if the
extension is enabled.
This change adds a fuzzer pass that sprinkles access chain
instructions into a module at random. This allows other passes to
have a richer set of pointers available to them, in particular the
passes that add loads and stores.
Adds a fuzzer pass that inserts function calls into the module at
random. Calls from dead blocks can be arbitrary (so long as they do
not introduce recursion), while calls from other blocks can only be to
livesafe functions.
The change fixes some oversights in transformations to replace
constants with uniforms and to obfuscate constants which testing of
this fuzzer pass identified.
This change ensures that global and local variables donated from other
modules are always initialized at their declaration in the module
being transformed. This is to help limit issues related to undefined
behaviour that might arise due to accessing uninitialized memory.
The change also introduces some helper functions in fuzzer_util to
make it easier to find the pointee types of pointer types.
This change adds fuzzer passes that sprinkle loads and stores into a
module at random, with stores restricted to occur in either dead
blocks, or to use pointers for which it is known that the pointee
value does not influence the module's overall behaviour.
The change also generalises the VariableValueIsArbitrary fact to
PointeeValueIsIrrelevant, to allow stores through access chains or
object copies of variables whose values are known to be irrelevant.
The change includes some other minor refactorings.
Adds two new fuzzer passes to add variables to a module: one that adds
Private storage class global variables, another that adds Function
storage class local variables.
If the fuzzer object-copies a pointer we would like to be able to
perform loads from the copy (and stores to it, if its value is known
not to matter). Undefined and null pointers present a problem here,
so this change disallows copying them.
This adds support for replacing TimeAMD with OpReadClockKHR. The scope
for OpReadClockKHR is fixed to be a subgroup because TimeAMD operates
only on subgroup.
* Implement constant folding for many transcendentals
This change adds support for folding of sin/cos/tan/asin/acos/atan,
exp/log/exp2/log2, sqrt, atan2 and pow.
The mechanism allows to use any C function to implement folding in the
future; for now I limited the actual additions to the most commonly used
intrinsics in the shaders.
Unary folder had to be tweaked to work with extended instructions - for
extended instructions, constants.size() == 2 and constants[0] ==
nullptr. This adjustment is similar to the one binary folder already
performs.
Fixes#1390.
* Fix Android build
On old versions of Android NDK, we don't get std::exp2/std::log2
because of partial C++11 support.
We do get ::exp2, but not ::log2 so we need to emulate that.
This change adds a new kind of fact to the fact manager, which records
when a variable (or pointer parameter) refers to an arbitrary value,
so that anything can be stored to it, without affecting the observable
behaviour of the module, and nothing can be guaranteed about values
loaded from it. Donated modules are the current source of such
variables, and other transformations, such as outlining, have been
adapted to propagate these facts appropriately.
This change allows the generator to (optionally and at random) make
the functions of a module "livesafe" during donation. This involves
introducing a loop limiter variable to each function and gating the
number of total loop iterations for the function using that variable.
It also involves eliminating OpKill and OpUnreachable instructions
(changing them to OpReturn/OpReturnValue), and clamping access chain
indices so that they are always in-bounds.
We must treat a branch to the merge node of a switch that is in the
header of a construct as a nested construced. The original merge
instruction is still needed in that case.
Fixes#3139
* If the header of the construct is also a merge block, jump to the
associated header instead of the immediate dominator
* prevents spurious failures from unrelated constructs
* new tests
This new API lets clients request a minimal spv_target_env value
that supports a given Vulkan and SPIR-V version, by a generic
numbering scheme (as already defined by Vulkan and SPIR-V specs).
This breaks a formal source dependency from Glslang to SPIRV-Tools.
When a new API is rolled out, such as Vulkan 1.2, Glslang currently
needs to reference a specific SPIRV-Tools enum by name.
* Allow OpExtInst for DebugInfo between secion 9 and 10
Fixes#3086
* Handle spirv-opt errors on DebugInfo Ext
* Add IR Loader test
* Fix ir loader bug
* Handle DebugFunction/DebugTypeMember forward reference
* Add test cases (forward reference to function)
* Support old DebugInfo extension
* Validate local debug info out of function
This change refactors some code for walking access chain indexes to
make it mirror the structure of other code (to improve readability in
the first instance and potentially enable a future refactoring to
extract common code), and fixes a problem related to module donation
and function types.
As explained in #3118, spirv-opt merge-blocks pass causes a
spirv-val error when an OpBranch has an OpLine in front of it.
OpLoopMerge
OpBranch ; Will be killed by merge-blocks pass
OpLabel ; Will be killed by merge-blocks pass
OpLine ; will be placed between OpLoopMerge and OpBranch - error!
OpBranch
To fix this issue, this commit moves line info of OpBranch to
OpLoopMerge.
Fixes#3118
This adds a new kind of fact to the fact manager that knows whether a
block is dead - i.e. guaranteed to be statically unreachable - and a
new transformation for adding a selection construct to a CFG that
conditionally branches to a fresh, dead block, such that the branch
will never be dynamically taken. Transformations that may create new
blocks ('split block' and 'outline function') are updated to propagate
dead block facts to newly-created blocks where appropriate. A fuzzer
pass randomly adds dead blocks to the module.
Future transformations will be able to exploit the fact that such
blocks are known to be dead.
This change adds a fuzzer pass that allows code from other SPIR-V
modules to be donated into the module under transformation. It also
changes the command-line options of the tools so that, in fuzzing
mode, a file must be specified that contains the names of available
donor modules.
* Clone opencl.debuginfo.100 grammar from debuginfo grammar
Update version number to 200 revision 2
* Apply content from OpenCL.DebugInfo.100 extension text
* Rename grammar file
* Support OpenCL.DebugInfo.100 extended instructions
Add support for prefixing operand type names, to disambiguate
them between different instruction sets.
* Add tests for OpenCL.DebugInfo.100
* Support lookup of OpenCL.DebugInfo.100 extinst
* Add tests for enum values
* Recognize 2017-2019 as copyright date range
* Android.mk: support OpenCL.DebugInfo.100 extended instruction set
Also, stop generating core instruction tables for non-unified1 versions
of the grammar.
* Imported entity operand type is concrete
* Bazel: Suppoort OpenCL.DebugInfo.100
* BUILD.gn: Support OpenCL.DebugInfo.100
In the context of SPIR-V 1.4 or higher, global variables cannot be
used by an instruction unless they are listed in the interface of all
entry points that might invoke the instruction. This change
conservatively adds new global variables to the interfaces of all
entry points (if the SPIR-V version is 1.4 or higher).
Issue #3111 notes that a more rigorous approach to entry point
interfaces could be taken in spirv-fuzz, which would allow being less
conservative here.
This change prevents the spirv-fuzz function outliner from outlining a
region that uses the result of an OpAccessChain not defined inside the
region. Such accesses were turning into parameters to the outlined
function, and the result of an OpAccessChain cannot be passed as a
function parameter according to the SPIR-V specification.
Add support for SPV_KHR_non_semantic_info
This entails a couple of changes:
- Allowing unknown OpExtInstImport that begin with the prefix `NonSemantic.`
- Allowing OpExtInst that reference any of those sets to contain unknown
ext inst instruction numbers, and assume the format is always a series of IDs
as guaranteed by the extension.
- Allowing those OpExtInst to appear in the types/variables/constants section.
- Not stripping OpString in the --strip-debug pass, since it may be referenced
by these non-semantic OpExtInsts.
- Stripping them instead in the --strip-reflect pass.
* Add adjacency validation of non-semantic OpExtInst
- We validate and test that OpExtInst cannot appear before or between
OpPhi instructions, or before/between OpFunctionParameter
instructions.
* Change non-semantic extinst type to single value
* Add helper function spvExtInstIsNonSemantic() which will check if the extinst
set is non-semantic or not, either the unknown generic value or any future
recognised non-semantic set.
* Add test of a complex non-semantic extinst
* Use DefUseManager in StripDebugInfoPass to strip some OpStrings
* Any OpString used by a non-semantic instruction cannot be stripped, all others
can so we search for uses to see if each string can be removed.
* We only do this if the non-semantic debug info extension is enabled, otherwise
all strings can be trivially removed.
* Silence -Winconsistent-missing-override in protobufs
* Make Instrumentation format version 2 the default (Step 1)
Add new interfaces without version number argument. Remove version 1
logic and tests. Version interfaces will be removed in step 2 after
layers have transitioned to new interface.
* Add error messages to InstrumentPass().
* Don't crash when folding construct of empty struct
An OpCompositeConstruct of an empty struct will be folded to a constant
under normal circumstances. However, if the id limit has been reached
and the constant cannot be generated, then other folding rules will be
tried.
These rules do not handle the case of an empty struct. We add allow it
to be handled.
Fixes http://crbug/1030194
* Changes based on the review.
A new transformation and associated fuzzer pass in spirv-fuzz that
selects single-entry single-exit control flow graph regions and for
each selected region outlines the region into a new function and
replaces the original region with a call to this function.
The passes that add dead breaks and continues suffer from the
challenge that a new control flow graph edge can change dominance
information, leading to the potenital for definitions to no longer
dominate their uses. The attempt at guarding against this was known
to be incomplete. This change calls on the SPIR-V validator to do the
necessary checking: in deciding whether adding such an edge would be
legitimate, we clone the module, add the edge, and use the validator
to check whether the transformed clone is valid.
This strategy is heavy-weight, and should be used sparingly, but seems
like a good option when the validity of transformations is intricate,
to avoid reimplementing swathes of validation logic in the fuzzer.
Fixes#2919.
Access chain indices are always interpreted as signed integers.
So use signed clamp instead of unsigned clamp. We must also
clamp to the max signed int for the index type.
Fixes#3072
* Validate that if a construct contains a header and it's merge is
reachable, the construct also contains the merge
* updated block merging to not merge into the continue
* update inlining to mark the original block of a single block loop as
the continue
* updated some tests
* remove dead code
* rename kBlockTypeHeader to kBlockTypeSelection for clarity
Adds an option to run the validator on the SPIR-V binary after each
fuzzer pass has been applied, to help identify when the fuzzer has
made the module invalid. Also adds a helper method to allow dumping
of the sequence of transformations that have been applied to a JSON
file.
Prior to this change, TransformationReplaceIdWithSynonym was designed
to be able to replace an id with some synonymous data descriptor,
possibly necessitating extracting from a composite into a fresh id in
order to get at the synonymous data. This change simplifies things so
that TransformationReplaceIdWithSynonym just allows one id to be
replaced by another id. It is the responsibility of the associated
fuzzer pass - FuzzerPassApplyIdSynonyms - to perform the extraction
operations, using e.g. TransformationCompositeExtract.
Re-enable OpReadClockKHR validation
Fixes#2952
* Refactor some common scope validation
* Perform correct validation for scope in OpReadClockKHR
* Scope must be Subgroup or Device
* new tests
Inroduces a new transformation that adds a vector shuffle instruction
to the module, with associated facts about how the result vector of
the shuffle relates to the input vectors.
A fuzzer pass to add such transformations is not yet in place.
When a data synonym fact about two composites is added, data synonym
facts between all sub-components of the composites are also added.
Furthermore, when data synonym facts been all sub-components of two
composites are known, a data synonym fact relating the two composites
is added. Identification of this case is done in a lazy manner, when
questions about data synonym facts are asked.
The change introduces helper methods to get the size of an array type
and the number of elements of a struct type, and fixes
TransformationCompositeExtract to invalidate analyses appropriately.
An equivalence relation is computed by traversing the tree of values
rooted at the class's representative. Children were represented by
unordered sets, meaning that the order of values in an equivalence
class could be nondeterministic. This change makes things
deterministic by representing children using a vector.
The path compression optimization employed in the implementation of
the underlying union-find data structure has the potential to change
the order in which elements appear in an equivalence class by changing
the structure of the tree, so the guarantee of determinism is limited
to being a deterministic function of the manner in which the
equivalence relation is updated and inspected.
This change fixes a bug in EquivalenceRelation, changes the interface
of EquivalenceRelation to avoid exposing (potentially
nondeterministic) unordered sets, and changes the interface of
FactManager to allow querying data synonyms directly. These interface
changes have required a lot of corresponding changes to client code
and tests.
Implements the following simplifications:
(a - b) + b => a
(a * b) + (a * c) => a * (b + c)
Also adds logic to simplification to handle rules that create new operations
that might need simplification, such as the second rule above.
Only perform the second simplification if the multiplies have the add as their
only use. Otherwise this is a deoptimization of size and performance.
At present, TransformationReplaceIdWithSynonym both extracts elements
from composite objects and replaces uses of ids with synonyms. This
new TransformationCompositeExtract class will allow that
transformation to be broken into smaller transformations.
Class TransformationConstructComposite has been renamed to
TransformationCompositeConstruct, to correspond to the name of the
SPIR-V instruction (as is done with e.g. TransformationCopyObject).
Running tests revealed an issue related to checking dominance in
TransformationReplaceIdWithSynonym, which is also fixed here.
This change uses the recently-added equivalence relation class to
re-work the way synonyms between data values are managed by the fact
manager.
The tests for 'transformation_replace_id_with_synonym' have been
temporarily removed. This is because those tests are going to be
split into a number of test classes in an upcoming PR, once some other
refactorings have been applied, and it would be burdensome to
temporarily refactor all the tests to be in a working state for this
intermediate change.
Adds a templated class for representing an equivalence relation on a
value data type. This will be used by spirv-fuzz for representing
sets of distinct pieces of data in a shader that are known to have
equal values.
A new pass that gives spirv-fuzz the ability to adjust the memory
operand masks associated with memory access instructions (such as
OpLoad and OpCopy Memory).
Fixes#2940.
We have a check that ensures that the optimizer did not change the
binary when it says that it did not. However, when the binary is
converted back to a binary, we made a decision to remove OpNop
instructions. This means that any spv file that contains a NOP
originally will fail this check.
To get around this, we convert the module to a second binary that keeps
the OpNop instructions. That binary is compared against the original.
Fixes https://crbug.com/1010191
This change refactors the 'split blocks' transformation so that an
instruction is identified via a base, opcode, and number of those
opcodes to be skipped when searching from the base, as opposed to the
previous design which used a base and offset.
* Validate that selections are structured
WIP
* new checks that switch and conditional branch are proceeded by a
selection merge where necessary
* Don't consider unreachable blocks
* Add some tests
* Changed how labels are marked as seen
* Moved check to more appropriate place
* Labels are now marked as seen when there are encountered in a
terminator instead of when the block is checked
* more tests
* more tests
* Method comment
* new test for a bad case
A refactoring that separates the identification of an instruction from
the identification of a use in an instruction, to enable the former to
be used independently of the latter.
A new pass that allows the fuzzer to change the 'loop control' operand
(and associated literal operands) of OpLoopMerge instructions.
Fixes#2938.
Fixes#2943.
Adds a fuzzer pass and transformation to create a composite (array,
matrix, struct or vector) from available constituent components, and
inform the fact manager that each component of the new composite is
synonymous with the id that was used to construct it. This allows the
"replace id with synonym" pass to then replace uses of said ids with
uses of elements extracted from the composite.
Fixes#2858.
* Remove Impl struct in Reducer; we can re-add it later (in a cleaner fashion) if we need to.
* Add cleanup passes in Reducer; needed so that removal of constants can be disabled during the main passes, and then enabled during cleanup passes, otherwise some main passes can perform worse due to lack of available constants.
* Delete passes: remove op name, remove relaxed precision. And delete associated tests.
* Add more tests for remove unreferenced instructions.
* Always return and write the output file, even if there was a reduction failure.
* Only exit with 0 if the reduction completed or we hit the reduction step limit.
Issue #2919 identifies a problem in spirv-fuzz's ability to determine
when it is safe to add a new control flow edge without breaking
dominance rules. This change adds a (currently disabled) test to
expose the issue, and a comment to document that the current solution
is incomplete.
We want to handle OpKill better. The wrap opkill causes lots of extra
code to be generated, even when they are not needed to avoid the main
problem: OpKill cannot be found directly in a continue construct.
This change will be more selective on which functions the OpKill will be
wrapped and inlining will avoid inlining.
Fixes#2912
* Add continue construct analysis to struct cfg analysis
Add the ability to identify which blocks are in the continue construct for a
loop, and to get functions that are called from those blocks, directly or
indirectly.
Part of https://github.com/KhronosGroup/SPIRV-Tools/issues/2912.
There is nothing in the spir-v spec that says the last
instructions in a module cannot be OpLine or OpNoLine.
However, the code that parses the module will simply drop
these instructions.
We add code that will preserve these instructions.
Strip-debug-info is updated to remove these instructions.
Fixes https://crbug.com/1000689.
Because dominance information becomes a bit unreliable when blocks are
unreachable, this change makes it so that the 'dead break'
transformation will not introduce a break to an unreachable block.
Fixes#2907.
This change introduces a robust check for whether an index in an
access chain is indexing into a struct, in which case the index needs
to be an OpConstant and cannot be replaced with a synonym.
Fixes#2906.
Issues #2898 and #2900 identify some cases where adding a dead
continue would lead to an invalid module, and these turned out to be
due to the lack of sensible dominance information when a continue
target is unreachable. This change requires that the header of a loop
dominates the loop's continue target if a dead continue is to be
added.
Furthermore, issue #2905 identified a shortcoming in the algorithm
being used to identify when it is OK, from a dominance point of view,
to add a new break/continue edge to a control flow graph. This change
replaces that algorithm with a simpler and more obviously correct
algorithm (that incidentally does not require the new edge to be a
break/continue edge in particular).
Fixes#2898.
Fixes#2900.
Fixes#2905.
Before this change, spirv-fuzz would replace a pointer argument to a
function call with a synonym, which is problematic when the synonym is
not a memory object declaration, since function call arguments are
required to be memory object declarations. This change adds a check
to ensure that such a replacement is not made.
Fixes#2896.
Before this change, spirv-fuzz would replace a constant boolean
argument to an OpPhi with the result of a binary operation, inserting
the instruction to compute the binary operation right before the
OpPhi, leading to an invalid module. This change conservatively
disallows replacing OpPhi arguments. Issue #2902 notes that there is
scope for being less conservative.
Fixes#2897.
* Handle extract with no indexes
It is possible that OpCompositeExtract instructions will not have any
indexes. This is not handled well by scalar replacement and instruction
folding.
Fixes https://crbug.com/1006435
* Fix typo.
This change to spirv-fuzz uses ideas from "Swarm Testing" (Groce et al. 2012), so that a random subset of fuzzer passes are enabled. These passes are then applied repeatedly in a randomized fashion, with the aggression with which they are applied being randomly chosen per pass.
There is plenty of scope for refining the probabilities introduce in this change; this is just meant to be a reasonable first effort.
* Use OpReturn* in wrap-opkill
The warp-opkill pass is generating incorrect code. It is placing an
OpUnreachable at the end of a basic block, when the block can be
reached. We can't reach the end of the block, but we can reach the end.
Instead we will add a return instruction.
Fixes#2875.
A previous change that disabled long-running tests by default failed
to enable short-running tests when long-running tests are enabled.
This change fixes that problem.
To aid in debugging issues in spirv-fuzz, this change adds an option whereby the SPIR-V module is validated after each transformation is applied during replay. This can assist in finding a transformation that erroneously makes the module invalid, so that said transformation can be debugged.
spirv-fuzz has useful tests that run the fuzzer and shrinker, to give
the whole tool a good shake up, effectively "fuzzing the fuzzer". The
problems that this detects are sensitive to the source of randomness
that is used, which can change from test platform to test platform.
It is thus not a good idea to run these tests by default during
continuous integration - they may end up failing due to environtal
factors, making it look like an unrelated change has broken the fuzzer
when really the fuzzer has revealed an already-existing bug in itself.
This change makes the tests disabled by default; they can enabled
during dedicated testing of the fuzzer.
The warp-opkill pass is generating incorrect code. It is placing an
OpUnreachable at the end of a basic block, when the block can be
reached. We can't reach the end of the block, but we can reach the end.
Instead we will add a return instruction.
Fixes#2875.
Many of the places in copy propagate arrays assumes that integer constant will be defined by an OpConstant instruction. That is not always true. We fix these spots by allowing for an OpConstantNull.
If the fuzzer's fact manager knows that ids A and B are synonymous, it
can replace a use of A with a use of B, so long as various conditions
hold (e.g. the definition of B must dominate the use of A, and it is
not legal to replace a use of an OpConstant in a struct's access chain
with a synonym that is not an OpConstant).
This change adds a fuzzer pass to sprinke such synonym replacements
through the module.
* When input or result is a pointer type also allow 32-bit integer
vectors for the other type
* Relaxation only applies to SPIR-V 1.5 or in the presence of
SPV_KHR_physical_storage_buffer
* new tests
* Vulkan specific checks
* storage buffer variables must be structs or arrays of structs
* storage buffer struct must be Block decorated
* uniform struct must be Block or BufferBlock decorated
* new tests
* Ensure same enum values have consistent extension lists
* val: fix checking of capabilities
The operand for an OpCapability should only be
checked for the extension or core version.
The InstructionPass registers a capability, and all its implied
sub-capabilities before actually checking the operand to an
OpCapability.
* Add basic support for SPIR-V 1.5
- Adds SPV_ENV_UNIVERSAL_1_5
- Command line tools default to spv1.5 environment
- SPIR-V 1.5 incorporates several extensions. Now the disassembler
prefers outputing the non-EXT or non-KHR names. This requires
updates to many tests, to make strings match again.
- Command line tests: Expect SPIR-V 1.5 by default
* Test validation of SPIR-V 1.5 incorporated extensions
Starting with 1.5, incorporated features no longer require
the associated OpExtension instruction.
If an OpKill instruction is inlined into a continue construct, then the
spir-v is no longer valid. To avoid this issue, we do inline into an
OpKill at all. This method was chosen because it is difficult to keep
track of whether or not you are in a continue construct while changing
the function that is being inlined into. This will work well with wrap
OpKill because every will still be inlined except for the OpKill
instruction itself.
Fixes#2554Fixes#2433
This reverts commit aa9e8f5380.
The implementation of these passes had overlooked the fact that adding
a new edge to a control flow graph can change dominance information.
Adding a dead break/continue risks causing uses to no longer be
dominated by their definitions. This change introduces various tests
to expose such scenarios, and augments the preconditions for these
transformations with checks to guard against the situation.
* Handle id overflow in the ssa rewriter.
Remove LocalSSAElim pass at the same time. It does the same thing as the SSARewrite pass. Then even share almost all of the same code.
Fixes crbug.com/997246
The first pass applies the RelaxedPrecision decoration to all executable
instructions with float32 based type results. The second pass converts
all executable instructions with RelaxedPrecision result to the equivalent
float16 type, inserting converts where necessary.
Add the first steps to removing the AMD extension VK_AMD_shader_ballot.
Splitting up to make the PRs smaller.
Adding utilities to add capabilities and change the version of the
module.
Replaces the instructions:
OpGroupIAddNonUniformAMD = 5000
OpGroupFAddNonUniformAMD = 5001
OpGroupFMinNonUniformAMD = 5002
OpGroupUMinNonUniformAMD = 5003
OpGroupSMinNonUniformAMD = 5004
OpGroupFMaxNonUniformAMD = 5005
OpGroupUMaxNonUniformAMD = 5006
OpGroupSMaxNonUniformAMD = 5007
and extentend instructions
WriteInvocationAMD = 3
MbcntAMD = 4
Part of #2814
* Refactor instruction folders
We want to refactor the instruction folder to allow different sets of
rules to be added to the instruction folder. We might want different
sets of rules in different circumstances.
We also need a way to add rules for extended instructions. Changes are
made to the FoldingRules class and ConstFoldingRules class to enable
that.
We added tests to check that we can fold extended instructions using the
new framework.
At the same time, I noticed that there were two tests that did not tests
what they were suppose to. They could not be easily salvaged. #2813 was
opened to track adding the new tests.
Adds a reduction pass that removes OpDecorate and OpMemberDecorate
instructions that annotate instructions and members with
RelaxedPrecision. As well as being useful in its own right, removing
such references allows other passes to remove further instructions.
Now we need to handle id overflow when we overflow while replacing uses of the variable. While looking at this code, I noticed an error in the way we handle access chains that cannot be replaced because of overflow. Name it will make some change, and then give up by returning SuccessWithoutChange. But it was changed.
This is fixed up by returning Failure if we notice the error at the time of rewriting the users. This is for both id overflow or out-of-bounds accesses.
Code is added to "CheckUses" to remove variables that have out-of-bounds accesses from the candidate list, so we don't even try to rewrite its uses.
Fixes https://crbug.com/995032
If we run out of ids when creating a new variable, sroa does not recognize
the error, and continues doing work. This leads to segmentation faults.
Fixes https://crbug/969655
Fixes#2793
* Don't special case matrix validation compared to other composites
* just check the constituents are constants or undefs
* later checking validates the column type
* new test
We are no able to inline OpKill instructions into a continue construct.
See #2433. However, we have to be able to inline to correctly do
legalization. This commit creates a pass that will wrap OpKill
instructions into a function of its own. That way we are able to inline
the rest of the code.
The follow up to this will be to not inline any function that contains
an OpKill.
Fixes#2726
This also fixes ADCE to not remove possibly needed OpTypeForwardPointer.
The bug, its fix and the corresponding test have a circular dependency
with the extension, so they are packaged together.
If a member of a struct has a relaxed precision, sroa will not split the
struct. This means we do not get all cases. This commit handles these
cases. The other part is that the decoration needs to be passed on to
the new variables.
Fixes#2786
This transformation can introduce an instruction that uses
OpCopyObject to make a copy of some other result id. This change
introduces the transformation, but does not yet introduce a fuzzer
pass to actually apply it.
Fixes#2768
* In scalar replacement, interpret access chain indexes as signed counts
* Use Constant::GetSignExtendedValue and Constant::GetZeroExtendedValue
where appropriate
* new tests
spirv-opt: Add --graphics-robust-access
Clamps access chain indices so they are always
in bounds.
Assumes:
- Logical addressing mode
- No runtime-array-descriptor-indexing
- No variable pointers
Adds stub code for clamping coordinate and samples
for OpImageTexelPointer.
Adds SinglePassRunAndFail optimizer test fixture.
Android.mk: add source/opt/graphics_robust_access_pass.cpp
Adds Constant::GetSignExtendedValue, Constant::GetZeroExtendedValue
This fixes#2608.
The original test case had an out-of-bounds reference that ended up
folding into OpCompositeExtract that was indexing right outside the
constant composite.
The returned constant would then cause a segfault during constant
propagation.
Fixes#2764
* Don't replace all uses when simplifying instructions, instead only
update non-debug, non-decoration uses
* added a test
* Add a new version of RAUW that takes a predicate to decide whether to
replace the use or not
* used in simplification pass
* Fix#2609 - Handle out-of-bounds scalar replacements.
When SROA tries to do a replacement for an OpAccessChain that is exactly
one element out of bounds, the code was trying to access its internal
array of replacements and segfaulting.
This protects the code from doing this, and it additionally fixes the
way SROA works by not returning failure when it refuses to do a
replacement. Instead of failing the optimization pass, SROA will now
simply refuse to do the replacement and keep going.
Additionally, this patch fixes the SROA logic to now return a proper status so we can
correctly state that the pass made no changes to the IR if it only found
invalid references.
The recently added fuzzer_replayer and fuzzer_shrinker tests were
rather heavyweight and were leading to CI timeouts. This change
reduces the runtime of those tests by having them do fewer iterations.
Merge return expects unreachable merge block to look a certain way, and
unreachable continue blocks to look a certain way. What if an
unreachable block is both a merge and a continue? The continue is
suppose to take precedent, but merge-return implements it with the merge
taking precedent. This change flips that around.
Fixes#2746
Similar to the existing 'add dead breaks' pass, this adds a pass to
add dead continues to blocks in loops where such a transformation is
viable. Various functionality common to this new pass and 'add dead
breaks' has been factored into 'fuzzer_util', and some small
improvements to 'add dead breaks' that were identified while reviewing
that code again have been applied.
Fixes#2719.
* Process OpDecorateId in ADCE
When there is an OpDecorateId instruction that is live,
the ids that is references must be kept live. This change
adds them to the worklist.
I've also updated a validator check to allow OpDecorateId
to be able to apply to decoration groups.
Fixes#1759.
* Remove dead code.
In merge return, we need to know the original dominator for a block in order to
traverse code from the original dominator to the new dominator and add
appropriate Phi nodes. The current code gets this wrong because the dominator
tree is build as needed. The first time we get the immediate dominator for a
function we just built the dominator tree and it takes into account that a
block has been split. The second time it does not.
This inconsistency needs to be fixed. We do that by recording the original
dominator for all blocks at the start of the pass.
If we were to record just the basic block, that could change if the block is
split. We want to traverse the code in the body of the original dominator,
whatever block it ends up in. To make this easy to track, we not save the
terminator instruction to represent the original dominator.
Fixes#2745
When a phi candidate is marked as trivial, we are suppose to update all
of its uses to the reference the value that it is being folded to.
However, the code updates the uses misses `defs_at_block_`. So at a
later time, the id for the trivial phi can reemerge.
Fixes#2744
* Bindless Instrument: Make init check depend solely on input_init_enabled
Previously was dependent on presense of descriptor_indexing extension
in SPIR-V, but this missed some cases. Tests updated to refect this new
policy.
* Fix format.
This change refactors all storage class validation for atomics
to reflect the similar refactoring in the specification.
It is currently not possible to write a test for the check
rejecting Generic in an OpenCL 1.2 environment as the required
GenericPointer capability isn't allowed there. I've decided
to keep the check nonetheless to guard against the capability
becoming available without the rules for atomics being updated.
The ID changes in existing tests aren't ideal but introducing
names drags in a substantial refactoring of this file.
Contributes to #2595.
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
* Fix bug in merge return
The merge return pass seems to assume that the only new edges in the cfg
are from return block to merge blocks. However, it is possible that a
merge block branches to a merge block when it did not before.
This change add a new variable to track all of the new edges. It also
renames some other variables and cleans us the code to make it a bit
easier to read.
Fixes#2702.
Dead branch elimination needs to know about the constructs that a block is contained it when determining what to do with its merge instruction. We currently fold branches in block as we see them, which is parent constructs before their children. This causes the struct cfg analysis to crash because it tries to get the parent construct for a block after the parent has been folded.
This can be fixed by folding the branch of the children before the parents.
Fixes#2667.
There are a couple spots where we are not looking at decorations when we should.
1. Value numbering is suppose to assign a different value number to ids if they have different decorations. However that is not being done for OpCopyObject and OpPhi.
1. Instruction simplification is propagating OpCopyObject instruction without checking for decorations. It should only do that if no decorations are being lost.
Add a new function to the decoration manager to check if the decorations of one id are a subset of the decorations of another.
Fixes#2715.
Fixes#2669
* Check capabilities when validating variables
* validate load and store types
* Constant check
* Don't checks pointers for stores, constants and loads
* Validate composite instructions
* Validate conversions for 8- and 16-bit limited types
* Unified tests and expanded them
* Disallow OpCopyMemory
* new tests and update old tests
Adds to spirv-fuzz the option to shrink a sequence of transformations
that lead to an interesting binary to be generated, to find a smaller
sub-sequence of transformations that still lead to an interesting (but
hopefully simpler) binary being generated. The notion of what counts
as "interesting" comes from a user-provided script, the
"interestingness function", similar to the way the spirv-reduce tool
works. The shrinking process will give up after a maximum number of
steps, which can be configured on the command line.
Tests for the combination of fuzzing and shrinking are included, using
a variety of interestingness functions.
Inlining does not inline functions that have a single return that is in a loop. This is because the return cannot be replaced by a branch outside of the loop easily. Merge return knows how to rewrite the function so the return is replaced by a branch.
Fixes#2038.
It is illegal to inline an OpKill instruction into a continue construct because the continue header will no longer dominate the backedge.
This commit adds a check for this, and does not inline.
If we still want to be able to inline a function that contains an OpKill, we can add a new pass that will wrap OpKill instructions into its own function with just the single instruction.
I do not believe that this is a common case right now, so I will not do that yet.
Fixes#2433.
When working on descriptor indexing validation for compute shaders, the
gl_GlobalInvocationID builtin was being loaded as uint which would cause
compute shaders instrumented by the bindless check pass to have:
%83 = OpLoad %uint %gl_GlobalInvocationID
%84 = OpCompositeExtract %uint %83 0
%85 = OpCompositeExtract %uint %83 1
%86 = OpCompositeExtract %uint %83 2
which results in validation failures:
error: line 127: Reached non-composite type while indexes still remain
to be traversed.
%84 = OpCompositeExtract %uint %83 0
for trying to extract a uint from a uint.
Fixes#2695. Allowing unreachable blocks to be moved can lead to an
unreachable block A getting placed after an unreachable successor B,
which is a problem if B uses ids that A generates.
* Represent uniform facts via descriptor set and binding.
Previously uniform facts were expressed with resepect to the id of a
uniform variable. Describing them with respect to a descriptor set
and binding is more convenient from the point of view of expressing
facts about a shader without requiring analysis of its SPIR-V.
* Fix equality testing for uniform buffer element descriptors.
The equality test now checks that the lengths of the index vectors
match. Added a test that exposes the previous omission.
Adds a new transformation that can replace a constant with a uniform known to have the same value, and adds a fuzzer pass that (a) replaces a boolean with a comparison of literals (e.g. replacing "true" with "42 > 24"), and then (b) obfuscates the literals appearing in this comparison by replacing them with identically-valued uniforms, if available.
The fuzzer_replayer test file has also been updated to allow initial facts to be provided, and to do error checking of the status results returned by the fuzzer and replayer components.
* Can only be used with Vulkan memory model
* Can only be used with atomics
* Bit setting must match for compare exchange opcodes
* Updated memory semantics checks to allow constant instructions
generally with CooperativeMatrixNV
The replayer takes an existing sequence of transformations and applies
them to a module. Replaying a sequence of transformations that were
obtained via fuzzing should lead to an identical module to the module
that was fuzzed. Tests have been added to check for this.
Adds a new (and first) kind of fact to the fact manager, which is that
a specific uniform value is guaranteed to be equal to a specific
constant. The point of this is that such information (if known to be
true by some external source) can be used by spirv-fuzz to transform
the module in interesting ways that a static compiler cannot reverse
via compile-time analysis.
This change introduces protobuf messages for the fact, and adds
capabilities to the fact manager to store this kind of fact and
provide information about it.
The transformation can, for example, replace "true" with "12.0 > 6.0",
if constants for those floating-point values are available.
This introduces a new 'id use descriptor' structure, which provides a
way to describe a particular use of an id, and which will be heavily
used in future transformations. Describing an id use is trivial if
the use occurs in an instruction that itself generates an id, but is
less straightforward if the id of interest is used by an instruction
such as OpStore that does not have a result id. The 'id use
descriptor' structure caters for such cases.
Also add a Builtin test generator variant that takes
capabilities and extensions.
Tests
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
accepted as Inputs in Vertex, Fragment, TessControl, TessEval, Geometry,
and Compute.
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
accepted as Inputs in MeshNV and TaskNV shaders.
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
accepted as Inputs in the 6 ray tracing stages
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
NOT accepted as Outputs.
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
NOT accepted as non-scalar integers (f32, uvec3)
- verify that the SMCountNV, SMIDNV, WarpsPerSMNV, and WarpIDNV Builtins are
NOT accepted as non-32-bit integers (u64)
There turned out to be a bug in the 'split blocks' transformation due
to blocks being split while they were being iterated over. This
change fixes that issue, and adds tests that were able to expose the
issue by running the fuzzer on some example shaders.
When it's an OpConstant or OpSpecConstant, then the literal
values are compared. If the OpSpecConstant also has a SpecId
decoration, then that's also compared.
Otherwise, it's an OpSpecConstantOp and we only compare the
ID of the OpSpecConstantOp instruction itself.
Fixes#2649
This new pass adds some basic ingredients to a module on which future
passes are likely to depend, such as boolean constants and some
specfic integer and floating-point values. This is not a fuzzer pass
in the true sense in that it does not employ randomization, but it
makes sense to define it as a fuzzer pass since it is the first of a
number of transformations passes that the fuzzer will run on a module.
* Types: Avoid comparing IDs for in Type::IsSameImpl
When linking, we end up with duplicate types for imported and exported
types, that needs to be removed. The current code would reject valid
import/export pairs of symbols due to IDs mismatch, even if the types or
constants behind those ID were the same.
Enabled remaining type_match_test
Fixes#2442
New version has additional word in stage-specific section. Also
some changes in content for tesselation and compute shaders. Either
version can be invoked at pass creation. This is done to ease integration
and updating of validation layers. Version 1 is deprecated and eventually
will go away.
Also sneaking in fix to version 1 compute shaders.
With this pass, the fuzzer can split blocks in the input module. This
is mainly useful in order to give other (future) transformations more
opportunities to apply.
* Handle nested breaks from switches.
There was a recent decision made to allow branches to the merge node of
a switch even if the switch is not the first enclosing construct. They
can be generated by glslang from break statements in switches.
Dead branch elimination seems to be the only optimization that will
break because of this change, so I will update that optimizations.
The change made are:
- Track switches in structured cfg analysis.
- In Dead branch elimination:
- Look for nested breaks that will require a switch instruction.
- Rewrite, but don't delete, switchs that are required even if it
could be replaced by an unconditional branch.
- When looking for the first break, consider the merge of a switch
as well.
See #2612.
* Fix variable names and comments.
* Add tests for the struct cfg analysis and switches.
* Fix typos in comments.
This adds a number of tests that check that all types will match to
identically written clones during linking, including nearly every Type
and some combinations (e.g. Functions of Arrays of Floats). Intent is
for use with https://github.com/KhronosGroup/SPIRV-Tools/pull/2580,
however that PR focuses on issues with TypeArray whereas these tests are
(more) comprehensive and test more subtle (and possibly incorrect)
cases.
A number of these tests fail, many are fixed by the aforementioned PR.
Some additional tests involving TypeForwardPointer are currently
disabled as they cause assertion failures.
Fixes#2604
* Allow selection constructs to branch to the nearest selection merge
whose header is terminated by an OpSwitch
* Cleanup break and continue checks generally
* add tests
In order to try to reduce code duplication and to be able
to fold more cases, we want to use the instruction folder
when folding an OpSpecConstantOp with constant operands.
A couple other changes are need to make this work. First
GetDefiningInstruction| in the constant manager is able
to handle |type_id| being logically equivalent to another
type, so we updated the interface, and removed the assert.
Some tests were also updated because we not generate
better code because constants are not duplicated as much
as before.
No need for new tests. The functionality of the instruction folder is
already tested. There are tests check that the instruction folder is
being used correctly for OpCompositeExtract and OpVectorShuffle in the
existing test cases.
Fixes#2585.
It is currently not possible to use an Image Format that is
not Unknown without requiring a capability forbidden by the
OpenCL environment. As such the validation of Image Format
currently leans on capability validation entirely.
Fixes#2592.
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Validate structured exits from constructs
* Add checks that exits from a construct are valid
* Add Construct::IsStructuredExit()
* uses specific rules for each type of construct
* Added a test and check for #2213
* Adding tests for bad loop and continue exits
* Fix identification of continue block that prevented some selections
from having any blocks
* Update memory model support for SPIR-V 1.4
Fixes#2552
* Upgrade memory model now supports two memory access operands for
OpCopyMemory*
* in all cases the pass will first generate two operands by either
adding them or copying
* updates accounts for multiple operands
* tests
There is a case where sroa is not handling id overflow gracefully. It
is handled and an error message is output when the ids overflow.
Fixes https://crbug.com/961030.
* Make pointers to logically matching types interchangeable with option.
DXC will be generating code where the function parameters will be a more
generic type that the actual parameter. They should be logically
matching and the decorations of the actual parameter must be a superset
of the decorations of the formal parameter.
We want to accept this code with an options so that spirv-opt can then
inline and fix the type mismatch. We will accept this under a new
options `--before-hlsl-legalization`.
The new option will also imply `relax-logical-pointer` so that HLSL
frontends will need to use just the one more generic option.
Moved the |LogicallyMatches| to the validation state to make it
available in more places. Also added a parameter to have it check the
decorations. I did not do a separate function for the decorations
because checking the decorations involves making sure the types
logically match anyway.
Fixes#2535
* Use grammar last version
Fixes#2560
* Parse last version and use it in checks
* Update grammar header generation
* Fix NonWritable tests
* Fix check and add specific tests
Fixes#2555
* Fix a bug in validation where interfaces were considered non-unique
between different entry points targeting the same function
* added a test
* Update private to local pass to remove localized private variables
from entry point interfaces
* added tests
Fixes#2551
* Add support for 1.4 entry point interface lists
* only input and output variables are automatically live
* can clean up interfaces after DCE
* added tests
* allow opt tests to specify a target environment
* SPIR-V 1.4 headers, add SPV_ENV_UNIVERSAL_1_4
* Support --target-env spv1.4 in help for command line tools
* Support asm/dis of UniformId decoration
* Validate UniformId decoration
* Fix version check on instructions and operands
Also register decorations used with OpDecorateId
* Extension lists can differ between enums that match
Example: SubgroupMaskEq vs SubgroupMaskEqKHR
* Validate scope value for Uniform decoration, for SPIR-V 1.4
* More unioning of exts
* Preserve grammar order within an enum value
* 1.4: Validate OpSelect over composites
* Tools default to 1.4
* Add asm/dis test for OpCopyLogical
* 1.4: asm/dis tests for PtrEqual, PtrNotEqual, PtrDiff
* Basic asm/Dis test for OpCopyMemory
* Test asm/dis OpCopyMemory with 2-memory access
Add asm/dis tests for OpCopyMemorySized
Requires grammar update to add second optional memory access operand
to OpCopyMemory and OpCopyMemorySized
* Validate one or two memory accesses on OpCopyMemory*
* Check av/vis on CopyMemory source and target memory access
This is a proposed rule. See
https://gitlab.khronos.org/spirv/SPIR-V/issues/413
* Validate operation for OpSpecConstantOp
* Validate NonWritable decoration
Also permit NonWritable on members of UBO and SSBO.
* SPIR-V 1.4: NonWrtiable can decorate Function and Private vars
* Update optimizer CLI tests for SPIR-V 1.4
* Testing tools: Give expected SPIR-V version in message
* SPIR-V 1.4 validation for entry point interfaces
* Allow only unique interfaces
* Allow all global variables
* Check that all statically used global variables are listed
* new tests
* Add validation fixture CompileFailure
* Add 1.4 validation for pointer comparisons
* New tests
* Validate with image operands SignExtend, ZeroExtend
Since we don't actually know the image texel format, we can't fully
validate. We need more context.
But we can make sure we allow the new image operands in known-good
cases.
* Validate OpCopyLogical
* Recursively checks subtypes
* new tests
* Add SPIR-V 1.4 tests for NoSignedWrap, NoUnsignedWrap
* Allow scalar conditions in 1.4 with OpSelect
* Allows scalar conditions with vector operands
* new tests
* Validate uniform id scope as an execution scope
* Validate the values of memory and execution scopes are valid scope
values
* new test
* Remove SPIR-V 1.4 Vulkan 1.0 environment
* SPIR-V 1.4 requires Vulkan 1.1
* FIX: include string for spvLog
* FIX: validate nonwritable
* FIX: test case suite for member decorate string
* FIX: test case for hlsl functionality1
* Validation test fixture: ease debugging
* Use binary version for SPIR-V 1.4 specific features
* Switch checks based on the SPIR-V version from the target environment
to instead use the version from the binary
* Moved header parsing into the ValidationState_t constructor (where
version based features are set)
* Added new versions of tests that assemble a 1.3 binary and validate a
1.4 environment
* Fix test for update to SPIR-V 1.4 headers
* Fix formatting
* Ext inst lookup: Add Vulkan 1.1 env with SPIR-V 1.4
* Update spirv-val help
* Operand version checks should use module version
Use the module version instead of the target environment version.
* Fix comment about two-access form of OpCopyMemory
Add functionality to fix-storage-class so that it can fix up mismatched
data types for pointers as well.
Fixes bugs in when fixing up storage class.
Move GenerateCopy to the Pass class to be reused.
The spirv-opt change for #2535.
* Change implementation of post order CFG traversal
It seems like the recursion is going very deep, and causing some problem
is particular situations. I've reimplemented the CFG post order
traversal to not use recursion.
Fixes#2539.
Recent change to the spec restricted the valid values for Memory
Semantics in OpAtomics* in the WebGPU env. Implementing enforcing
these changes.
Fixes#2499
WebGPU requires certain variables to be initialized, whereas there are
known issues with using initializers in Vulkan. This PR is the first
of three implementing a pass to decompose initialized variables into
a variable declaration followed by a store. This has been broken up
into multiple PRs, because there 3 distinct cases that need to be
handled, which require separate implementations.
This first PR implements the basic infrastructure that is needed, and
handling of Function storage class variables. Private and Output will
be handled in future PRs.
This is part of resolving #2388
* Fix#2320. `conditional_branch_to_simple_conditional_branch` reduction pass changes conditional branches so both targets point to the same block id (creating a "simple" conditional branch).
* Fix#2501. `simple_conditional_branch_to_branch` reduction pass changes "simple" conditional branches to branches.
* Fix#2503. `conditional_branch_to_simple_conditional_branch` proper handling of back-edges.
In WebGPU, the component operand 0xFFFFFFFF is forbidden, but in
Vulkan it is used to indicate a value is undefined. When converting to
WebGPU, 0xFFFFFFFF needs to converted to a legal value, though the
specific one does not matter, since it was used to indicate an
undefined entry in the original code. Choosing to use 0, since the
operands are required to be on [0, N-1], so 0 is guaranteed to always
be valid.
Fixes#2349
Fixes#2470
* Only require the *WithoutFormat capabilities for Unknown image reads
and writes in the Vulkan environment
* update tests and add new vulkan specific tests
Renames the existing flag '--webgpu-mode' to '--vulkan-to-webgpu' for
the Vulkan->WebGPU operation, and adds a new flag '--webgpu-to-vulkan'
for the WebGPU->Vulkan operation.
Currently '--webgpu-to-vulkan' doesn't have any passes associated with
it yet, but further patches will implement them.
Fixes#2495
This pass tries to fix validation error due to a mismatch of storage classes
in instructions. There is no guarantee that all such error will be fixed,
and it is possible that in fixing these errors, it could lead to other
errors.
Fixes#2430.
Fixes#2452
Swaps priority of handling unreachable merge and continues so that the
back-edge is retained in the case a block is both a loop continue and
loop merge
* Check var pointer capability in ADCE.
* Check var ptr capability for common uniform.
* Check var ptr capability in access chain convert.
Since we want this pass to run even if there are variable pointer on
storage buffers, we had to remove asserts that assumed there were no
variable pointers. The functions with the asserts will now work, it
becomes the responsibility of the callers to deal with the output as
appropriate.
* Single block elimination and variable pointers.
It seems like the code in local single block elimination is able to
handle cases with variable pointers already. This is because the
function `HasOnlySupportedRefs` ensures that variables that feed a
variable pointer are not candidates.
* Single store elimination and variable pointers.
It seems like the code in local single stroe elimination is able to
handle cases with variable pointers already. This is because the
function `FindSingleStoreAndCheckUses` ensures that variables that feed
a variable pointer are not candidates.
* SSA rewriter and variable pointers.
It seems like the code in the two passes that call the SSA rewriter are
able to handle cases with variable pointers already. This is because the
function `HasOnlySupportedRefs` ensures that variables that feed
a variable pointer are not candidates.
Fixes#2458.
Fixes#2456
* When eliminating a structured construct that has an unreachable merge,
replace that unreachable terminator with an appropriate return
* New tests
Fixes#2488
* Validator doesn't identify back-edge of the loop, so the merge is
never set
* Construct::blocks() has safe uses of `merge` so the assert can be
removed
* Added a test
Fixes#2453
* Enable addition of OpPhi instructions when the loop has multiple
predecessors of the merge due to a break
* This can result in some values no longer dominating their uses
* Track return blocks in structured flow to produce OpPhis that have
multiple undef and non-undef arguments
* New tests to catch the bug
* When a block is predicated, mark the new body as a return if the old
block as already a return
* Fix#2478. The fix is to just not try to simplify such loops.
* Also added `BasicBlock::MergeBlockId()` and `BasicBlock::ContinueBlockId()`.
* Some minor changes to `structured_loop_to_selection_reduction_opportunity.cpp`.
* Added test.
Fix#2475. Fix#2476.
* Improve reducer algorithm: shrink granularity, remove an early return, no lazy initialization, notify pass if binary is interesting, add comments.
* Add fail-on-validation-error option to fail a reduction if an invalid state is reached; useful for tests.
* Set fail-on-validation-error in tests.
* Improve some documentation comments.
* Add Reducer::AddDefaultReductionPasses so tests (and other library consumers) can add the default reduction passes.
* Add CLIMessageConsumer in test_reduce so we can see messages for tricky tests.
* Remove test RemoveUnreferencedInstructionReductionPassTest_ApplyReduction because it was indirectly testing the reduction algorithm, not the RemoveUnreferencedInstruction pass.
* Tweak tests where needed.
Fix#2396
* Check that initial state is valid. Add kInitialStateInvalid.
* Fix RemoveOpnameAndRemoveUnreferenced test; turns out the original shader is invalid, but we never notice because we don't check this and the reduced shader is valid; fix original shader. Assert reduction status is kComplete.
* Always check return value from `Reducer::Run`.
* Change Reducer::Run to *not* immediately copy the input binary.
Changing the stored value for a sampled image consumer to be the
instruction instead of result ID, since not all instructions have
result IDs. Using result IDs led to a potential crash when using
OpReturnValue, which doesn't have a result ID. OpReturnValue is not a
legal consumer, but the validator needs to look at the instruction to
determine this, thus storing the pointer to the instruction, instead
of trying to fetch the pointer using the instruction.
Issue #1528 covers fixing the check.
Fixes#2463
If SPV_EXT_descriptor_indexing is enabled, add check that for a
descriptor-based reference, the descriptor is initialized. Initialization
data is stored in the debug input buffer, added to the length information
already there. This feature must be seperately enabled on the pass
creation routine. NOTE: Currently just supports image references; buffer
references are still TODO.
Adds an optimization pass to remove usages of AtomicCounterMemory
bit. This bit is ignored in Vulkan environments and outright forbidden
in WebGPU ones.
Fixes#2242
If relax-logical-pointer is enabled, this commit makes Validator
accept function param even when its Storage Class is different from
the expected one.
Related to #2423, #2430
In constant propagation, decoration are transfered from the original
expression to the constant that will replace it. This can be wrong
because there are no decorations that apply to constants. We choose to
simply delete the decorations.
Fixes#2441
In WebGPU all blocks are required to be reachable, unless they are one of two
specific degenerate cases for merge-block or continue-target. This PR adds in
checking for these conditions.
Fixes#2068
Updated script to work with python3 and python2.
Added required tools.
We added a section to the readme to mention the tools that are needed to
build and test spirv-tools. For the compiler, the compilers used by the
bots are mentioned.
The bots have been changed. The windows bots will not use python 3.6 for testing. The other bots will still use python 2.7. Both Python2 and Python3 will be tested.
Fixes#2407.
Fixes#1856.
* Handle back edges better in dead branch elim.
Loop header must have exactly one back edge. Sometimes the branch
with the back edge can be folded. However, it should not be folded
if it removes the back edge.
The code to check this simply avoids folding the branch in the
continue block. That needs to be changed to not fold the back edge,
wherever it is.
At the same time, the branch can be folded if it folds to a branch to
the header, because the back edge will still exist.
Fixes#2391.
In relaxed addressing mode, we want to accept non memory objects
because this is a very natural translation of hlsl. It should be fixed
by legalization by inlining the calls.
* Fix OpDot folding of half float vectors.
The code that folds OpDot does not handle half floats correctly. After
trying to multiple the first components, we get a nullptr because we
don't fold half float values. This nullptr gets passed to the code that
does the addition, and causes an assert.
Fixes#2405.
The types of input and output variables must match for the pipeline. We
cannot see the uses in all of the shader, so dead member
elimination cannot safely change the type of input and output variables.
Add a pass that looks for members of structs whose values do not affects
the output of the shader. Those members are then removed and just
treated like padding in the struct.
* Fixes#2358. Added to the reducer the ability to remove a function that is not directly called. Factored out some code from the optimizer to help with this.
Fixes#2120
Enhanced the reducer so that it can merge blocks together, leveraging the functionality extracted from the block_merge pass in the optimizer.
* Fixes#2338. Added check for phi node before merging blocks.
* Added functionality to merge blocks A and B even when B starts with OpPhi instructions, by replacing uses of the OpPhi results with the definitions coming from A. Added some tests for this.
* Fixed assertion.
* Remove use of deprecated googletest macro
INSTANTIATE_TEST_CASE_P has been deprecated. We need to use
INSTANTIATE_TEST_SUITE_P instead.
* Remove extra commas from test suites.
This CL adds in the specific checks required for WebGPU, enables
running the builtin checks for WebGPU, and refactors the existing
testing infrastructure to support testing the new checks.
This PR is part of resolving #2276
In a recent PR, we allowed a forward reference for the element type in
an array declaration. However, we do not have other check to make sure
the forward reference is a pointer type first reference in
OpTypeForawrdPointer. We add that check.
Fixes https://crbug.com/920074.
When looking at the uses of the result of an instruction, code sinking
assumes that all uses are in a basic block. However, this is not true
if there is a decoration or name for the result of that insturction.
This commit checks for this.
Fixes https://crbug.com/923243.
It is legal, but not generated by any SPIR-V producer: an OpCompositeExtract
with no indexes. This is essentially just a copy of the object, so we
treat them that way. We simply propagate the live variables of the
result to the operand.
Fixes https://crbug.com/919181.
It is legal, but not generated by any SPIR-V producer: an OpAccessChain
with no indexes. This is essentially just a copy of the pointer.
I have decided to treat it like an OpCopyObject. In CheckUses, we
return that it is not okay.
When looking at this I realized that we had code in GetUsedComponents
that cannot be reached. If there is a use in an OpCopyObject the it
will not call GetUsedComponents. I removed that dead code.
Fixes https://crbug.com/918311.
During unrolling a new loop is created, but its ownership is not clear
as it gets passed through the code. Changed something to unique_ptr to
make that clearer.
Fixes#2299.
Fixing other memory leaks at the same time.
Fixes#2296Fixes#2297
In C++, a bit shift of the same size as the type is undefined, but it is
defined in spir-v. When folding those cases, we have to be careful. We
cannot simply do the shift in C++.
Fixes https://crbug.com/917697.
Fixes#2138
* Modf and frexp are upgraded to use the struct version of the
instruction and generate an explicit store whose flags can be upgraded
separately
* Fixed major bug where availability and visibility were reversed for
non-copy memory instructions
* Fixed bug where availability and visibility scope operands were reversed for copy memory
* Upgraded all opt tests to use SPV_ENV_UNIVERSAL_1_3
* Upgrade tests moved into unified tests and removed standalone test
Broader check for ids that require a type
Fixes https://crbug.com/911700
* Adds a broader check for when id operands require a type
* updated a few tests
* added a test to catch the original issue
* Handle CompositeInsert with no indices in VDCE
In the spec, there it nothing that forces an OpCompositeInsert to have
an index, but VDCE assumes there is at least 1 in a couple places.
This commit updates VDCE to handle these cases.
* Added additional changes for the new AccelerationStructureNV type.
* Added NVIDIA ray tracing storage classes for checking in ValidateVariable.
* For NVIDIA ray tracing storage classes added test to load bool type (allowed) in new storage class.
This Cl changes the binary parser to keep track of the instruction count
being processed. The parser will then use that instruction number as the
error number, instead of the binary word.
This should make it easier to match the error up to what the
disassembler would output for the error.
Issue #2091
When processing options in a file, it does have access to the
ValidatorOptions and OptimizerOptions object, so options that change
those do not work. We just need to pass it in.
Fixes#2219.
* Added additional changes for the new AccelerationStructureNV type.
* Added additional changes for the new AccelerationStructureNV type. Change tabs to space...
* Added additional changes for the new accelerationStructureNV type -- add proper type name.
Fix TypeManager.TypeStrings test:
[----------] 29 tests from TypeManager
[ RUN ] TypeManager.TypeStrings
[ OK ] TypeManager.TypeStrings (7 ms)
When we are predicating the continue target for a loop, it can no longer
be the continue target because it will have a branch that exits the loop
and is not the bach edge. The continue target will have to be the
target of that branch that is still in the loop.
Fixes#2211.
The function `UpdatePhiNodes` was being called inconsistently. In one
case, the cfg had already been updated to include the new edge, and in
another place the cfg was not updated. This caused the function to
miss flagging a block as needing new phi nodes. I picked that the cfg
should not be updated before making the call. I documented it, and
change the call sites to match.
Fixes#2207.
We initially assumed that if the type manager returned the correct id
for the pointee type, that we would get the correct pointer type back,
but that is not true. See the unit test added with this commit. We
need to fall back to the linear search any time we are looking for a
pointer to a type that may not be unique.
At the same time, SROA considered an OpName on a variable to be a use of
the entire variable. That has been fixed.
Fixes#2209.
We currently place the load instructions at the start of the basic block
that dominates all of the loads. If that basic block contains OpPhi
instructions, then this will generate invalid code. We just need to
search for a location that comes after all of the OpPhi instructions.
Fixes#2204.
* Add OperandToUndefReductionPass.
Fixes#2115.
Also added some tests that are similar to those in OperandToConstantReductionPassTest.
In addition, refactor FindOrCreateGlobalUndef into reduction_util.cpp. Fixes#2184.
Removed many documentation comments that were identical or very similar to the overridden function's documentation comment.
* Don't fold specialized branchs in loop unswitch
Folding branches can have a lot of special cases, and can be a little
error prone. So I only want it in one place. That will be in dead
branch elimination. I will change loop unswitching to set the branches
that were being folded to have a constant condition. Then subsequent
pass of dead branch elimination will be able to remove the code.
At the same time, I added a check that loop unswitching will not
unswitch a branch with a constant condition. It is not useful to do it
because dead branch elimination will simple fold the branch anyway.
Also it avoid an infinite loop that would other wise be introduced by my
first change.
Fixes#2203.
Loop unswitching is unswitching the conditional branch that creates the
back-edge. In the version of the loop, where the bachedge is not taken,
there is no back-edge. This is what causes the validator to complain.
The solution I will go with will be to now unswitch a condition with a
back-edge. At this time we do not now if loop unswitching is used. We do
not include it in the optimization sets provided, nor is it used in
glslang's set. When there are opportunities and no breaks from the loop,
the loop with either be a single iteration loop, or an infinite loop.
There is no performance advantage to performing loop unswitching in
either of those cases. If there is a break, maintaining structured
control flow will be tricky. Unless we see a clear advantage to handling
these case, I would go with the safer simpler solution.
Fixes#2201.
If there are multiple edges to a basic block, then the ssa rewriter will
create OpPhi instructions with duplicate entries. This is invalid, and
it is fixed in this commit.
Fixes#2202.
* Run validator during reduction.
* Added functionality to validate modules after each reduction step, and some tests to check this is working. Also fixed an issue where reduction passes were not guaranteed to be executed at their minimum granularities.
There is inconsistencies between the different specs about whether or
not this capability is required/allowed, so tooling like glslang
currently ignores it. Once this is resolved the check and test can be
re-enabled.
* Invalidate the decoration manager at the start of ADCE.
If the decoration manager is kept live the the contex will try to keep
it up to date. ADCE deals with group decorations by changing the
operands in |OpGroupDecorate| instructions directly without informing
the decoration manager. This puts it in an invalid state, which will
cause an error when the context tries to update it. To Avoid this
problem, we will invalidate the decoration manager upfront.
At the same time, the decoration manager is now considered when checking
the consistency of the decoration manager.
Add a spirv-reduce pass which removes OpName and OpMemberName instructions.
This is useful to enable other reduction passes, e.g. RemoveUnreferencedInstruction may not be able to remove an instruction creating an id whose only usage is an OpName for this id.
* Fix invalid OpPhi generated by merge-return.
When we create a new phi node for a value say %10, we have to replace
all of the uses of %10 that are no longer dominated by the def of %10
by the result id of the new phi. However, if the use is in a phi node,
it is possible that the bb contains the use is not dominated by either.
In this case, needs to be handled differently.
* Split loop headers before add a new branch to them.
In merge return, Phi node in loop header that are also merges for loop
do not get updated correctly. Those cases do not fit in with our
current analysis. Doing this will simplify the code by reducing the
number of cases that have to be handled.
Added documentation to the ir context to indicates that TakeNextId()
returns 0 when the max id is reached. TODOs were added to each call
sight so that we know where we have to start to handle this case.
Handle id overflow in |SplitLoopHeader|.
Handle id overflow in |GetOrCreatePreHeaderBlock|.
Handle failure to create preheader in LICM.
Part of https://github.com/KhronosGroup/SPIRV-Tools/issues/1841.
* Only check for binding and descriptor set on variables that are
statically used by an entry point
* updated tests and added a couple new ones
* new method for collecting entry points that statically reference an
id
* Validate OpForwardPointer
The validator does not have a a check that OpForwardPointer is giving
a forward reference to a pointer type. We add that check.
https://crbug.com/910852
* Remove more specialized check.
There was a check that the forward pointer is actually a poiner type,
but it was only done if it was used in a struct. This was too specific.
Remove it in favour of the more general check that was added.
* Format
* Check the storage type in OpTypeForwardPointer
* Fix typo is test case epxected results.
We currently simulate all shift operations when the two operand are
constants. The problem is that if the shift amount is larger than
32, the result is undefined.
I'm changing the folder to return 0 if the shift value is too high.
That way, we will have defined behaviour.
https://crbug.com/910937.
Fixes#2147
* Checks that device scope is not used for availability and visibility
operations unless VulkanMemoryModelDeviceScopeKHR capability is present
* implemented for atomics, barriers and memory instructions currently
This CL changes the id/name output from the validator to always use a
consistent id[%name] style. This removes the need for getIdOrName. The
name lookup is changed to use the NameMapper so the output is consistent
with what the disassembler will produce.
Fixes#2137
* Validate uses of ids defined in unreachable blocks.
For some reason we do not make sure the uses of ids that are defined
in unreachable blocks are dominated by their def. This is causing
invalid code to pass the validator.
Fixes#2143
* Add test for unreachable code after a return.
We want to allow code like:
```
void foo() {
a = ...;
...
return; // for debugging
<use of a>;
...
}
```
I added a test to make sure that something like this is still accepted
by the validator.
* Add test for unreachable def used in phi.
Upgrade to VulkanKHR memory model
* Converts Logical GLSL450 memory model to Logical VulkanKHR
* Adds extension and capability
* Removes deprecated decorations and replaces them with appropriate
flags on downstream instructions
* Support for Workgroup upgrades
* Support for copy memory
* Adding support for image functions
* Adding barrier upgrades and tests
* Use QueueFamilyKHR scope instead of device
* Move ProcessFunction* function from pass to the context.
There are a few functions that are used to traverse the call tree.
They currently live in the Pass class, but they have nothing to do with
a pass, and may be needed outside of a pass. They would be better in
the ir context, or in a specific call tree class if we ever have a need
for it.
* Don't inline recursive functions.
Inlining does not check if a function is recursive or not. This has
been fine as long as the shader was a Vulkan shader, which forbid
recursive functions. However, not all shaders are vulkan, so either
we limit inlining to Vulkan shaders or we teach it to look for recursive
functions.
I prefer to keep the passes as general as is reasonable. The change
does not require much new code in inlining and gives a reason to refactor
some other code.
The changes are to add a member function to the Function class that
checks if that function is recursive or not.
Then this is used in inlining to not inlining a function call if it calls
a recursive function.
* Add id to function analysis
There are a few places that build a map from ids to Function whose
result is that id. I decided to add an analysis to the context for this
to reduce that code, and simplify some of the functions.
* Add missing file.
Fixes#2104
* Checks the rules for logical addressing and variable pointers
* Has an out for relaxed logical pointers
* Updated PassFixture to expose validator options
* enabled relaxed logical pointers for some tests
* New validator tests
Restrict capabilities to WebGPU spec
This covers whitelisting Matrix, Shader, Sampled1D, Image1D,
DerivativeControl, and ImageQuery. These are the allowed capabilities
that don't require an extension. Whitelisting VulkanMemoryModelKHR
will be handled by whitelisting its extension in a seperate patch.
Fixes#2101
* Added a reduction pass to replace ids with ids of the same type that dominate them.
* Introduce helper method for querying whether an operand type is an input id.
Make sure that initialized variable have correct storage class
For WebGPU and Vulkan environments, variables must have the storage
class; Output, Private, or Function, if they have an initializer.
Fixes#2071
Adding validation that the addressing declared by OpMemoryModel is
Logical and the memory model declared is VulkanKHR. Updating a bunch
of tests that were broken by this.
Fixes#2060
Check forbidden Annotation instructions for WebGPU env
From the WebGPU SPIR-V Execution Enviroment spec:
OpDecorationGroup, OpGroupDecorate, OpGroupMemberDecorate are not
allowed.
Fixes#2062
Validate that debugging instructions are not present for WebGPU
For WebGPU execution environments, check that all of the debug
instructions have already been stripped before validation.
Fixes#2063
We had a test that checks that a spirv binary where the depth of the
deepest nested construct is just below the default limit. This test
would take a long time causing a timeout in some builds. We are
questioning the value of the test since we already have a test that can
tell us if we are within a custom limit. We will remove the test.
Add tests for matrix type data rule validation
This covers the data rules for matrix types specified in the SPIR-V
spec, section 2.16.1, and the WebGPU SPIR-V Execution Environment
spec.
Fixes#2065Fixes#2080
Ban sequentially consistent with VulkanKHR
* Added validation check that SequentiallyConsistent memory semantics
are not used if the memory model is VulkanKHR
* Added tests
* Fixed a bug in evaluating constant 32-bit integers and updated some
handling to avoid inferring a value from a spec constant default
Remaining memory semantics validation
* Adds checks that OutputMemoryKHR, MakeAvailableKHR and MakeVisibleKHR
are only used if the VulkanMemoryModelKHR capabailty is present
* Added checks that MakeAvailableKHR requires release semantics
* Added checks that MakeVisibleKHR requires acquire semantics
* Added checks that MakeAvailableKHR and MakeVisibleKHR require a
storage class
Adds validator option to specify scalar block layout rules.
Both VK_KHR_relax_block_layout and VK_EXT_scalar_block_layout can be
enabled at the same time. But scalar block layout is as permissive
as relax block layout.
Also, scalar block layout does not require padding at the end of a
struct.
Add test for scalar layout testing ArrayStride 12 on array of vec3s
Cleanup: The internal getSize method does not need a round-up argument,
so remove it.
These are bookend passes designed to help preserve line information
across passes which delete, move and clone instructions. The propagation
pass attaches a debug line instruction to every instruction based on
SPIR-V line propagation rules. It should be performed before optimization.
The redundant line elimination pass eliminates all line instructions
which match the previous line instruction. This pass should be performed
at the end of optimization to reduce physical SPIR-V file size.
Fixes#2027.
From the Vulkan 1.1 spec 14.5.2:
Variables identified with the Uniform storage class are used to access
transparent buffer backed resources. Such variables must be typed as
OpTypeStruct, or an array of this type.
Fixes#1949
Validate variable types for UniformConstant storage in Vulkan (#2008)
From the Vulkan 1.1 spec 14.5.2:
Variables identified with the UniformConstant storage class are used
only as handles to refer to opaque resources. Such variables must be
typed as OpTypeImage, OpTypeSampler, OpTypeSampledImage, or an array
of one of these types.
Fixes#2008
That function currently only handled OpPtrAccessChain if it was in the
middle of the chain, but not at the start. Fixing that up.
Fixes crbug.com/905271.
The Vulkan specification does not permit use of the VertexId and
InstanceId BuiltIn decorations, so add a check to ensure they are not
being used when the target environment is Vulkan.
* Add base and core bindless validation instrumentation classes
* Fix formatting.
* Few more formatting fixes
* Fix build failure
* More build fixes
* Need to call non-const functions in order.
Specifically, these are functions which call TakeNextId(). These need to
be called in a specific order to guarantee that tests which do exact
compares will work across all platforms. c++ pretty much does not
guarantee order of evaluation of operands, so any such functions need to
be called separately in individual statements to guarantee order.
* More ordering.
* And more ordering.
* And more formatting.
* Attempt to fix NDK build
* Another attempt to address NDK build problem.
* One more attempt at NDK build failure
* Add instrument.hpp to BUILD.gn
* Some name improvement in instrument.hpp
* Change all types in instrument.hpp to int.
* Improve documentation in instrument.hpp
* Format fixes
* Comment clean up in instrument.hpp
* imageInst -> image_inst
* Fix GetLabel() issue.
If there is only 1 return and it is in a loop, then the function cannot be inlined.
Fix condition when inlined code needs one-trip loop wrapper. The dummy loop is needed when there is a return inside a selection construct. Even if there is only 1 return.
* Validate the id bound.
Validates that the id bound for the module is not larger than the max id
bound. Also adds an option to set the max id bound. Allows the
optimizer option to set the max id bound to also set the id bound for
the validation run done by the optimizer.
Fixes#2030.
This CL takes the various opt unit tests and makes a single executable
instead of one per test. This reduces the number of build targets by
~125 when building with ninja.
When looking for a break from a selection construct, we do not realize
that a jump to the continue target of a loop containing the selection
is a break. This causes and infinit loop, or possibly other failures.
Fixes#2004.
When looking for a break from a selection construct, we do not need to
look inside nested constructs. However, if a loop header has an
unconditional branch, then we enter the loop. Entering the loop causes
an infinite loop because we keep going through the loop.
The solution is to look for a merge block, if one exsits, even for block
terminated by an OpBranch.
Fixes#1979.
The SPV_KHR_8bit_storage extension does not permit 8-bit integers to be
cast directly to floating point types. We are seeing shaders in the
wild, being produced by toolchains like glslang, that are generating
invalid SPIR-V.
This change adds validation to check for the patterns not permitted, and
some tests that expose the failure.
ADCE liveness algorithm should treat OpUnreachable at least like other
branch instructions. It was being treated as always live which was
preventing useless structured constructs from being eliminated.
OpUnreachable is generated by dead branch elimination which is now
being required by merge return, so this fix should accompany that
change.
We currently run merge-return on all functions, but
dead-branch-elimination only runs on function reachable from an entry
point or exported function. Since dead-branch-elimination is needed for
merge-return, they have to match.
Fixes#1976.
In logical addressing mode, we are not allowed to generate variables
pointers. There is already a check for OpSelect. However, OpPhi
and OpPtrAccessChain are not checked to make sure it does not
generate an variable pointer. I've added those checks.
Fixes#1957.
Was removing control structures which didn't have data dependency
with enclosed live loop and otherwise did not contain live code.
An example is a counting loop around a live loop.
Fixes#1967.
* MakePointerVisibleKHR cannot be used with OpStore
* MakePointerAvailableKHR cannot be used with OpLoad
* MakePointerAvailableKHR and MakePointerVisibleKHR both require
NonPrivatePointerKHR
* NonPrivatePointerKHR is limited to a subset of storage classes
* many tests
* Validation checks for new image operands MakeTexelAvailableKHR and
MakeTexelVisibleKHR
* added tests
* Tests that NonPrivateTexelKHR is accepted for all image operands
Updating test environments
* fixed build errors
* changed image types for *FetchSuccess tests to use a type defined in
1.3 shader body
Merge return assumes that the only unreachable blocks are those needed
to keep the structured cfg valid. Even those must be essentially empty
blocks.
If this is not the case, we get unpredictable behaviour. This commit
add a check in merge return, and emits an error if it is not the case.
Added a pass of dead branch elimination before merge return in both the
performance and size passes. It is a precondition of merge return.
Fixes#1962.
The current implementation in the folder when seeing a division by zero
is to assert. In the release build, the compiler will attempt to
compute the value, which causes its own problems.
The solution I will go with is to fold the division, and just give it
the value of 0. The same goes for remainder and mod operations.
Fixes#1961.
This commit checks the following when Shader capability exists:
"The FPRoundingMode decoration can be applied only to a width-only
conversion instruction that is used as the Object operand of an
OpStore storing through a pointer to a 16-bit floating-point object
in the StorageBuffer, Uniform, PushConstant, Input, or Output
Storage Classes.".
The HlslCounterBufferGOOGLE that was introduced changed the OpDecorateId
so that is can now reference an id other than the target. If that other
id is used only in the decoration, then the definition of the id will be
removed because decoration do not count as real uses.
However, if the target of the decoration is still live the decoration
will not be removed. This leaves a reference to an id that is not
defined.
There are two solutions to consider. The first is that is the decoration
is kept, then the definition of the id should be kept live. Implementing
this change would be involved because the way ADCE handles decorations
will have to be reimplemented.
The other solution is to remove the decoration the id is otherwise dead.
This works for this specific case. Also this is the more desirable
behaviour in this case. The id will always be the id of a variable that
belongs to a descriptor set. If that variable is not bound and we do
not remove it, the driver will complain.
I chose to implement the second solution. The first will be left to when
a case for it comes up.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1885.
This commit will change the message for unknown extensions from an error
to a warning.
Code was added to limit the number of warning messages so that consummer
of the messages are not overwhelmed. This is standard practice in
compilers.
Many other issues were found at while looking into this. They have been
documented in #1950.
Fixes http://crbug.com/875547.
* Check rules from Execution Mode tables, 2.16.2 and the Vulkan
environment spec
* Allows MeshNV execution model with the following execution modes
* LocalSize, LocalSizeId, OutputPoints and OutputVertices
* Done to not break their validation
There are a few spots where copy propagate arrays is trying
to go from a Type to an id, but the type is not unique. When generating
code this pass needs specific ids, otherwise we get type mismatches.
However, the ambigous types means we can sometimes get the wrong type
and generate invalid code.
That code has been rewritten to not rely on the type manager, and just
look at the instructions instead.
I have opened https://github.com/KhronosGroup/SPIRV-Tools/issues/1939 to
try to get a way to make this more robust.
In DecorationManager::RemoveDecorationsFrom, we do not remove the id
from a decoration group if the group has no decorations. This causes
problems because KillNamesAndDecorates is suppose to remove all
references to the id, but in this case, there is still a reference.
This is fixed by adding a special case.
Also, there is the possibility of a double free because
RemoveDecorationsFrom will delete the instructions defining |id| when
|id| is a decoration group. Later, KillInst would later write to memory
that has been deleted when trying to turn it into a Nop. To fix this,
we will only remove the decorations that use |id| and not its definition
in RemoveDecorationsFrom.
OpPhi instruction must appear before all non-OpPhi instructions
except for OpLine. Without this commit, Validator does not check
the case that an OpPhi is preceeded by an OpLine and the OpLine is
preceeded by a non-OpPhi instruction that is not OpLine.
Checked all instructions whose object is OpTypeSampledImage or
OpTypeImage as suggested in #487. OpImageTexelPointer instruction
is missing and others look good. This commit adds only
OpImageTexelPointer.
This CL removes the use of SetContextMessageConsumer from the
binary_parse_test tests and creates a Context object and uses
SetMessageConsumer instead.
Instead of using the source/table.h methods, this CL switches the stats
tool to use the spvtools::Context class and assign the message consumer
through the public API.
A limit of 0 for the scalar replacement options it used to indicate that
there is no limit. The current implementation does not allow 0. This
should be fixed.
It seems like the current implementation of KillNameAndDecorates does
not handle group decorations correctly. The id being removed is not
removed from the OpGroupDecorate instructions. Even worst, any
decorations that apply to that group are removed.
The solution is to use the function in the decoration manager that will
remove the decorations and update the instructions instead of doing the
work itself.
Adds unrolling to the legalization passes.
After enabling unrolling I found a bug when there is a self-referencing
phi node. That has been fixed.
The test that checks for that the order of optimizations is correct also
needed to be updated.
The current implementation of merge return can create bad, but correct,
code. When it is not in a loop construct, it will insert a lot of
extra branch around code. The potentially large number of branches are
bad. At the same time, it can separate code store to variables from
its uses hiding the fact that the store dominates the load.
This hurts the later analysis because the compiler thinks that multiple
values can reach a load, when there is really only 1. This poorer
analysis leads to missed optimizations.
The solution is to create a dummy loop around the entire body of the
function, then we can break from that loop with a single branch. Also
only new merge nodes would be those at the end of loops meaning that
most analysies will not be hurt.
Remove dead code for cases that are no longer possible.
It seems like some drivers expect there the be an OpSelectionMerge
before conditional branches, even if they are not strictly needed.
So we add them.
* Create structed cfg analysis.
There are lots of optimization that have to traverse the CFG in a
structured order just because it wants to know which constructs a
basic block in contained in. This adds extra complexity to these
optimizations, for causes too much refactoring of older optimizations.
To help with this problem, I have written an analysis that can give this
information.
* Identify branches breaking from loops.
Dead branch elimination does a search for a conditional branch to the
end of the current selection construct. This search assumes that the
only way to leave the construct is through the merge node. But that is
not true. The code can jump to the merge node of a loop that contains
the construct.
The search needs to take this into consideration.
In merge blocks, we do not allow the merging of two blocks with merge
instructions. This is because if the two block are merged only 1 of
those instructions can exists. However, if the successor block is the
merge block of the predecessor, then we can delete the merge instruction
in the predecessor. In this case, we are able to merge the blocks.
* Create a new entry point for the optimizer
Creates a new struct to hold the options for the optimizer, and creates
an entry point that take the optimizer options as a parameter.
The old entry point that takes validator options are now deprecated.
The validator options will be one of the optimizer options.
Part of the optimizer options will also be the upper bound on the id bound.
* Add a command line option to set the max value for the id bound. The default is 0x3FFFFF.
* Modify `TakeNextIdBound` to return 0 when the limit is reached.
Support collapsed into one commit:
- Asm/Dis support for SPV_KHR_vulkan_memory_model
- Add Vulkan mem model image operands to switch
- Add TODO for source/validate_image.cpp
- val: Image operands NonPrivateTexelKHR, VolatileTexelKHR have no operands
This is required for memory model tests to pass SPIR-V validation.
- Round trip tests: Test new flags on OpCopyMemory*
This splits the spvtools_config into a public and private part to avoid
leaking internal bits to dependents. A new target is added for the
public headers so that "gn check" works for dependents.
Also formats test/fuzzers/BUILD.gn
* Validate all type ids.
The validator does not check if the type of an instruction is actually
a type unless the OpCode has a specific requirement. For example,
OpFAdd is checked, but OpUndef is not.
The commit add a generic check that if there is a type id then the id
defines a type.
http://crbug.com/876694
* Merge other checks for type into new one.
There are a couple check that the type id is a type for specific
opcodes. Those have been mereged into 1.
Small changes to other test cases to make them valid enough for the
purpose of the test.
In the specification of `OpTypeFunction`, it says
> OpFunction is the only valid use of OpTypeFunction.
This commit add a check in the validator for this rule.
A test started to fail because the new check happens before the check
the test case is testing. Updated the test case to still fail the
check it was suppose to fail originally.
http://crbug.com/874571
* Copy decorations when creating new ids.
When creating a new value based on an old value, we need to copy the
decorations to the new id. This change does this in 3 places:
1) The variable holding the return value of the function generated by
merge return should get decorations from the function.
2) The results of the OpPhi instructions should get decorations from the
variable they are replacing in the ssa writer.
3) In local access chain convert the intermediate struct (result of
OpCompositeInsert) generated for the store replacement should get its
decorations from the variable being stored to.
Fixes#1787.
If seems like at least 1 driver does not like a condition jump to the end
of a selection construct. We are generating these in the merge return
pass. This change stops merge return from generating this sequence.
Part of #1861.
When doing predicate blocks, we need to traverse every block in
structured order in order to keep track of which construct a block is
contained in. The standard way of traversing code in structured order
is to create a list with all of the nodes in order. However, when
predicating blocks, new blocks are created, and those blocks are missed.
This causes branches that go too far.
The solution is to update the order as new blocks are created. Since
we are using an std::list, we do not have to worry about invalidation of
iterators when changing the list.
* Split constant opcode validation out of idUsage and into
validate_constants.cpp
* minor style fixes
* reduced duplication
* fixed an issue with array sizing
* Refactor PredicateBlocks
Refactor PredicateBlocks so that we know which constructs a return
is contained in. Will be used later.
* Have PredicateBlocks jump the existing merge blocks.
In PredicateBlocks, we currently skip instructions with side effects,
but it still follows the same control flow (sort-of). This causes a
problem, when we are trying to predicate code in a loop. We skip all
of the code with side effects (IV increment), but still follow the
same control flow (jump back the start of the loop). This creates an
infinite loop because the code will keep jumping back to the start of
the loop without changing the values that effect the exit condition.
This is a large change to merge-return. When predicating a block that
is in a loop or merge construct, it will jump to the merge block of the
construct. Once out of all constructs we will generate code as we did
before.
* Handle breaks from structured-ifs in DCE.
dead code elimination assumes that are conditional branches except for
breaks and continues in loops will have an OpSelectionMerge before them.
That is not true when breaking out of a selection construct.
The fix is to look for breaks in selection constructs in the same place
we look for breaks and continues for loops.
When dead-branch-elim folds a conditional branch, it also deletes the
OpSelectionMerge instruction. If that construct contains a
conditional branch to the merge node, it will not have its own
OpSelectionMerge. When the headers merge instruction is deleted, the
the inner conditional branch will no longer be legal. It will be a
selection to a node that is not a merge node.
We fix this up by moving the OpSelectionMerge to a new location if it is
still needed.
This forks the testing harness from https://github.com/google/shaderc
to allow testing CLI tools.
New features needed for SPIRV-Tools include:
1- A new PlaceHolder subclass for spirv shaders. This place holder
calls spirv-as to convert assembly input into SPIRV bytecode. This is
required for most tools in SPIRV-Tools.
2- A minimal testing file for testing basic functionality of spirv-opt.
Add tests for all flags in spirv-opt.
1. Adds tests to check that known flags match the names that each pass
advertises.
2. Adds tests to check that -O, -Os and --legalize-hlsl schedule the
expected passes.
3. Adds more functionality to Expect classes to support regular
expression matching on stderr.
4. Add checks for integer arguments to optimization flags.
5. Fixes#1817 by modifying the parsing of integer arguments in
flags that take them.
6. Fixes -Oconfig file parsing (#1778). It reads every line of the file
into a string and then parses that string by tokenizing every group of
characters between whitespaces (using the standard cin reading
operator). This mimics shell command-line parsing, but it does not
support quoting (and I'm not planning to).