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.
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
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.
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
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
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.
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
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.
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
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
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.
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.
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')
This class moves some of the CFG-related functionality into a new
class opt::CFG. There is some other code related to the CFG in the
inliner and in opt::LocalSingleStoreElimPass that should also be moved,
but that require more changes than this pure restructuring.
I will move those bits in a follow-up PR.
Currently, the CFG is computed every time a pass is instantiated, but
this should be later moved to the new IRContext class that @s-perron is
working on.
Other re-factoring:
- Add BasicBlock::ContinueBlockIdIfAny. Re-factored out of MergeBlockIdIfAny
- Rewrite IsLoopHeader in terms of GetLoopMergeInst.
- Run clang-format on some files.
This is the first part of adding the IRContext. This class is meant to
hold the extra data that is build on top of the module that it
owns.
The first part will simply create the IRContext class and get it passed
to the passes in place of the module. For now it does not have any
functionality of its own, but it acts more as a wrapper for the module.
The functions that I added to the IRContext are those that either
traverse the headers or add to them. I did this because we may decide
to have other ways of dealing with these sections (for example adding a
type pool, or use the decoration manager).
I also added the function that add to the header because the IRContext
needs to know when an instruction is added to update other data
structures appropriately.
Note that there is still lots of work that needs to be done. There are
still many places that change the module, and do not inform the context.
That will be the next step.
Including a re-factor of common behaviour into class Pass:
The following functions are now in class Pass:
- IsLoopHeader.
- ComputeStructuredOrder
- ComputeStructuredSuccessors (annoyingly, I could not re-factor all
instances of this function, the copy in common_uniform_elim_pass.cpp
is slightly different and fails with the common implementation).
- GetPointeeTypeId
- TakeNextId
- FinalizeNextId
- MergeBlockIdIfAny
This is a NFC (non-functional change)
This is the first step in replacing the std::vector of Instruction
pointers to using and intrusive linked list.
To this end, we created the InstructionList class. It inherites from
the IntrusiveList class, but add the extra concept of ownership. An
InstructionList owns the instruction that are in it. This is to be
consistent with the current ownership rules where the vector owns the
instruction that are in it.
The other larger change is that the inst_ member of the BasicBlock class
was changed to using the InstructionList class.
Added test for the InsertBefore functions, and making sure that the
InstructionList destructor will delete the elements that it contains.
I've also add extra comments to explain ownership a little better.
This adapts the fix for the single-block loop. Split the loop like
before. But when we move the OpLoopMerge back to the loop header,
redirect the continue target only when the original loop was a single
block loop.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/800
If the caller block is a single-block loop and inlining will
replace the caller block by several blocks, then:
- The original OpLoopMerge instruction will end up in the *last*
such block. That's the wrong place to put it.
- Move it back to the end of the first block.
- Update its Continue Target ID to point to the last block
We also have to take care of cases where the inlined code
begins with a structured header block. In this case
we need to ensure the restored OpLoopMerge does not appear
in the same block as the merge instruction from the callee's
first block.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/787
