When the parser saw more significant hex digits than fit in
the target type, it would compute a nonsensical shift amount, resulting
in undefined behaviour.
Now, drop the excess bits, effectively truncating the significand.
Also guard against overflow of the exponent in the extraordinary (and untested)
case where we see more than, for example, 2**(32-4+1) significant hex digits
for a 32-bit float, or 2**(16-4+1) significant hex digits for a 16-bit
float.
Also guard against overflow of the indexing counting the number of
significant bits. When that would occur silently drop any further
significant bits. (Untested)
Avoid hex floats in C++ code. It's a C++17 feature.
Fixes: #4724
* spirv-as: Avoid overflow when parsing exponents on hex floats
When an exponent is so large that it would overflow the int
type in the parser, saturate the exponent.
This allows extremely large exponents, and saturates
to infinity when the exponent is positive, and zero when the exponent
is negative.
Fixes#4721.
* Avoid unexpected narrowing conversions from arithmetic operations
Co-authored-by: Alastair F. Donaldson <alastair.donaldson@imperial.ac.uk>
This reverts commit d18d0d92e5.
This is reverted because it causes a 7X slowdown when legalizing
SPIR-V with NonSemantic.Shader.DebugInfo.100 instructions.
This is due to the creation of very large UseLists for several
heavily used operands for this extension combined with the fact
that the original commit changed the performance of Uselists to O(n).
* Optimize DefUseManager allocations
Saves around 30-35% of compilation time.
For inst->use_ids, use a pool linked list instead of allocating vectors for every instruction. For inst->uses, use a "PooledLinkedList"' -- a linked list that has shared storage for all nodes. Neither re-use nodes, instead we do a bulk compaction operation when too much memory is being wasted (tuneable).
Includes separate PooledLinkedList templated datastructure, a very special case construct, but split out to make the code a little easier to understand.
Incrementally compute the hash instead of collecting words
Avoids allocating temporary space in a std::vector and std::u32string, and making three passes over all the hashed data.
Switch to using std::vector to prevent processing duplicates instead of std::unordered_set: avoids an allocation/deletion every call to ComputeHashValue, and ends up faster due to much better cache behaviour and smaller constant-factor when searching the (generally very small) list.
In my test case, made Type::HashValue go from 7.5% of compilation time to .5%
C++20 automatically adds reversed versions of operator overloads for
consideration; in this particular instance this results in infinite
recursion, which has now been pointed out elsewhere as a known issue
when migrating to C++20. Here we just disable one of the overloads in
C++20 mode and let the auto-reversing take care of it for us.
* Fix endianness of string literals
To get correct and consistent encoding and decoding of string literals
on big-endian platforms, use spvtools::utils::MakeString and MakeVector
(or wrapper functions) consistently for handling string literals.
- add variant of MakeVector that encodes a string literal into an
existing vector of words
- add variants of MakeString
- add a wrapper spvDecodeLiteralStringOperand in source/
- fix wrapper Operand::AsString to use MakeString (source/opt)
- remove Operand::AsCString as broken and unused
- add a variant of GetOperandAs for string literals (source/val)
... and apply those wrappers throughout the code.
Fixes #149
* Extend round trip test for StringLiterals to flip word order
In the encoding/decoding roundtrip tests for string literals, include
a case that flips byte order in words after encoding and then checks for
successful decoding. That is, on a little-endian host flip to big-endian
byte order and then decode, and vice versa.
* BinaryParseTest.InstructionWithStringOperand: also flip byte order
Test binary parsing of string operands both with the host's and with the
reversed byte order.
Ensures that when an attempt to read a floating-point value from an
input stream fails, the recieving variable for the read does not end up
uninitialised.
Fixes OSS-Fuzz:40432
Fixes#4644
- The binary exponent must have some decimal digits
- A + or - after the binary exponent digits should not be interpreted as
part of the binary exponent.
Fixes: #4500
When we want to set a the value of a HexFloat to inf or nan, we
construct the specific bit pattern in an appropriately sized integer.
That integer is copied to a FloatProxy object through a memcpy. GCC8
complains about the memcpy because it is overwriting a private member of
the class.
The original solution worked well because the template to the HexFloat
could be anything. However, we only used some instantiation of FloatProxy,
which has a construction from that takes its uint_type, so I decided to use
that constructor instead of the memcpy. This puts an extra requirement
on the templace for HexFloat, but it will be fine for us.
Part of #1541.
* 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.
This CL moves the SPIRV_TIMER_ENABLED preprocesser guard to encompass
the includes along with the source. Currently we will try to pull in
sys/resource.h on machines which may not have the file available and the
build will fail. If we don't need timers, then we don't need the
includes as well.
Currently the utils/ folder uses both spvutils:: and spvtools::utils.
This CL changes the namespace to consistenly be spvtools::utils to match
the rest of the codebase.
We replace the std::vector in the Operand class by a new class that does
a small size optimization. This helps improve compile time on Windows.
Tested on three sets of shaders. Trying various values for the small
vector. The optimal value for the operand class was 2. However, for
the Instruction class, using an std::vector was optimal. Size of "0"
means that an std::vector was used.
Instruction size
0 4 8
Operand Size
0 489 544 684
1 593 487
2 469 570
4 473
8 505
This is a single thread run of ~120 shaders. For the multithreaded run
the results were the similar. The basline time was ~62sec. The
optimal configuration was an 2 for the OperandData and an
std::vector for the OperandList with a compile time of ~38sec. Similar
expiriments were done with other sets of shaders. The compile time still
improved, but not as much.
Contributes to https://github.com/KhronosGroup/SPIRV-Tools/issues/1609.
Introduce a pass that does a DCE type analysis for vector elements
instead of the whole vector as a single element.
It will then rewrite instructions that are not used with something else.
For example, an instruction whose value are not used, even though it is
referenced, is replaced with an OpUndef.
The unordered_set in ADCE that holds all of the live instructions takes
a very long time to be destroyed. In some shaders, it takes over 40% of
the time.
If we look at the unique ids of the live instructions, I believe they
are dense enough make a simple bit vector a good choice for to hold that
data. When I check the density of the bit vector for larger shaders, we
are usually using less than 4 bytes per element in the vector, and
almost always less than 16.
So, in this commit, I introduce a simple bit vector class, and
use it in ADCE.
This help improve the compile time for some shaders on windows by the
40% mentioned above.
Contributes to https://github.com/KhronosGroup/SPIRV-Tools/issues/1328.
CPPreference.com has this description of digits10:
“The value of std::numeric_limits<T>::digits10 is the number of
base-10 digits that can be represented by the type T without change,
that is, any number with this many significant decimal digits can be
converted to a value of type T and back to decimal form, without
change due to rounding or overflow.”
This means that any number with this many digits can be represented
accurately in the corresponding type. A change in any digit in a
number after that may or may not cause it a different bitwise
representation. Therefore this isn’t necessarily enough precision to
accurately represent the value in text. Instead we need max_digits10
which has the following description:
“The value of std::numeric_limits<T>::max_digits10 is the number of
base-10 digits that are necessary to uniquely represent all distinct
values of the type T, such as necessary for
serialization/deserialization to text.”
The patch includes a test case in hex_float_test which tries to do a
round-robin conversion of a number that requires more than 6 decimal
places to be accurately represented. This would fail without the
patch.
Sadly this also breaks a bunch of other tests. Some of the tests in
hex_float_test use ldexp and then compare it with a value which is not
the same as the one returned by ldexp but instead is the value rounded
to 6 decimals. Others use values that are not evenly representable as
a binary floating fraction but then happened to generate the same
value when rounded to 6 decimals. Where the actual value didn’t seem
to matter these have been changed with different values that can be
represented as a binary fraction.
This patch adds a new option --time-report to spirv-opt. For each pass
executed by spirv-opt, the flag prints resource utilization for the pass
(CPU time, wall time, RSS and page faults)
This fixes issue #1378
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.
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
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.
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 change will replace a number of the
std::vector<std::unique_ptr<Instruction>> member of the module to
InstructionList. This is for consistency and to make it easier to
delete instructions that are no longer needed.
Markv codec now receives two optional callbacks:
LogConsumer for internal codec logging
DebugConsumer for testing if encoding->decoding produces the original
results.
We want to run the optimization when using -O and -Os, but it was not
added at part of https://github.com/KhronosGroup/SPIRV-Tools/pull/905.
This change will add that a well as some minor formatting changes
requested in that same pull request.
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 commit is the initial implementation of the intrusive linked list
class. It includes the implementation in the header files, and unit
test.
The iterators are circular: incrementing end() gives begin() and
decrementing begin() gives end(). Also made it valid to
decrement end().
Expliticly defines move constructor and move assignment
- Visual Studio 2013 does not implicitly generate the move constructor or
move assignments. So they need to be explicit, otherwise it will try to
use the copy constructor, which we explicitly deleted.
- Can't use "= default" either.
Seems like VS2013 does not support explicitly using the default move
constructors and move assignments, so I wrote them out.
This keeps the previous behavior for other compilers that will
throw warnings on a negative shift operation, but works around
the internal compiler error in GCC.
Includes:
- Multi-sequence move-to-front
- Coding by id descriptor
- Statistical coding of non-id words
- Joint coding of opcode and num_operands
Removed explicit form Huffman codec constructor
- The standard use case for it is to be constructed from initializer list.
Using serialization for Huffman codecs
Bit stream writer was manifesting incorrect behaviour when the following
two conditions were met:
- writer was on 64-bit word boundary
- WriteBits was invoked with num_bits=0 (can happen when a Huffman codec has only one
value)
The bug was causing very rare sporadic corruption which was detected by
tests after a random experimental change in MARK-V model.
Refactored the Huffman codec implementation and added ability to
serialize to C++-like text format. This would reduce the time-complexity
if loading hard-coded codecs.
Add MultiMoveToFront class which supports multiple move-to-front
sequences and allows to promote value in all sequences at once.
Added caching for last accessed sequence handle and last accessed value
in each sequence.
