We are seeing shaders that have multiple returns in a functions. These
functions must get inlined for legalization purposes; however, the
inliner does not know how to inline functions that have multiple
returns.
The solution we will go with it to improve the merge return pass to
handle structured control flow.
Note that the merge return pass will assume the cfg has been cleanedup
by dead branch elimination.
Fixes#857.
Previously we keep a separate static grammar table for opcodes/
operands per SPIR-V version. This commit changes that to use a
single unified static grammar table for opcodes/operands.
This essentially changes how grammar facts are queried against
a certain target environment. There are only limited filtering
according to the desired target environment; a symbol is
considered as available as long as:
1. The target environment satisfies the minimal requirement of
the symbol; or
2. There is at least one extension enabling this symbol.
Note that the second rule assumes the extension enabling the
symbol is indeed requested in the SPIR-V code; checking that
should be the validator's work.
Also fixed a few grammar related issues:
* Rounding mode capability requirements are moved to client APIs.
* Reserved symbols not available in any extension is no longer
recognized by assembler.
Strips reflection info. This is limited to decorations and
decoration instructions related to the SPV_GOOGLE_hlsl_functionality1
extension.
It will remove the OpExtension for SPV_GOOGLE_hlsl_functionality1.
It will also remove the OpExtension for SPV_GOOGLE_decorate_string
if there are no further remaining uses of OpDecorateStringGOOGLE.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1398
Optimizations should work in the presence of recent
SPV_GOOGLE_decorate_string and SPV_GOOGLE_hlsl_functionality1
SPV_GOOGLE_decorate_string:
- Adds operation OpDecorateStringGOOGLE to decorate an object with decorations
having string operands.
SPV_GOOGLE_hlsl_functionality1:
- Adds HlslSemanticGOOGLE, used to decorate an interface variable with
an HLSL semantic string. Optimizations already preserve those variables
as required because they are interface variables (with uses), independent
of whether they have HLSL decorations.
- Adds HlslCounterBufferGOOGLE, used to associate a buffer with a
counter variable.
Fixes#1391
This reimplementation fixes several issues when removing decorations associated
to an ID (partially addresses #1174 and gives tools for fixing #898), as well
as making it easier to remove groups; a few additional tests have been added.
DecorationManager::RemoveDecoration() will still not delete dead decorations it
created, but I do not think it is its job either; given the following input
```
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %2 Restrict
%2 = OpDecorationGroup
OpGroupDecorate %2 %1 %3
OpDecorate %4 Invariant
%4 = OpDecorationGroup
OpGroupDecorate %4 %2
%uint = OpTypeInt 32 0
%1 = OpVariable %uint Uniform
%3 = OpVariable %uint Uniform
```
which of the following two outputs would you expect RemoveDecoration(2) to produce:
```
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
%uint = OpTypeInt 32 0
%1 = OpVariable %uint Uniform
%3 = OpVariable %uint Uniform
```
or
```
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %4 Invariant
%4 = OpDecorationGroup
%uint = OpTypeInt 32 0
%1 = OpVariable %uint Uniform
%3 = OpVariable %uint Uniform
```
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/924
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1174
Remove extension whitelists from transforms that are essentially
combinatorial (and avoiding pointers) or which affect only control flow.
It's very very unlikely an extension will add a new control flow construct.
Remove from:
- dead branch elimination
- dead insertion elimination
- insert extract elimination
- block merge
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1392
The default target is SPIR-V 1.3.
For example, spirv-as will generate a SPIR-V 1.3 binary by default.
Use command line option "--target-env spv1.0" if you want to make a SPIR-V
1.0 binary or validate against SPIR-V 1.0 rules.
Example:
# Generate a SPIR-V 1.0 binary instead of SPIR-V 1.3
spirv-as --target-env spv1.0 a.spvasm -o a.spv
spirv-as --target-env vulkan1.0 a.spvasm -o a.spv
# Validate as SPIR-V 1.0.
spirv-val --target-env spv1.0 a.spv
# Validate as Vulkan 1.0
spirv-val --target-env vulkan1.0 a.spv
The merging types we do not remove other information related to the
types. We simply leave it duplicated, and hope it is removed later.
This is what happens with decorations. They are removed in the next
phase of remove duplicates. However, for OpNames that is not the case.
We end up with two different names for the same id, which does not make
sense.
The solution is to remove the names and decorations for the type being
removed instead of rewriting them to refer to the other type.
Note that it is possible that if the first type does not have a name,
then the types will end up with no name. That is fine because the names
should not have any semantic significance anyway.
The was identified in issue #1372, but this does not fix that issue.
* Also mark function parameters as varying
* Conservatively mark assignment instructions as varying if any input is
varying after attempting to fold
* Added a test to catch this case
As per Vulkan spec, BuiltIn variables can't have Location or Component
decorations. On some drivers, these can lead to driver crashing when
compiling the shader pipeline; for example, NVidia/AMD desktop drivers:
https://github.com/KhronosGroup/glslang/issues/1182.
This change adds validation and tests to catch this.
* getFloatConstantKind() now handles OpConstantNull
* PerformOperation() now handles OpConstantNull for vectors
* Fixed some instances where we would attempt to merge a division by 0
* added tests
The algorithm used in DCEInst to remove dead code is very slow. It is
fine if you only want to remove a small number of instructions, but, if
you need to remove a large number of instructions, then the algorithm in
ADCE is much faster.
This PR removes the calls to DCEInst in the load-store removal passes
and adds a pass of ADCE afterwards.
A number of different iterations of the order of optimization, and I
believe this is the best I could find.
The results I have on 3 sets of shaders are:
Legalization:
Set 1: 5.39 -> 5.01
Set 2: 13.98 -> 8.38
Set 3: 98.00 -> 96.26
Performance passes:
Set 1: 6.90 -> 5.23
Set 2: 10.11 -> 6.62
Set 3: 253.69 -> 253.74
Size reduction passes:
Set 1: 7.16 -> 7.25
Set 2: 17.17 -> 16.81
Set 3: 112.06 -> 107.71
Note that the third set's compile time is large because of the large
number of basic blocks, not so much because of the number of
instructions. That is why we don't see much gain there.
Use indirection through latest_version_spirv.h
Also, when generating enum tables, use the unified1 JSON grammar since
it now has FragmentFullyCoveredEXT but the other JSON grammars don't.
They are starting to fall behind.
Adding basis of arithmetic merging
* Refactored constant collection in ConstantManager
* New rules:
* consecutive negates
* negate of arithmetic op with a constant
* consecutive muls
* reciprocal of div
* Removed IRContext::CanFoldFloatingPoint
* replaced by Instruction::IsFloatingPointFoldingAllowed
* Fixed some bad tests
* added some header comments
Added PerformIntegerOperation
* minor fixes to constants and tests
* fixed IntMultiplyBy1 to work with 64 bit ints
* added tests for integer mul merging
Adding test for vector integer multiply merging
Adding support for merging integer add and sub through negate
* Added tests
Adding rules to merge mult with preceding divide
* Has a couple tests, but needs more
* Added more comments
Fixed bug in integer division folding
* Will no longer merge through integer division if there would be a
remainder in the division
* Added a bunch more tests
Adding rules to merge divide and multiply through divide
* Improved comments
* Added tests
Adding rules to handle mul or div of a negation
* Added tests
Changes for review
* Early exit if no constants are involved in more functions
* fixed some comments
* removed unused declaration
* clarified some logic
Adding new rules for add and subtract
* Fold adds of adds, subtracts or negates
* Fold subtracts of adds, subtracts or negates
* Added tests
It moves all conditional branching and switch whose conditions are loop
invariant and uniform. Before performing the loop unswitch we check that
the loop does not contain any instruction that would prevent it
(barriers, group instructions etc.).
In some shaders there are a lot of very large and deeply nested
structures. This creates a lot of work for scalar replacement. Also,
since commit ca4457b we have been very aggressive as rewriting
variables. This has causes a large increase in compile time in creating
and then deleting the instructions.
To help low the costs, I want to run a cleanup of some of the easy loads
and stores to remove. This reduces the number of symbols sroa has to
work on. It also reduces the amount of code the simplifier has to
simplify because it was not generated by sroa.
To confirm the improvement, I ran numbers on three different sets of
shaders:
Time to run --legalize-hlsl:
Set #1: 55.89s -> 12.0s
Set #2: 1m44s -> 1m40.5s
Set #3: 6.8s -> 5.7s
Time to run -O
Set #1: 18.8s -> 10.9s
Set #2: 5m44s -> 4m17s
Set #3: 7.8s -> 7.8s
Contributes to #1328.
Fixes a bug at the same time. In `UpdateDefUse`, if the definition
already exists, we are not suppose to analyse it again. When you do
the entries for the definition are deleted, and we don't want that.
The check for this was wrong.
This function now checks for side-effects before adding operand
instructions to the dead instruction work list.
Because this fix puts more pressure on IsCombinatorInstruction() to
be correct, this commit adds all OpConstant* and OpType* instructions
to combinator_ops_ set.
Fixes#1341.
When inlining a function call the instructions in the same basic block
as the call get cloned. The clone is added to the set of new blocks
containing the inlined code, and the original instructions are deleted.
This PR will change this so that we simply move the instructions to the
new blocks. This saves on the creation and deletion of the
instructions.
Contributes to #1328.
This change implements instruction folding for arithmetic operations
that are redundant, specifically:
x + 0 = 0 + x = x
x - 0 = x
0 - x = -x
x * 0 = 0 * x = 0
x * 1 = 1 * x = x
0 / x = 0
x / 1 = x
mix(a, b, 0) = a
mix(a, b, 1) = b
Cache ExtInst import id in feature manager
This allows us to avoid string lookups during optimization; for now we
just cache GLSL std450 import id but I can imagine caching more sets as
they become utilized by the optimizer.
Add tests for add/sub/mul/div/mix folding
The tests cover scalar float/double cases, and some vector cases.
Since most of the code for floating point folding is shared, the tests
for vector folding are not as exhaustive as scalar.
To test sub->negate folding I had to implement a custom fixture.
Building the def-use chains is very expensive, so we do not want to
invalidate them it if is not necessary. At the moment, it seems like
most optimizatoins are good at not invalidating the def-use chains, but
simplification does.
This PR get the simlification pass to keep the analysies valid.
Contributes to #1328.
On some shader code we have in our testsuite, Phi insertion is showing
massive compile time slowdowns, particularly during destruction. The
specific shader I was looking at has about 600 variables to keep track
of and around 3200 basic blocks. The algorithm is currently O(var x
blocks), which means maps with around 2M entries. This was taking about
8 minutes of compile time.
This patch changes the tracking of stored variables to be more sparse.
Instead of having every basic block contain all the tracked variables in
the map, they now have only the variables actually stored in that block.
This speeds up deallocation, which brings down compile time to about
1m20s.
Note that this is not the definite fix for this. I will re-write Phi
insertion to use a standard SSA rewriting algorithm
(https://github.com/KhronosGroup/SPIRV-Tools/issues/893).
This contributes to
https://github.com/KhronosGroup/SPIRV-Tools/issues/1328.
I mixed up two cases when folding an OpCompositeExtract that is feed by
and OpCompositeInsert. The specific cases are demonstracted in the new
test. I mixed up the conditions for the cases, and treated one like the
other.
Fixes#1323.
* Now track propagation status and assert on bad statuses
* Added helper methods to access instruction propagation status
* Modified the phi meet operator to properly reflect the paper it is
based on
* Modified SSA edge addition so that all edge are added, but only on
state changes
* Fixed a bug in instruction simulation where interesting conditional
branches would not mark the interesting edge as executed
* Added a test to catch this bug
* Added an ostream operator for SSAPropagator::PropStatus
The simplification pass works better after all of the dead branches are
removed. So swapping them around in the legalization passes. Also
adding the simplification pass to performance passes right after dead
branch elimination.
Added CCP to the legalization passes so we can propagate the constants
into the branchs, and remove as many branches a possible. CCP is
designed to still get opportunities even if the branches are dead, so it
is a good place for it.
Fixes#1118
This change handles all 6 regular comparison types in two variations,
ordered (true if values are ordered *and* comparison is true) and
unordered (true if values are unordered *or* comparison is true).
Ordered comparison matches the default floating-point behavior on host
but we use std::isnan to check ordering explicitly anyway.
This change also slightly reworks the floating-point folding support
code to make it possible to define a folding operation that returns
boolean instead of floating point.
These tests exhaustively test ordered/unordered comparisons for
float/double.
Since for NaN inputs the comparison result doesn't depend on the
comparison function, we just test == and !=; NaN inputs result in true
unordered comparisons and false ordered comparisons.
Registering a constant in constant manager establishes a relation
between instruction that defined it and constant object. On complex
shaders this could result in the constant definition getting removed as
part of one of the DCE pass, and a subsequent simplification pass trying
to use the defining instruction for the constant.
To fix this, we now remove associated constant entries from constant
manager when killing constant instructions; the constant object is still
registered and can be remapped to a new instruction later.
GetDefiningInstruction shouldn't ever return nullptr after this change
so add an assertion to check for that.
In dead branch elimination, we already recognize unreachable continue
blocks, and update OpPhi instruction accordingly. This change adds an
extra check: if the head block has exactly 1 other incoming edge, then
replace the OpPhi with the value from that edge.
Fixes#1314.
unordered_map is not POD. Using it as static may cause problems
when operator new() and operator delete() is customized.
Also changed some function signatures to use const char* instead
of std::string, which will give caller the flexibility to avoid
creating a std::string.
We can fold OpSelect into one of the operands in two cases:
- condition is constant
- both results are the same
Even if the original shader doesn't have either of these, if-conversion
pass sometimes ends up generating instructions like
%7127 = OpSelect %int %3220 %7058 %7058
And this optimization cleans them up.
Adding a map from an id to it set of OpName and OpMemberName
instructions. This will be used in KillNameAndDecorates to kill the
names for the ids that are being removed.
In my test, the compile time for 50 shaders went from 1m57s to 55s.
This was on linux using the release build.
Fixes#1290.
This patch adds initial support for loop unrolling in the form of a
series of utility classes which perform the unrolling. The pass can
be run with the command spirv-opt --loop-unroll. This will unroll
loops within the module which have the unroll hint set. The unroller
imposes a number of requirements on the loops it can unroll. These are
documented in the comments for the LoopUtils::CanPerformUnroll method in
loop_utils.h. Some of the restrictions will be lifted in future patches.
There seems to only be a single location where the def-use manager is
used. It is to get information about a type. We can do that with the
type manager instead.
Fixes#1285
Implementation of the simplification pass.
- Create pass that calls the instruction folder on each instruction and
propagate instructions that fold to a copy. This will do copy
propagation as well.
- Did not use the propagator engine because I want to modify the instruction
as we go along.
- Change folding to not allocate new instructions, but make changes in
place. This change had a big impact on compile time.
- Add simplification pass to the legalization passes in place of
insert-extract elimination.
- Added test cases for new folding rules.
- Added tests for the simplification pass
- Added a method to the CFG to apply a function to the basic blocks in
reverse post order.
Contributes to #1164.
Add pkg-config file for shared libraries
Properly build SPIRV-Tools DLL
Test C interface with shared library
Set PATH to shared library file for c_interface_shared test
Otherwise, the test won't find SPIRV-Tools-shared.dll.
Do not use private functions when testing with shared library
Make all symbols hidden by default for shared library target
* Added TypeManager::RebuildType
* rebuilds the type and its constituent types in terms of memory owned
by the manager.
* Used by TypeManager::RegisterType to properly allocate memory
* Adding an unit test to expose the issue
* Added some tests to provide coverage of RebuildType
* Added an accessor to the target pointer for a forward pointer
The combinator initialization was only looking at the capabilities
in the shader and not the inferred capabilities. Geometry and tessellation
shaders were not setting the Shader capability which is inferred. So the
combinator set was not initialized correctly causing problems for ADCE.
Create the folding engine that will
1) attempt to fold an instruction.
2) iterates on the folding so small folding rules can be easily combined.
3) insert new instructions when needed.
I've added the minimum number of rules needed to test the features above.
* Moved initial insert/extract passes later to cover more opportunities
* Added an extra set of passes to clean up opportunities exposed later
in the pipeline
This patch adds LoopUtils class to handle some loop related transformations. For now it has 2 transformations that simplifies other transformations such as loop unroll or unswitch:
- Dedicate exit blocks: this ensure that all exit basic block
(out-of-loop basic blocks that have a predecessor in the loop)
have all their predecessors in the loop;
- Loop Closed SSA (LCSSA): this ensure that all definitions in a loop are used inside the loop
or in a phi instruction in an exit basic block.
It also adds the following capabilities:
- Loop::IsLCSSA to test if the loop is in a LCSSA form
- Loop::GetOrCreatePreHeaderBlock that can build a loop preheader if required;
- New methods to allow on the fly updates of the loop descriptors.
- New methods to allow on the fly updates of the CFG analysis.
- Instruction::SetOperand to allow expression of the index relative to Instruction::NumOperands (to be compatible with the index returned by DefUseManager::ForEachUse)
Creates a pass that will remove instructions that are invalid for the
current shader stage. For the instruction to be considered for replacement
1) The opcode must be valid for a shader modules.
2) The opcode must be invalid for the current shader stage.
3) All entry points to the module must be for the same shader stage.
4) The function containing the instruction must be reachable from an entry point.
Fixes#1247.
* Had to remove templating from InstructionBuilder as a result
* now preserved analyses are specified as a constructor argument
* updated tests and uses
* changed static_assert to a runtime assert
* this should probably get further changes in the future
* When handling unreachable merges and continues, do not optimize to the
same IR
* pass did not check whether the unreachable blocks were in the
optimized form before transforming them
* added a test to catch this issue
* Should handle all possibilities
* Stricter checks for what is disallowed:
* header and header
* merge and merge
* Allow header and merge blocks to be merged
* Erases the structured control declaration if merging header and
merge blocks together.
* If the dead branch elim is performed on a module without structured
control flow, the OpSelectionMerge may not be present
* Add a check for pointer validity before dereferencing
* Added a test to catch the bug
* Forces traversal of phis if the def has changed to varying
* Mark a phi as varying if all incoming values are varying
* added a test to catch the bug
This adds Dead Insert Elimination to the end of the
--eliminate-insert-extract pass. See the new tests for examples of code
that will benefit.
Essentially, this removes OpCompositeInsert instructions which are not
used, either because there is no instruction which uses the value at the
index it is inserted, or because a subsequent insert intercepts any such
use.
This code has been seen to remove significant amounts of dead code from
real-life HLSL shaders being ported to Vulkan. In fact, it is needed to
remove dead texture samples which cause Vulkan validation layer errors
(unbound textures and samplers) if not removed . Such DCE is thus
required for fxc equivalence and legalization.
This analysis operates across "chains" of Inserts which can also contain
Phi instructions.
* Handles simple cases only
* Identifies phis in blocks with two predecessors and attempts to
convert the phi to an select
* does not perform code motion currently so the converted values must
dominate the join point (e.g. can't be defined in the branches)
* limited for now to two predecessors, but can be extended to handle
more cases
* Adding if conversion to -O and -Os
Ban floating point case for OpAtomicLoad, OpAtomicExchange,
OpAtomicCompareExchange. In graphics (Shader) environments, these
instructions only operate on scalar integers. Ban the floating point
case. OpenCL supports atomic_float.
Implemented Vulkan-specific rules:
- OpTypeImage must declare a scalar 32-bit float or 32-bit integer type
for the “Sampled Type”.
- OpSampledImage must only consume an “Image” operand whose type has its
“Sampled” operand set to 1.
The current folding routines have a very cumbersome interface, make them
harder to use, and not a obvious how to extend.
This change is to create a new interface for the folding routines, and
show how it can be used by calling it from CCP.
This does not make a significant change to the behaviour of CCP. In
general it should produce the same code as before; however it is
possible that an instruction that takes 32-bit integers as inputs and
the result is not a 32-bit integer or bool will not be folded as before.
It seems like andriod has a problem with INT32_MAX and the like. I'll
explicitly define those if the are not already defined.
The class factorize the instruction building process.
Def-use manager analysis can be updated on the fly to maintain coherency.
To be updated to take into account more analysis.
* AddToWorklist can now be called unconditionally
* It will only add instructions that have not already been marked as
live
* Fixes a case where a merge was not added to the worklist because the
branch was already marked as live
* Added two similar tests that fail without the fix
We have come across a driver bug where and OpUnreachable inside a loop
is causing the shader to go into an infinite loop. This commit will try
to avoid this bug by turning OpUnreachable instructions that are
contained in a loop into branches to the loop merge block.
This is not added to "-O" and "-Os" because it should only be used if
the driver being targeted has this problem.
Fixes#1209.
This ensure that all basic blocks in a function have a valid entry the CFG object.
The entry block has no predecessors but remains a valid basic block
for which we might want to query the number of predecessors.
Some unreachable basic blocks may not have predecessors as well.
At the moment specialization constants look like constants to ccp. This
causes a problem because they are handled differently by the constant
manager.
I choose to simply skip over them, and not try to add them to the value
table. We can do specialization before ccp if we want to be able to
propagate these values.
Fixes#1199.
With work that Alan has done, some passes have become redundant. ADCE
now removed unused variables. Dead branch elimination removes
unreachable blocks. This means we can remove CFG Cleanup and dead
variable elimination.
The current code expects the users of the constant manager to initialize
it with all of the constants in the module. The problem is that you do
not want to redo the work multiple times. So I decided to move that
code to the constructor of the constant manager. This way it will
always be initialized on first use.
I also removed an assert that expects all constant instructions to be
successfully mapped. This is because not all OpConstant* instruction
can map to a constant, and neither do the OpSpecConstant* instructions.
The real problem is that an OpConstantComposite can contain a member
that is OpUndef. I tried to treat OpUndef like OpConstantNull, but this
failed because an OpSpecConstantComposite with an OpUndef cannot be
changed to an OpConstantComposite. Since I feel this case will not be
common, I decided to not complicate the code.
Fixes#1193.
* Added for Instruction, BasicBlock, Function and Module
* Uses new disassembly functionality that can disassemble individual
instructions
* For debug use only (no caching is done)
* Each output converts module to binary, parses and outputs an
individual instruction
* Added a test for whole module output
* Disabling Microsoft checked iterator warnings
* Updated check_copyright.py to accept 2018
* Changed MemPass::InsertPhiInstructions to set basic blocks for new
phis
* Local SSA elim now maintains instr to block mapping
* Added a test and confirmed it fails without the updated phis
* IRContext::set_instr_block no longer builds the map if the analysis is
invalid
* Added instruction to block mapping verification to
IRContext::IsConsistent()
This improves Extract replacement to continue through VectorShuffle.
It will also handle Mix with 0.0 or 1.0 in the a-value of the desired
component.
To facilitate optimization of VectorShuffle, the algorithm was refactored
to pass around the indices of the extract in a vector rather than pass the
extract instruction itself. This allows the indices to be modified as the
algorithm progresses.
The current folding routines have a very cumbersome interface, make them
harder to use, and not a obvious how to extend.
This change is to create a new interface for the folding routines, and
show how it can be used by calling it from CCP.
This does not make a significant change to the behaviour of CCP. In
general it should produce the same code as before; however it is
possible that an instruction that takes 32-bit integers as inputs and
the result is not a 32-bit integer or bool will not be folded as before.
Modified ADCE to remove dead globals.
* Entry point and execution mode instructions are marked as alive
* Reachable functions and their parameters are marked as alive
* Instruction deletion now deferred until the end of the pass
* Eliminated dead insts set, added IsDead to calculate that value
instead
* Ported applicable dead variable elimination tests
* Ported dead constant elim tests
Added dead function elimination to ADCE
* ported dead function elim tests
Added handling of decoration groups in ADCE
* Uses a custom sorter to traverse decorations in a specific order
* Simplifies necessary checks
Updated -O and -Os pass lists.
Pass now paints live blocks and fixes constant branches and switches as
it goes. No longer requires structured control flow. It also removes
unreachable blocks as a side effect. It fixes the IR (phis) before doing
any code removal (other than terminator changes).
Added several unit tests for updated/new functionality.
Does not remove dead edge from a phi node:
* Checks that incoming edges are live in order to retain them
* Added BasicBlock::IsSuccessor
* added test
Fixing phi updates in the presence of extra backedge blocks
* Added tests to catch bug
Reworked how phis are updated
* Instead of creating a new Phi and RAUW'ing the old phi with it, I now
replace the phi operands, but maintain the def/use manager correctly.
For unreachable merge:
* When considering unreachable continue blocks the code now properly
checks whether the incoming edge will continue to be live.
Major refactoring for review
* Broke into 4 major functions
* marking live blocks
* marking structured targets
* fixing phis
* deleting blocks
This fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1143.
When an instruction transitions from constant to bottom (varying) in the
lattice, we were telling the propagator that the instruction was
varying, but never updating the actual value in the values table.
This led to incorrect value substitutions at the end of propagation.
The patch also re-enables CCP in -O and -Os.
In HLSL structured buffer legalization, pointer to pointer types
are emitted to indicate a structured buffer variable should be
treated as an alias of some other variable. We need an option to
relax the check of pointer types in logical addressing mode to
catch other validation errors.
Add post-order tree iterator.
Add DominatorTreeNode extensions:
- Add begin/end methods to do pre-order and post-order tree traversal from a given DominatorTreeNode
Add DominatorTree extensions:
- Add begin/end methods to do pre-order and post-order tree traversal
- Tree traversal ignore by default the pseudo entry block
- Retrieve a DominatorTreeNode from a basic block
Add loop descriptor:
- Add a LoopDescriptor class to register all loops in a given function.
- Add a Loop class to describe a loop:
- Loop parent
- Nested loops
- Loop depth
- Loop header, merge, continue and preheader
- Basic blocks that belong to the loop
Correct a bug that forced dominator tree to be constantly rebuilt.
Turn `Linker::Link()` into free functions
As very little information was kept in the Linker class, we can get rid
of the whole class and have the `Link()` as free functions instead; the
environment target as well as the consumer are passed along through an
`spv_context` object.
The resulting linked_binary is passed as a pointer rather than a
reference to follow the Google C++ Style guidelines.
Addresses remaining comments from
https://github.com/KhronosGroup/SPIRV-Tools/pull/693 about the SPIR-V
linker.
Fix variable naming in the linker
Some of the variables were using mixed case, which did not follow the
Google C++ Style guidelines.
Linker: Use EXPECT_EQ when possible and update some test
* Replace occurrences of ASSERT_EQ by EXPECT_EQ when possible;
* Reformulated some of the error messages;
* Added the symbol name in the error message when there is a type or
decoration mismatch between the imported and exported declarations.
Opt: List all duplicates removed by RemoveDuplicatePass in the header
Opt: Make the const version of GetLabelInst() return a pointer
For consistency with the non-const version, as well as other similar
functions.
Opt: Rename function_end to EndInst()
As pointed out by dneto0 the previous name was quite confusing and could
be mistaken with a function returning an end iterator.
Also change the return type of the const version to a pointer rather
than a reference, for consistency.
Opt: Add performance comment to RemoveDuplicateTypes and decorations
This comment was requested during the review of
https://github.com/KhronosGroup/SPIRV-Tools/pull/693.
Opt: Add comments and fix variable naming in RemoveDuplicatePass
* Add missing comments to private functions;
* Rename variables that were using mixed case;
* Add TODO for moving AreTypesEqual out.
Linker: Remove commented out code and add TODOs
Linker: Merged together strings that were too much splitted
Implement a C++ RAII wrapper around spv_context
In value numbering, we treat loads and stores of images, ie OpImageLoad,
as a memory operation where it is interested in the "base address" of
the instruction. In those cases, it is an image instruction.
The problem is that `Instruction::GetBaseAddress()` does not account for
the image instructions, so the assert at the end to make sure it found
a valid base address for its addressing mode fails.
The solution is to look at the load/store instruction to determine how
the assertion should be done.
Fixes#1160.
This fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1159. I
had missed a nuance in the original algorithm. When simulating Phi
instructions, the SSA edges out of a Phi instruction should never be
added to the list of edges to simulate.
Phi instructions can be in SSA def-use cycles with other Phi
instructions. This was causing the propagator to fall into an infinite
loop when the same def-use edge kept being added to the queue.
The original algorithm in the paper specifically separates the visit of
a Phi instruction vs the visit of a regular instruction. This fix makes
the implementation match the original algorithm.
When deleting branches and blocks, also remove them from
the backedges set, in case they were there.
This prevents us from keeping stale pointers to deleted Instruction
objects. That memory could be used later by another instruction,
incorrectly signaling that something has a backedge reference, and
the dead branch eliminator could end up deleting live blocks.
Adds accessor method ir::BasicBlock::terminator
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1168
Adds optimizer API to write disassembly to a given output stream
before each pass, and after the last pass.
Adds spirv-opt --print-all option to write disassembly to stderr
before each pass, and after the last pass.
1. Added OpCompositeExtract/Insert out of bounds checks where possible
(everything except RuntimeArray)
2. Moved validation of OpCompositeExtract/Insert from validate_id.cpp to
validate_composites.cpp.
I've a few passes the legalization passes. The first is to add the
more specialized load-store removal passes to help improve the compile
time, as was suggested in #1118.
I've also added dead branch elimination while we wait for the behaviour
of dead branch elimination to be folded into CFG cleanup.
I did not add CCP because it seems like most of the constant propagation
what is needed is already being done by the load-store removal passes,
which call `ReplaceAllUsesWith`. We can reconsider this if needed.
Calling `ToNop` leaves around instructions that are pointless. In
general it is better to remove the instruction completely. That way
other optimizations will not need to look at them.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1003.
In CCP we should not need to insert Phi nodes because CCP never looks at
loads/stores. This required adjusting two tests that relied on Phi
instructions being inserted. I changed the tests to have the Phi
instructions pre-inserted.
I also added a new test to make sure that CCP does not try to look
through stores and loads.
Finally, given that CCP does not handle loads/stores, it's better to run
mem2reg before it. I've changed the -O/-Os schedules to run local
multi-store elimination before CCP.
Although this is just an efficiency fix for CCP, it is
also working around a bug in Phi insertion. When Phi instructions are
inserted, they are never associated a basic block. This causes a
segfault when the propagator tries to lookup CFG edges when analyzing
Phi instructions.
This addresses review feedback for the CCP implementation (which fixes
https://github.com/KhronosGroup/SPIRV-Tools/issues/889).
This adds more protection around the folding of instructions that would
not be supported by the folder.
Add grammar file for DebugInfo extended instruction set
- Each new operand enum kind in extinst.debuginfo.grammar.json maps
to a new value in spv_operand_type_t.
- Add new concrete enum operand types for DebugInfo
Generate a C header for the DebugInfo extended instruction set
Add table lookup of DebugInfo extended instrutions
Handle the debug info operand types in binary parser,
disassembler, and assembler.
Add DebugInfo round trip tests for assembler, disassembler
Android.mk: Support DebugInfo extended instruction set
The extinst.debuginfo.grammar.json file is currently part of
SPIRV-Tools source.
It contributes operand type enums, so it has to be processed
along with the core grammar files.
We also generate a C header DebugInfo.h.
Add necessary grammar file processing to Android.mk.
This implements the conditional constant propagation pass proposed in
Constant propagation with conditional branches,
Wegman and Zadeck, ACM TOPLAS 13(2):181-210.
The main logic resides in CCPPass::VisitInstruction. Instruction that
may produce a constant value are evaluated with the constant folder. If
they produce a new constant, the instruction is considered interesting.
Otherwise, it's considered varying (for unfoldable instructions) or
just not interesting (when not enough operands have a constant value).
The other main piece of logic is in CCPPass::VisitBranch. This
evaluates the selector of the branch. When it's found to be a known
value, it computes the destination basic block and sets it. This tells
the propagator which branches to follow.
The patch required extensions to the constant manager as well. Instead
of hashing the Constant pointers, this patch changes the constant pool
to hash the contents of the Constant. This allows the lookups to be
done using the actual values of the Constant, preventing duplicate
definitions.
In order to keep track of all of the implicit capabilities as well as
the explicit ones, we will add them all to the feature manager. That is
the object that needs to be queried when checking if a capability is
enabled.
The name of the "HasCapability" function in the module was changed to
make it more obvious that it does not check for implied capabilities.
Keep an spv_context and AssemblyGrammar in IRContext
* changed the way duplicate types are removed to stop copying
instructions
* Reworked RemoveDuplicatesPass::AreTypesSame to use type manager and
type equality
* Reworked TypeManager memory management to store a pool of unique
pointers of types
* removed unique pointers from id map
* fixed instances where free'd memory could be accessed
Changes the set of optimizations done for legalization. While doing
this, I added documentation to explain why we want each optimization.
A new option "--legalize-hlsl" is added so the legalization passes can
be easily run from the command line.
The legalize option implies skip-validation.
A few optimizations are updates to handle code that is suppose to be
using the logical addressing mode, but still has variables that contain
pointers as long as the pointer are to opaque objects. This is called
"relaxed logical addressing".
|Instruction::GetBaseAddress| will check that pointers that are use meet
the relaxed logical addressing rules. Optimization that now handle
relaxed logical addressing instead of logical addressing are:
- aggressive dead-code elimination
- local access chain convert
- local store elimination passes.
When a private variable is used in a single function, it can be
converted to a function scope variable in that function. This adds a
pass that does that. The pass can be enabled using the option
`--private-to-local`.
This transformation allows other transformations to act on these
variables.
Also moved `FindPointerToType` from the inline class to the type manager.
- Test validation success for OpEmitVertex OpEndPrimitive
- Test missing capabilities for primitive instructions
- Primitive instructions require Geometry execution model
@ehsannas had filed an issue against SPIR-V spec, concerning
Image Operands section (3.14):
Sample
A following operand is the sample number of the sample to use. Only
valid with OpImageFetch, OpImageRead, and OpImageWrite.
Relaxing the check to allow OpImageSparseRead and
OpImageSparseFetch to fix failing tests.
types. This allows the lookup of type declaration ids from arbitrarily
constructed types. Users should be cautious when dealing with non-unique
types (structs and potentially pointers) to get the exact id if
necessary.
* Changed the spec composite constant folder to handle ambiguous composites
* Added functionality to create necessary instructions for a type
* Added ability to remove ids from the type manager
This fixes issue #1075
- Mark continue when conditional branch with merge block.
Only mark if merge block is not continue block.
- Handle conditional branch break with preceding merge
Inlining is not setting the parent (function) for each basic block.
This can cause problems for later optimizations. The solution is to set
the parent for each new block just before it is linked into the
function.
include: Add target environment enums for OpenCL 1.2 and 2.0
Validator: Validate OpenCL capabilities
Update validate capabilities to handle embedded profiles
Add test for OpenCL capabilities validation
Update messages to mention the OpenCL profile used
Re-format val_capability_test.cpp
Adds a scalar replacement pass. The pass considers all function scope
variables of composite type. If there are accesses to individual
elements (and it is legal) the pass replaces the variable with a
variable for each composite element and updates all the uses.
Added the pass to -O
Added NumUses and NumUsers to DefUseManager
Added some helper methods for the inst to block mapping in context
Added some helper methods for specific constant types
No longer generate duplicate pointer types.
* Now searches for an existing pointer of the appropriate type instead
of failing validation
* Fixed spec constant extracts
* Addressed changes for review
* Changed RunSinglePassAndMatch to be able to run validation
* current users do not enable it
Added handling of acceptable decorations.
* Decorations are also transfered where appropriate
Refactored extension checking into FeatureManager
* Context now owns a feature manager
* consciously NOT an analysis
* added some test
* fixed some minor issues related to decorates
* added some decorate related tests for scalar replacement
This patch adds a new constant manager class to interface with
analysis::Constant. The new constant manager lives in ir::IRContext
together with the type manager (analysis::TypeManager).
The new analysis::ConstantManager is used by the spec constant folder
and the constant propagator (in progress).
Another cleanup introduced by this patch removes the ID management from
the fold spec constant pass, and ir::IRContext and moves it to
ir::Module. SSA IDs were maintained by IRContext and Module. That's
pointless and leads to mismatch IDs. Fixed by moving all the bookkeeping
to ir::Module.
Adds a pass that looks for redundant instruction in a function, and
removes them. The algorithm is a hash table based value numbering
algorithm that traverses the dominator tree.
This pass removes completely redundant instructions, not partially
redundant ones.
Currently when inlining a call, the name and decorations for the result of the
call is not deleted. This should be changed. Added a test for this as well.
This fixes issue #622.
Commit 8ec62deb2 merged the code from PR #810, but it also re-introduces
code that had been removed in #885.
This patch removes the (now superfluous code).
Support for dominator and post dominator analysis on ir::Functions. This patch contains a DominatorTree class for building the tree and DominatorAnalysis and DominatorAnalysisPass classes for interfacing and caching the built trees.
In DCEInst, it is possible that the same instruction ends up in the
queue multiple times, if the same id is used multiple times in the
same instruction.
The solution is to keep the ids in a set, to ensure no duplication in
the list.
The current method of removing an instruction is to call ToNop. The
problem with this is that it leaves around an instruction that later
passes will look at. We should just delete the instruction.
In MemPass there is a utility routine called DCEInst. It can delete
essentially any instruction, which can invalidate pointers now that they
are actually deleted. The interface was changed to add a call back that
can be used to update any local data structures that contain
ir::Intruction*.
Computing the value numbers on demand, as we do now, can lead to
different results depending on the order in which the users asks for
the value numbers. To make things more stable, we compute them ahead
of time.
Previously we required _PRINT to enable _COLOR, which forbids
outputting colored disassembly into a string in library.
This commit will allow library users to request enabling
ANSI color escape sequences.
This needs custom code since the rules from the extension
are not encoded in the grammar.
Changes are:
- The new group instructions don't require Group capability
when the extension is declared.
- The Reduce, InclusiveScan, ExclusiveScan normally require the Kernel
capability, but don't when the extension is declared.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/991
This should fix#1034. It changes the predicate on operator< to use
label IDs from each block and compares them as std:pair to define a weak
ordering for std::set.
This class implements a generic value propagation algorithm based on the
conditional constant propagation algorithm proposed in
Constant propagation with conditional branches,
Wegman and Zadeck, ACM TOPLAS 13(2):181-210.
The implementation is based on
A Propagation Engine for GCC
Diego Novillo, GCC Summit 2005
http://ols.fedoraproject.org/GCC/Reprints-2005/novillo-Reprint.pdf
The purpose of this implementation is to act as a common framework for any
transformation that needs to propagate values from statements producing new
values to statements using those values.
Re-formatted the source tree with the command:
$ /usr/bin/clang-format -style=file -i \
$(find include source tools test utils -name '*.cpp' -or -name '*.h')
This required a fix to source/val/decoration.h. It was not including
spirv.h, which broke builds when the #include headers were re-ordered by
clang-format.
Removed the check that result type of OpImageRead should be a vector4.
Will reenable/adapt once the spec is clarified on what the right
dimension should be.
