The comment in `Array::GetExtraHashWords` is misleading because getting
the hash words is split up between the generic `Type::GetHashWords` and
the type specific `Type::GetExtraHashWords`. While `IsSameImpl` is
self-contained. Removing the comment since it is misleading and no
comment is really needed.
Fixes#3248
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.
The change in
commit 4ac8e5e541
Author: Greg Fischer <greg@lunarg.com>
Date: Wed Sep 15 12:38:34 2021 -0600
Add preserve_interface mode to aggressive_dead_code_elim (#4520)
Broke the C++ ABI for spirv-tools shared libraries on Linux, for not a great reason.
Restore the previous ABI.
* 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.
Currently if an ID overflow occurs, spirv-opt (and other users of
IRContext) emits a warning and starts returning 0 when fresh ids are
requested. This tends to lead to crashes - such as null pointer
exceptions. When these arise during fuzzing they lead to auto-reported
bugs.
This change uses an ifdef guard to instead gracefully exit as soon as an
ID overflow occurs when the build is a fuzzing build.
Related issue: #4539.
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)
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
Having IsLocalVar work only sometimes is something that could easily
lead to an error. This change refactors the code so that the function
can be called at any point. The current implementation was used because
we did not want to do multiple searches to see if a function was an
entry point or if it had a call. This was maintained by added a cache
that will store of a given function is an entry point with no calls.
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>
* Have ADCE use cfg struct analysis (NFC)
ADCE has a lot of code and variables to keep track of
information that is easily obtains using the Struct
cfg analysis. Most of this change is to refactor the
code to have small functions to get the information
from the struct cfg analysis.
A few other changes small refactoring changes are
done.
* Factor out work list initialization in ADCE (NFC)
We move the code that will initially populate the work list into its own
function. We also simplify the code by making use of the struct cfg
analysis. That way we can reduce the number of tables used to track
information as we traverse the CFG.
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 ...
Only the first two operands were tested for constness, missing the third
one. Since the FoldFPBinaryOp() at the end of FoldClamp1() returns null
when not both of its operands are constant, this doesn't change any
behavior, but it avoids some needless work.
Also the comment for FoldClamp2() was fixed.