This reverts commit d18d0d92e5.
This is reverted because it causes a 7X slowdown when legalizing
SPIR-V with NonSemantic.Shader.DebugInfo.100 instructions.
This is due to the creation of very large UseLists for several
heavily used operands for this extension combined with the fact
that the original commit changed the performance of Uselists to O(n).
Arrays do not have to have a size that is known at compile time. It
could be a spec constant. In these cases, treat the array
as if it is arbitrarily long. This commit will treat it like it is an
array of size UINT32_MAX.
Fixes https://crbug.com/oss-fuzz/47397.
If the `instruction` operand in an extended instruction instruction is
too large, it causes undefined behaviour when that value is cast to the
enum for the corresponding set. This is done with the
NonSemanticDebug100 instruction set. We need to avoid the undefined
behaviour.
Fixes#4727
Which functions are processed is determined by which ones are on the
call tree from the entry points before dead code is removed. So it is
possible that a function is process because it is called from an entry
point, but the CFG is not cleaned up because the call to the function
was removed.
The fix is to process and cleanup every function in the module. Since
all of the dead functions would have already been removed in an earlier
step of DCE, it should not make a different in compile time.
Fixes#4731
* Structural dominance introduced in SPIR-V 1.6 rev2
* Changes the structured cfg validation to use structural dominance
* structural dominance is based on a cfg where merge and continue
declarations are counted as graph edges
* Basic blocks now track structural predecessors and structural
successors
* Add validation for entry into a loop
* Fixed an issue with inlining a single block loop
* The continue target needs to be moved to the latch block
* Simplify the calculation of structured exits
* no longer requires block depth
* Update many invalid tests
We currently build the structured order for all nodes reachable from the
loop header when unrolling a loop. However, unrolling only needs the
nodes in the loop and possibly the merge node.
To avoid needless computation, I have implemented a search that will
stop at the merge node.
Fixes#4827
The code in `CFG::SplitLoopHeader` assumes the loop header is not the
latch. This leads to it not being able to find the latch block. This
has been fixed, and a test added.
Fixes#4527
If the predecessor blocks are the same, then there is only 1 value for the
OpPhi. The simplition pass will simplify it, and it causes problems for
if-conversion. In these cases, if-conversion can just punt.
Fixes#3554.
An access chain could have a constant index that is an out of bounds
access. This is valid spir-v, even if it can cause problems at runtime.
However, it is not valid to have an OpCompositeExtract with an out of
bounds access. This means we have to stop local-access-chain-convert
from making that change.
Fixes#4605
This change adds a folding rule which transforms x * y - a and a - x * y
into FMA(x, y, -a) and FMA(-x, y, a), respectively.
While the SPIR-V instruction count remains the same, target instruction
sets typically feature FMA instruction variants that can negate an
operand. Also this transformation may unlock further optimizations which
eliminate the negation.
(Google bug: b/226145988)
* Add move folding for composite instructions
Fold chains of insert into construct
If a chain of OpCompositeInsert instruction write to every element of a
composite object, then we can replace it with an OpCompositeConstruct.
Fold a construct fed by extracts to a single extract
We already fold an OpCompositeConstruct when it is simlpy reconstructing
an object that was decomposed by a series of OpCompositeExtract
instructions. However, we do not do that if that object is an element
of a larger object.
I have updated the rule, so that if the original object is a an element
of a larger object, then the OpCompositeConstruct is replaced with a
single OpCompositeExtract from the larger object.
Fixes#4371.
* Handle 64-bit integers in local access chain convert
The local access chain convert pass does on run on module that have
64-bit integers, even if they have nothing to to with access chains.
This is very limiting because other passes rely on the access chains
being removed. So this commit will add this functionality to the pass.
spirv validation require OpFunctionCall with memory object, usually this
is non issue as all the functions are inlined.
This pass deal with some case for
DontInline function. accesschain input operand would be replaced new
created variable
This reverts commit 671f6e633f.
PR #4783 was reverted because it caused OpenCL CTS failures for clvk.
The was in clspv, which was not adding the no contract decoration when
it was required. This has been fixed in
https://github.com/google/clspv/pull/845. We can now reapply #4783.
Adding Fma instruction can speed up the code. This was requested by
swiftshader, so they do not have to do this analysis themselves. It can
also help reduce the code size, and the work the ICD compilers have to
do.
spread-volatile-semantics pass spreads Volatile semantics for builtin
variables used by certain execution models based on
VUID-StandaloneSpirv-VulkanMemoryModel-04678 and
VUID-StandaloneSpirv-VulkanMemoryModel-04679 (See "Standalone SPIR-V
Validation" section of Vulkan spec "Appendix A: Vulkan Environment for
SPIR-V"). Therefore, shaders without execution model (e.g., used only
for linkage) are not the target of the pass. This commit lets the pass
just return SuccessWithoutChange in that case.
If the body of the module does not have any ids change, compact ids will
not change the id bound. This can cause problems because the id bound
could be much higher than the largest id in that is used. It should be
reset any time it is not the larger id used + 1.
Fixes#4604
When folding a vector shuffle feeding a vector shuffle, we do not
propagate an 0xFFFFFFFF, which has a special meaning, correctly. We
adjust the value making it lose it meaning as an undefined value.
Fixes#4581
CCP does not want to fold an instruction unless it folds to a constant.
There is an asser to check for this. The question if a spec constant
counts as a constant. The constant folder considers a spec constant a
constand, but CCP does not. I've fixed the assert in CCP to match what
the folder does. It should not require any new changes to CCP.
Swift shader needs a way to inline all functions, even those marked as
DontInline. See https://github.com/KhronosGroup/SPIRV-Tools/pull/4471.
This implements the suggestion I made in the PR. We add a pass that
will remove the DontInline function control, so that the inlining passes
will inline them.
SwiftShader will still have to modify their code to add this pass before
the other passes are run.
The function `BuildInvalideAnalyses` will be rebuilt for every analysis that
has been requested, but it is not necessary. It also can cause problems
because if the CFG needs to be rebuilt, so do the dominator trees.
This change will make the functionality match the description of the
function.
* Optimize DefUseManager allocations
Saves around 30-35% of compilation time.
For inst->use_ids, use a pool linked list instead of allocating vectors for every instruction. For inst->uses, use a "PooledLinkedList"' -- a linked list that has shared storage for all nodes. Neither re-use nodes, instead we do a bulk compaction operation when too much memory is being wasted (tuneable).
Includes separate PooledLinkedList templated datastructure, a very special case construct, but split out to make the code a little easier to understand.
Scalar replacement generates a null when there value for a member will
not be used. The null is used to make sure things are
deterministic in case there is an error.
However, some type cannot be null, so we will change that to use undef.
To keep the code simpler we will always use the undef.
Fixes#3996
The handling of the RayQueryKHR type is not complete in the type
manager. The tests were not picking this up. I've added a test to make
sure that the `GenerateAllTypes` function actually does generate all of
the types. Once it is added there other tests should pick up on the
other parts that were missing.
Add a pass to spread Volatile semantics to variables with SMIDNV,
WarpIDNV, SubgroupSize, SubgroupLocalInvocationId, SubgroupEqMask,
SubgroupGeMask, SubgroupGtMask, SubgroupLeMask, or SubgroupLtMask BuiltIn
decorations or OpLoad for them when the shader model is the ray
generation, closest hit, miss, intersection, or callable shaders. This
pass can be used for VUID-StandaloneSpirv-VulkanMemoryModel-04678 and
VUID-StandaloneSpirv-VulkanMemoryModel-04679 (See "Standalone SPIR-V
Validation" section of Vulkan spec "Appendix A: Vulkan Environment for
SPIR-V").
Handle variables used by multiple entry points:
1. Update error check to make it working regardless of the order of
entry points.
2. For a variable, if it is used by two entry points E1 and E2 and
it needs the Volatile semantics for E1 while it does not for E2
- If VulkanMemoryModel capability is enabled, which means we have to
set memory operation of load instructions for the variable, we
update load instructions in E1, but do not update the ones in E2.
- If VulkanMemoryModel capability is disabled, which means we have
to add Volatile decoration for the variable, we report an error
because E1 needs to add Volatile decoration for the variable while
E2 does not.
For the simplicity of the implementation, we assume that all functions
other than entry point functions are inlined.
The pass to remove the nonsemantic information and instructions
is used for drivers or tools that may not support them. Debug
information was only partially handle, which is causing a
problem. We need to either fully remove debug information or
not remove it all. Since I can see it being useful to keep the
debug information even when the nonsemantic instructions are
removed, I propose we do not remove debug info.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4269
In https://github.com/KhronosGroup/SPIRV-Tools/pull/3110, the strip reflect
pass was changed to also remove all explicitly nonsemantic instructions. This
makes it so that the name of the pass no longer reflects what the pass actually
does. This change renames the pass so that it reflects what the pass actaully does.
* Fix endianness of string literals
To get correct and consistent encoding and decoding of string literals
on big-endian platforms, use spvtools::utils::MakeString and MakeVector
(or wrapper functions) consistently for handling string literals.
- add variant of MakeVector that encodes a string literal into an
existing vector of words
- add variants of MakeString
- add a wrapper spvDecodeLiteralStringOperand in source/
- fix wrapper Operand::AsString to use MakeString (source/opt)
- remove Operand::AsCString as broken and unused
- add a variant of GetOperandAs for string literals (source/val)
... and apply those wrappers throughout the code.
Fixes #149
* Extend round trip test for StringLiterals to flip word order
In the encoding/decoding roundtrip tests for string literals, include
a case that flips byte order in words after encoding and then checks for
successful decoding. That is, on a little-endian host flip to big-endian
byte order and then decode, and vice versa.
* BinaryParseTest.InstructionWithStringOperand: also flip byte order
Test binary parsing of string operands both with the host's and with the
reversed byte order.
This prevents CCP from making constant -> constant transitions when
evaluating instruction values. In this case, FClamp is evaluated twice.
On the first evaluation, if computes FClamp(0.5, 0.5, -1) which returns
-1. On the second evaluation, it computes FClamp(0.5, 0.5, VARYING)
which returns 0.5.
Both fold() computations are correct given the semantics of FClamp() but
this causes a lateral transition in the constant lattice which was not
being considered VARYING by CCP.
Along with OpDecorate, also clone the OpDecorateString instructions for
variables created in the descriptor scalar replacement pass.
Fixesmicrosoft/DirectXShaderCompiler#3705
* https://github.com/KhronosGroup/Vulkan-Docs/issues/666 clearly
specified that interfaces do not require an input if there is an
associated output
* ADCE can now remove unused input variables (though they are kept if
the preserve interfaces option is used)
Fixes#4469
* Checks that decorations only usable with structure members are not
used by OpDecorate or OpDecorateId
* Checks that decorations not allowed on structure members are not used
with OpMemberDecorate
* Checks decoration targets for most core decorations
* Performs some Vulkan specific validation on deorations
If the ids overflow when creating an integer constant in the ir_builder, there will be a nullptr dereference. This is happening from inside merge return.
We need to propagate the error up, and make sure it is handled appropriately.
Consider the new test case. The conditional branch in the continue
block is never marked as live. However, `IsDead` will say it is not
dead, so it does not get deleted. Because it was never marked as live,
`%false` was not mark as live either, but it gets deleted. This results
in invalid code.
To fix this properly, we had to reconsider how branches are handle. We
make the following changes:
1) Terminator instructions that are not branch or OpUnreachable must be
kept, so they are marked as live when initializing the worklist.
2) Branches and OpUnreachable instructions are marked as live if
a) the block does not have a merge instruction and another instruction
in the block is marked as live, or
b) the merge instruction in the same block is marked as live.
3) Any instruction that is not marked as live is removed.
4) If a terminator is to be removed, an OpUnreachable is added. This
happens when the entire block is dead, and the block will be removed.
The OpUnreachable is generated to make sure the block still has a
terminator, and is valid.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4509.
Spirv-opt has not had to handle module with function declarations. This
lead many passes to assume that every function has a body. This is not
always true. This commit will modify a number of passes to handle
function declarations.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4443
When setting default value for spec constants, for numeric bit types smaller
than 32 bits, follow the SPIR-V rules for narrow literals:
- signed integers are sign-extended
- otherwise, upper bits are zero.
Followup to #4588
* test: add a test to show 8/16-bit
* opt/spec_constants: fix bit pattern width checks.
The input bit patterns are always at least 32-bits, so let the test
pass for 8/16-bit values as well. This shouldn't have any effect on the
64-bit patterns I assume this was introduced for.
Do this if Constant or DefUse managers are invalid. Using the
ConstantManager attempts to regenerate the DefUseManager
which is not valid during inlining.
According to spec this opcode is a constant instruction - that's it
can appear outside of function bodies.
Co-authored-by: DmitryBushev <dmitry.bushev@intel.com>
* Don't eliminate dead members from StructuredBuffer as layout(offset) qualifiers cannot be applied to structure fields.
* Traverse arrays when marking structs as fully used.
Co-authored-by: Steven Perron <stevenperron@google.com>
Debug[No]Line are tracked and optimized using the same mechanism that tracks
and optimizes Op[No]Line.
Also:
- Fix missing DebugScope at top of block.
- Allow scalar replacement of access chain in DebugDeclare
* Fix extract with out-of-bounds index
When folding a OpCompositeExtract that is fed by an
OpCompositeConstruct, we handle and out of bounds
index, but only in the case where the result of the
OpCompostiteConstruct is a struct. This change
refactors that folding rule and then improves it to
handle an out-of-bounds access when the result of the
OpCompositeConstruct is a vector.
Includes:
- Shift to use of spirv-header extinst.nonsemantic.shader grammar.json
- Remove extinst.nonsemantic.vulkan.debuginfo.100.grammar.json
- Enable all optimizations for Shader.DebugInfo
Also fixes scalar replacement to only insert DebugValue after all
OpVariables. This is not necessary for OpenCL.DebugInfo, but it is
for Shader.DebugInfo.
Likewise, fixes Private-to-Local to insert DebugDeclare after all
OpVariables.
Also fixes inlining to handle FunctionDefinition which can show up
after first block if early return processing happens.
Co-authored-by: baldurk <baldurk@baldurk.org>
In SPIR-V, integers use 2s complement representation, so that signed
integer overflow and underflow is well defined. However, the constant
folder was causing overflow / underflow at the C++ level. This change
avoids such overflows by performing constant folding for IAdd, ISub and
IMul in the context of unsigned values, which works because signedness
is irrelevant according to the SPIR-V semantics for these instructions.
Fixes#4510.
It is possible that other optimization will propagate
a value into an OpCompositeExtract or OpVectorShuffle
instruction that is larger than the vector size.
Vector DCE has to be able to handle it.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4513.
ADCE does not handle exported functions. This was an explicit decision
because we did not believe that the linkage attribute could be used in
shaders, but it can now. This change has been made.
While fixing this error, I noticed that the OpName for labels is
sometimes removed because the label instructions are not marked
explicitly marked as live. This has able been fixed.
convert-to-sampled-image pass converts images and/or samplers with
given pairs of descriptor set and binding to sampled image.
If a pair of an image and a sampler have the same pair of descriptor
set and binding that is one of the given pairs, they will be
converted to a sampled image. In addition, if only an image has the
descriptor set and binding that is one of the given pairs, it will
be converted to a sampled image as well.
For example, when we have
%a = OpLoad %type_2d_image %texture
%b = OpLoad %type_sampler %sampler
%combined = OpSampledImage %type_sampled_image %a %b
%value = OpImageSampleExplicitLod %v4float %combined ...
1. If %texture and %sampler have the same descriptor set and binding
%combine_texture_and_sampler = OpVaraible %ptr_type_sampled_image_Uniform
...
%combined = OpLoad %type_sampled_image %combine_texture_and_sampler
%value = OpImageSampleExplicitLod %v4float %combined ...
2. If %texture and %sampler have different pairs of descriptor set and binding
%a = OpLoad %type_sampled_image %texture
%extracted_image = OpImage %type_2d_image %a
%b = OpLoad %type_sampler %sampler
%combined = OpSampledImage %type_sampled_image %extracted_image %b
%value = OpImageSampleExplicitLod %v4float %combined ...
* Disallow loading a runtime-sized array
Fixes#4472
* Disallow loading a runtime-sized array or a composite containing one
* Refactor type traversal into a separate function used by both runtime
array checks and sized int/float checks
* Update invalid tests
This PR adds a generic dataflow analysis framework to SPIRV-opt, with the intent of being used in SPIRV-lint. This may also be useful for SPIRV-opt, as existing ad-hoc analyses can be rewritten to use a common framework, but this is not the target of this PR.
Control dependence analysis constructs a control dependence graph,
representing the conditions for a block's execution relative to the
results of other blocks with conditional branches, etc.
This is an analysis pass that will be useful for the linter and
potentially also useful in opt. Currently it is unused except for the
added unit tests.
The new pass will removed interface variable on the OpEntryPoint instruction when they are not statically referenced in the call tree of the entry point.
It can be enabled on the command line using the options `remove-unused-interface-variables`.
Fix dangling phi bug from loop-unroll
When unrolling the following loop:
```
%const0 = OpConstant ...
%const1 = OpConstant ...
...
%LoopHeader = OpLabel
%phi0 = OpPhi %float %const0 %PreHeader %phi1 %Latch
%phi1 = OpPhi %float %const1 %PreHeader %x %Latch
...
%LoopBody = OpLabel
%x = OpFSub %float %phi1 %phi0
...
```
the loop-unroll pass sets the value of `%phi0` as `%phi1` for the second
copy of the loop body. For example, the second copy of
`%x = OpFSub %float %phi1 %phi0` will be
`%y = OpFSub %float %x %phi1`.
Since all phi instructions for inductions will are removed after the
loop unrolling, `%phi1` will be a dead dangling phi.
It happens only for the phi values of the first loop iteration. Replacing those
dangling phis with their initial values fixes this issue.
For example, the second copy of `%x = OpFSub %float %phi1 %phi0` should be
`%y = OpFSub %float %x %const1` because the value of `%phi1` from the
first loop iteration is `%const1`.
This pass converts an internal form of GLSLstd450 Interpolate ops
to the externally valid form. The external form takes the lvalue
of the interpolant. The internal form can do a load of the interpolant.
The pass replaces the load with its pointer. The internal form is
generated by glslang and possibly other frontends for HLSL shaders.
The new pass is called as part of HLSL legalization after all
propagation is complete.
Also adds internal interpolate form to pre-legalization validation
This allows the GPU-AV layer to differentiate between errors with
uniform buffers versus storage buffers and map these to the relevant
VUIDs.
This is a resubmit of a previously reverted commit. The revert was
done as someone erroneously attempted to build the latest validation
layers with a TOT spirv-tools. The validation layers must be built with
their known-good glslang and its known-good spirv-tools and spirv-headers.
* Mark module as modified if convert-to-half removes decorations.
If the convert-to-half pass does not change the body of the function,
but removes decorations, it returns that nothing changed. This is
incorrect, and will be fixed.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4117
* Update comment for RemoveDecorationsFrom
The existing spirv-opt `DebugInfoManager::AddDebugValueForDecl()` sets
the scope and line info of the new added DebugValue using the scope and
line of DebugDeclare. This is wrong because only a single DebugDeclare
must exist under a scope while we have to add DebugValue for all the
places where the variable's value is updated. Therefore, we have to set
the scope and line of DebugValue based on the places of the variable
updates.
This bug makes
https://github.com/google/amber/blob/main/tests/cases/debugger_hlsl_shadowed_vars.amber
fail. This commit fixes the bug.
Propagating the OpLine/OpNoLine to preserve the debug information
through transformations results in integrity check failures because of
the extra line instructions. This commit lets spirv-opt skip the
integrity check when the code contains OpLine or OpNoLine.
When there is an array of strutured buffers, desc sroa will only split
the array, but not a struct type in the structured buffer. However,
the calcualtion of the number of binding a struct requires does not take
this into consideration. This commit will fix that.