Commit Graph

368 Commits

Author SHA1 Message Date
greg-lunarg
c37388f1ad Add passes to propagate and eliminate redundant line instructions (#2027). (#2039)
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.
2018-11-15 14:06:17 -05:00
Steven Perron
dc9d155d62
Fix folding of volatile store. (#2048)
When looking for the Volatile mask on a store, the instruction folder
accesses an out-of-bounds element.  We fix that up.

Fixes crbug.com/903530.
2018-11-14 13:52:18 -05:00
Steven Perron
ec5574a9c6
Instruction::GetBaseAddress to handle OpPtrAccessChain (#2050)
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.
2018-11-14 12:42:25 -05:00
greg-lunarg
1e9fc1aac1 Add base and core bindless validation instrumentation classes (#2014)
* 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.
2018-11-08 13:54:54 -05:00
greg-lunarg
6721478ef1 Don't assume one return means function can be inlined. (#2018) (#2025)
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.
2018-11-08 09:11:20 -05:00
Jeff Bolz
c06a35b902 Rename PCH macro to spvtools_pch to avoid conflicts with other projects. Also add pch to test/opt. (#2034) 2018-11-07 09:15:04 -05:00
Jeff Bolz
60fac96c6b Enable precompiled headers for spirv-tools(-shared) and some unit tests (#2026) 2018-11-06 09:26:23 -05:00
Steven Perron
f2cc71e5cb
Handle OpMemberDecorateStringGOOGLE in ACDE (#2029)
Add missing case to the switch statement for the annotation
instructions.

See https://github.com/KhronosGroup/glslang/issues/1561.
2018-11-02 13:42:45 -04:00
dan sinclair
9e6f5134d1
Reduce number of test targets (#2024)
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.
2018-11-01 10:19:37 -04:00
Steven Perron
6647884a13
Remove MemberDecorateStringGOOGLE during stript-refect. (#2021)
The strip-reflect pass is not removing the reflection decorations that
are decorating members.  With this commit, they will now be removed.

Fixes #2019.
2018-10-30 16:17:35 -04:00
Steven Perron
18fe6d59e5
Fix dead branch elim infinite loop. (#2009)
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.
2018-10-24 09:10:30 -04:00
Steven Perron
0ba35798c3
Fix dead branch elim infinite loop. (#1997)
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.
2018-10-22 13:59:20 -04:00
alan-baker
6e85d1a6fc
Fix restrictions in if conversion (#1998)
Fixes #1991

* Improved identification of potential conditional branches
* Pass changed to only work for shaders
* added a test to catch the bug
2018-10-19 15:16:46 -04:00
Steven Perron
715afb0cea
Add a nullptr check to array copy propagation. (#1987)
We are missing a check for a nullptr that is causing things to fail.

Added an extra test case, and fixed up others.

This is the fix for https://github.com/Microsoft/DirectXShaderCompiler/issues/1598.
2018-10-19 12:53:40 -04:00
greg-lunarg
c4687889b7 Fix ADCE to treat OpUnreachable correctly during liveness analysis (#1984)
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.
2018-10-19 10:16:35 -04:00
Steven Perron
0e68bb3632
Only run merge-returnon reachable functions. (#1983)
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.
2018-10-18 08:48:27 -04:00
greg-lunarg
ab45d69154 Fix ADCE liveness to include all enclosing control structures. (#1975)
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.
2018-10-16 08:00:07 -04:00
Nuno Subtil
5bc30788fd Fix gtest.h include in test/opt/pass_utils.h
Fixes builds where googletest is outside the SPIRV-Tools tree.
2018-10-12 10:22:25 -04:00
Steven Perron
82663f34c9
Check for unreachable blocks in merge-return. (#1966)
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.
2018-10-10 15:18:15 -04:00
Steven Perron
4e266f775a
Fold divisions by 0. (#1963)
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.
2018-10-10 11:17:26 -04:00
alan-baker
fae1e61ab8
Fix bug in construct block calculation (#1964)
Fixes #1960

* Only allows blocks that are dominated by the header
* Fixed a bad loop fusion test
* Added a test derived from the reported bug
2018-10-10 11:14:01 -04:00
Steven Perron
497958d899 Removing HLSLCounterBuffer decorations when not needed. (#1954)
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.
2018-10-05 08:23:09 -04:00
Jaebaek Seo
ebcc58b5f8 Validator: function scope variable at start of entry block #1923
All OpVariable instructions in a function must be the first
instructions in the first block.
2018-10-04 15:05:47 -04:00
Alan Baker
3b5960174f Don't scalarize spec constant sized arrays
Fixes #1952

* Prevent scalarization of arrays that are sized by a specialization
constant
2018-10-04 11:58:23 -04:00
Steven Perron
c4c68712c4
Make EFFCEE required (#1943)
Fixes #1912.

Remove the non-effcee build as EFFCEE is now required.
2018-10-04 10:00:11 -04:00
Alan Baker
a77bb2e54b Add validation for execution modes
* 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
2018-10-02 10:22:23 -04:00
Steven Perron
146eb3bdcf
Fix erroneous uses of the type manager in copy-prop-arrays. (#1942)
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.
2018-10-01 14:45:44 -04:00
Steven Perron
32381e30ef
Handle decoration groups with no decorations. (#1921)
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.
2018-09-28 14:16:04 -04:00
Steven Perron
b85fb4a300
Get KillNameAndDecorates to handle group decorations. (#1919)
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.
2018-09-25 12:57:44 -04:00
Lei Zhang
9bfe0eb25e Wrap tests needing effcee inside SPIRV_EFFCEE 2018-09-20 11:51:40 -04:00
Steven Perron
9fbcce4ca1
Add unrolling to the legalization passes (#1903)
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.
2018-09-19 16:40:09 -04:00
Steven Perron
7075c49923
Add dummy loop in merge-return. (#1896)
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.
2018-09-18 08:52:47 -04:00
Steven Perron
5f599e700e
Fix infinite loop in dead-branch-elimination (#1891)
* 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.
2018-09-17 13:00:24 -04:00
Steven Perron
6d5f1bc2e8
Allow merge blocks to merge two header blocks in some cases. (#1890)
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.
2018-09-14 13:37:18 -04:00
Steven Perron
75c1bf2843
Add option for the max id bound. (#1870)
* 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.
2018-09-10 11:49:41 -04:00
Steven Perron
d746681fe9
Copy decorations when creating new ids. (#1843)
* 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.
2018-08-24 11:55:39 -04:00
Steven Perron
b4d3618f77
Don't "break" from selection constructs. (#1862)
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.
2018-08-23 14:38:25 -04:00
Steven Perron
6c73b1fb70
Update the order when predicating blocks. (#1859)
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.
2018-08-23 12:59:31 -04:00
Steven Perron
19264ef42c
Have PredicateBlocks jump the existing merge blocks. (#1849)
* 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.
2018-08-21 12:04:08 -04:00
Steven Perron
d693a83e36
Handle breaks from structured-ifs in DCE. (#1848)
* 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.
2018-08-21 11:54:44 -04:00
Steven Perron
45c235d41f
Have dead-branch-elim handle conditional exits from selections. (#1850)
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.
2018-08-21 11:49:56 -04:00
Steven Perron
e065cc208f
Keep decorations when replacing loads in access-chain-convert. (#1829)
In local-access-chain-convert, we replace loads by load the entire
variable, then doing the extract.  The extract will have the same value
as the load.  However, if the load has a decoration on it, the
decoration is lost because we do not copy any them to the new id.

This is fixed by rewritting the load into the extract and keeping the
same result id.

This change has the effect that we do not call DCEInst on the loads
because the load is not being deleted, but replaced.  This could leave
OpAccessChain instructions around that are not used.  This is not a
problem for -O and -Os.  They run local_single_*_elim passes and then
dead code elimination.  The dce will remove the unused access chains,
and the load elimination passes work even if there are unused access
chains.  I have added test to them to ensure they will not loss
opportunities.

Fixes #1787.
2018-08-15 09:14:21 -04:00
dan sinclair
ef678672fb
Remove source/message.h (#1838)
The code in source/message was only used in a single set of tests to
format the output results. This CL changes the test to verify the
message instead of all the error values and removes the source/message
code.
2018-08-14 15:41:21 -04:00
dan sinclair
1963a2dbda
Use MakeUnique. (#1837)
This CL replaces instances of reset(new ..) with MakeUnique.
2018-08-14 15:01:50 -04:00
dan sinclair
1553025f4c
Move make_unique to source/util. (#1836)
This MakeUnique code is used in places other then source/opt so move it
to source/utils.
2018-08-14 12:44:54 -04:00
Steven Perron
5c8b4f5a1c
Validate the input to Optimizer::Run (#1799)
* Run the validator in the optimization fuzzers.

The optimizers assumes that the input to the optimizer is valid.  Since
the fuzzers do not check that the input is valid before passing the
spir-v to the optimizer, we are getting a few errors.

The solution is to run the validator in the optimizer to validate the
input.

For the legalization passes, we need to add an extra option to the
validator to accept certain types of variable pointers, even if the
capability is not given.  At the same time, we changed the option
"--legalize-hlsl" to relax the validator in the same way instead of
turning it off.
2018-08-08 11:16:19 -04:00
dan sinclair
05057c9846
Fixup readabilty/inheritance warnings (#1805)
This CL removes the un-needed virtual qualifiers from methods already
marked as override.
2018-08-07 09:10:03 -04:00
dan sinclair
eda2cfbe12
Cleanup includes. (#1795)
This Cl cleans up the include paths to be relative to the top level
directory. Various include-what-you-use fixes have been added.
2018-08-03 15:06:09 -04:00
dan sinclair
7861df9bb3
Remove using namespace commands. (#1794)
This CL removes the last two 'using namespace' commands.
2018-08-03 08:05:52 -04:00
dan sinclair
58a6876cee
Rewrite include guards (#1793)
This CL rewrites the include guards to make PRESUBMIT.py include guard
check happy.
2018-08-03 08:05:33 -04:00