Commit Graph

22 Commits

Author SHA1 Message Date
Steven Perron
86e45efe15
Handle decorations better in some optimizations (#2716)
There are a couple spots where we are not looking at decorations when we should.

1. Value numbering is suppose to assign a different value number to ids if they have different decorations.  However that is not being done for OpCopyObject and OpPhi.

1. Instruction simplification is propagating OpCopyObject instruction without checking for decorations.  It should only do that if no decorations are being lost.

Add a new function to the decoration manager to check if the decorations of one id are a subset of the decorations of another.

Fixes #2715.
2019-07-10 11:37:16 -04:00
Steven Perron
e07dabc25f
Invalidate the decoration manager at the start of ADCE. (#2189)
* Invalidate the decoration manager at the start of ADCE.

If the decoration manager is kept live the the contex will try to keep
it up to date.  ADCE deals with group decorations by changing the
operands in |OpGroupDecorate| instructions directly without informing
the decoration manager.  This puts it in an invalid state, which will
cause an error when the context tries to update it.  To Avoid this
problem, we will invalidate the decoration manager upfront.

At the same time, the decoration manager is now considered when checking
the consistency of the decoration manager.
2018-12-10 13:24:33 -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
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
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
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
c7da51a085
Cleanup extraneous namespace qualifies in source/opt. (#1716)
This CL follows up on the opt namespacing CLs by removing the
unnecessary opt:: and opt::analysis:: namespace prefixes.
2018-07-12 15:14:43 -04:00
dan sinclair
e6b953361d
Move the ir namespace to opt. (#1680)
This CL moves the files in opt/ to consistenly be under the opt::
namespace. This frees up the ir:: namespace so it can be used to make a
shared ir represenation.
2018-07-09 11:32:29 -04:00
David Neto
884933366b Teach DecorationManager about OpDecorateStringGOOGLE
Also add more decoration manager test coverage for OpDecorateId.

Fixes #1396
2018-03-13 22:18:33 -04:00
Pierre Moreau
5bd55f10cd Reimplement the DecorationManager
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
2018-03-12 09:56:14 -04:00
Alan Baker
6587d3f8a3 Adding early exit versions of several ForEach* methods
* Looked through code for instances where code would benefit from early
exit
 * Added a corresponding WhileEach* method and updated the code
2018-01-12 17:05:09 -05:00
Pierre Moreau
702852bd22 Opt: Make DecorationManager::HaveTheSameDecorations symmetric
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1112

Also: Add SpvOpDecorateId to IsAnnotationInst()
2018-01-04 14:07:25 -05:00
Pierre Moreau
424f744db1 Opt: Fix implementation and comment of AreDecorationsTheSame
Target should not be ignored when comparing decorations in RemoveDuplicates
Opt: Remove unused code in RemoveDuplicateDecorations
2017-12-19 15:36:47 -05:00
David Neto
188cd3780d Erase decorations removed from internal collections
Fixes Android arm-64-v8a build with NDK r14.  That's because
we no longer ignore the result of the std::remove.
2017-11-30 11:35:02 -05:00
Diego Novillo
83228137e1 Re-format source tree - NFC.
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.
2017-11-27 14:31:49 -05:00
Alan Baker
746bfd210a Adding new def -> use mapping container
Replaced representation of uses

* Changed uses from unordered_map<uint32_t, UseList> to
set<pairInstruction*, Instruction*>>
* Replaced GetUses with ForEachUser and ForEachUse functions
* updated passes to use new functions
* partially updated tests
* lots of cleanup still todo

Adding an unique id to Instruction generated by IRContext

Each instruction is given an unique id that can be used for ordering
purposes. The ids are generated via the IRContext.

Major changes:
* Instructions now contain a uint32_t for unique id and a cached context
pointer
 * Most constructors have been modified to take a context as input
 * unfortunately I cannot remove the default and copy constructors, but
 developers should avoid these
* Added accessors to parents of basic block and function
* Removed the copy constructors for BasicBlock and Function and replaced
them with Clone functions
* Reworked BuildModule to return an IRContext owning the built module
 * Since all instructions require a context, the context now becomes the
basic unit for IR
* Added a constructor to context to create an owned module internally
* Replaced uses of Instruction's copy constructor with Clone whereever I
found them
* Reworked the linker functionality to perform clones into a different
context instead of moves
* Updated many tests to be consistent with the above changes
 * Still need to add new tests to cover added functionality
* Added comparison operators to Instruction

Adding tests for Instruction, IRContext and IR loading

Fixed some header comments for BuildModule

Fixes to get tests passing again

* Reordered two linker steps to avoid use/def problems
* Fixed def/use manager uses in merge return pass
* Added early return for GetAnnotations
* Changed uses of Instruction::ToNop in passes to IRContext::KillInst

Simplifying the uses for some contexts in passes
2017-11-23 16:40:02 -05:00
Alan Baker
a771713e42 Adding an unique id to Instruction generated by IRContext
Each instruction is given an unique id that can be used for ordering
purposes. The ids are generated via the IRContext.

Major changes:
* Instructions now contain a uint32_t for unique id and a cached context
pointer
 * Most constructors have been modified to take a context as input
 * unfortunately I cannot remove the default and copy constructors, but
 developers should avoid these
* Added accessors to parents of basic block and function
* Removed the copy constructors for BasicBlock and Function and replaced
them with Clone functions
* Reworked BuildModule to return an IRContext owning the built module
 * Since all instructions require a context, the context now becomes the
basic unit for IR
* Added a constructor to context to create an owned module internally
* Replaced uses of Instruction's copy constructor with Clone whereever I
found them
* Reworked the linker functionality to perform clones into a different
context instead of moves
* Updated many tests to be consistent with the above changes
 * Still need to add new tests to cover added functionality
* Added comparison operators to Instruction
* Added an internal option to LinkerOptions to verify merged ids are
unique
* Added a test for the linker to verify merged ids are unique

* Updated MergeReturnPass to supply a context
* Updated DecorationManager to supply a context for cloned decorations

* Reworked several portions of the def use tests in anticipation of next
set of changes
2017-11-20 17:49:10 -05:00
Steven Perron
eb4653a67f Add the decoration manager to the IRContext.
To make the decoration manger available everywhere, and to reduce the
number of times it needs to be build, I add one the IRContext.

As the same time, I move code that modifies decoration instruction into
the IRContext from mempass and the decoration manager.  This will make
it easier to keep everything up to date.

This should take care of issue #928.
2017-11-15 12:48:03 -05:00
Daniel Schürmann
a76d0977ac Fix decorations of inlined functions.
Fixes issue #728.  Currently the inliner is not generating decorations for
inlined code which corresponds to function code which has decorations. An
example of decorations that are relevant: RelaxedPrecision, NoContraction.

The solution is to replicate the decoration during inlining.
2017-11-13 12:49:25 -05:00
Diego Novillo
d2938e4842 Re-format files in source, source/opt, source/util, source/val and tools.
NFC. This just makes sure every file is formatted following the
formatting definition in .clang-format.

Re-formatted with:

$ clang-format -i $(find source tools include -name '*.cpp')
$ clang-format -i $(find source tools include -name '*.h')
2017-11-08 14:03:08 -05:00
Steven Perron
5834719fc1 Add pass to remove dead variables at the module level.
There does not seem to be any pass that remove global variables.  I
think we could use one.  This pass will look specifically for global
variables that are not referenced and are not exported.  Any decoration
associated with the variable will also be removed.  However, this could
cause types or constants to become unreferenced.  They will not be
removed.  Another pass will have to be called to remove those.
2017-10-23 13:57:05 -04:00
Pierre Moreau
86627f7b3f Implement Linker (module combiner)
Add extra iterators for ir::Module's sections
Add extra getters to ir::Function
Add a const version of BasicBlock::GetLabelInst()

Use the max of all inputs' version as version

Split debug in debug1 and debug2
- Debug1 instructions have to be placed before debug2 instructions.

Error out if different addressing or memory models are found

Exit early if no binaries were given

Error out if entry points are redeclared

Implement copy ctors for Function and BasicBlock
- Visual Studio ends up generating copy constructors that call deleted
  functions while compiling the linker code, while GCC and clang do not.
  So explicitly write those functions to avoid Visual Studio messing up.

Move removing duplicate capabilities to its own pass

Add functions running on all IDs present in an instruction

Remove duplicate SpvOpExtInstImport

Give default options value for link functions

Remove linkage capability if not making a library

Check types before allowing to link

Detect if two types/variables/functions have different decorations

Remove decorations of imported variables/functions and their types

Add a DecorationManager

Add a method for removing all decorations of id

Add methods for removing operands from instructions

Error out if one of the modules has a non-zero schema

Update README.md to talk about the linker

Do not freak out if an imported built-in variable has no export
2017-10-06 18:33:53 -04:00