When using PhysicalStorageBuffer it is possible for a function to
return a pointer type. This was not being handled correctly in
`GetLoadedVariablesFromFunctionCall` in the DCE pass because
`IsPtr` returns the wrong result.
Fixes#5270.
* NFC: makes the FeatureManager immutable for users
The FeatureManager contains some internal state, like
a set of capabilities and extensions. Those are derived
from the module.
Before this commit, the FeatureManager exposed Remove* functions
which could unsync the reported extensions/capabilities from
the truth: the module.
The only valid usecase to remove items directly from the FeatureManager
is by the context itself, when an instruction is killed:
instead of running the whole an analysis, we remove the single outdated
item.
The was 2 users who mutated its state:
- one to invalidate the manager. Moved to call a reset function.
- one who removed an extension from the feature manager after removing
it from the module. This logic has been moved to the context, who
now handles the extension removal itself.
Signed-off-by: Nathan Gauër <brioche@google.com>
* clang-format
* add RemoveCapability since the fuzztests are using it
* add tests
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
The iterator class was initialized by setting the offset
and bucket to 0. Big oversight: what if the first enum is
not valid? Then `*iterator->begin()` would return the wrong
value.
Because the first capacity is Matrix, this bug was not visible by
any SPIRV test.
And this specific case wasn't tested correctly in the new enumset tests.
Signed-off-by: Nathan Gauër <brioche@google.com>
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
This avoids errors like this from instrumenting vertex shaders:
error: 165: Expected Constituents to be scalars or vectors of the
same type as Result Type components
%195 = OpCompositeConstruct %v4uint %uint_0 %191 %194 %uint_0
4a1a299b20...cc366710bb
$ git log 4a1a299b2..cc366710b --date=short --no-merges --format='%ad %ae %s'
2023-07-03 steve Use template type FloatType in the cast.
Created with:
roll-dep external/googletest
Co-authored-by: GitHub Actions[bot] <>
This commit adds forward iterator, and renames functions to
it matches the std::unordered_set/std::set better.
This goes against the SPIR-V coding style, but might be better in
the long run, especially when this set is used along real STL
sets.
(Right now, they are not compatible, and requires 2 syntaxes).
This container could in theory handle bidirectional
iterator, but for now, only forward seemed required for
our use-cases.
Signed-off-by: Nathan Gauër <brioche@google.com>
* Use android ndk r25
We currently use R21 of the Android NDK for our tests. There have been
to LTS release since that one, and we do not expect people to use it
anymore. Also, it contains Python 2.7, not Python3. The python scripts
in SPIR-V Tools expect Python 3, so we have to update.
We chose the latest LTS release.
* Roll external/googletest/ be03d00f5..4a1a299b2 (1 commit)
be03d00f5f...4a1a299b20
$ git log be03d00f5..4a1a299b2 --date=short --no-merges --format='%ad %ae %s'
2023-07-07 absl-team Update docstring of PrintWithFallback(..) to reflect the recently changed ordering.
Created with:
roll-dep external/googletest
* Roll external/re2/ 1c1ffbe3c..a57a1d646 (2 commits)
1c1ffbe3c6...a57a1d6462
$ git log 1c1ffbe3c..a57a1d646 --date=short --no-merges --format='%ad %ae %s'
2023-07-06 junyer Stop using `std::map<std::string, Prefilter*>`.
2023-07-06 junyer Bump the CMake baseline to 3.13.
Created with:
roll-dep external/re2
* Roll external/spirv-headers/ 3469b164e..d0006a393 (3 commits)
3469b164e2...d0006a3938
$ git log 3469b164e..d0006a393 --date=short --no-merges --format='%ad %ae %s'
2023-07-05 lynix680 Regenerate headers
2023-06-30 lynix680 Add NZSL as a source language
2023-06-30 lynix680 Add NZSLc as a generator
Created with:
roll-dep external/spirv-headers
* Treat spir-v.xml as utf-8
Treat the input file as utf-8, and configure the XML parser to use
the utf-8 encoding.
The Chromium build was breaking when trying to parse the XML
file as ASCII.
* Use Python io.open
Try to fix the android-ndk-build flow. It seems to be using
an old Python. For example, Python 2.6 builtin function open()
did not support the 'encoding' keyword argument. But its io.open
method did support it.
The current EnumSet implementation is only efficient for enums with
values < than 64. The reason is the first 63 values are stored as a
bitmask in a 64 bit unsigned integer, and the other values are stored
in a std::set.
For small enums, this is fine (most SPIR-V enums have IDs < than 64),
but performance starts to drop with larger enums (Capabilities,
opcodes).
Design considerations:
----------------------
This PR changes the internal behavior of the EnumSet to handle enums
with arbitrary values while staying performant.
The idea is to extend the 64-bits buckets sparsely:
- each bucket can store 64 value, starting from a multiplier of 64.
This could be considered as a hashset with linear probing.
- For small enums, there is a slight memory overhead due to the bucket
storage, but lookup is still constant.
- For linearly distributed values, lookup is constant.
- Worse case for storage are for enums with values which are multiples of 64.
But lookup is constant.
- Worse case for lookup are enums with a lot of small ranges scattered in
the space (requires linear probing).
For enums like capabilities/opcodes, this bucketing is useful as values
are usually scatters in distinct, but almost contiguous blocks.
(vendors usually have allocated ranges, like [5000;5500], while [1000;5000]
is mostly unused).
Benchmarking:
-------------
Benchmarking was done in 2 ways:
- a benchmark built for the occasion, which only measure the EnumSet
performance.
- SPIRV-Tools tests, to measure a more realist scenario.
Running SPIR-V tests with both implementations shows the same
performance (delta < noise). So seems like we have no regressions.
This method is noisy by nature (I/O, etc), but the most representative
of a real-life scenario.
Protocol:
- run spirv-tests with no stdout using perf, multiple times.
Result:
- measure noise is larger than the observed difference.
The custom benchmark was testing EnumSet interfaces using SPIRV enums.
Doing thousand of insertion/deletion/lookup, with 2 kind of scenarios:
- add once, lookup many times.
- add/delete/loopkup many time.
For small enums, results are similar (delta < noise). Seems relevant
with the previously observed results as most SPIRV enums are small, and
SPIRV-Tools is not doing that many intensive operations on EnumSets.
Performance on large enums (opcode/capabilities) shows an improvement:
+-----------------------------+---------+---------+---------+
| Metric | Old | New | Delta % |
+-----------------------------+---------+---------+---------+
| Execution time | 27s | 7s | -72% |
| Instruction count | 174b | 129b | -25% |
| Branch count | 28b | 33b | +17% |
| Branch miss | 490m | 26m | -94% |
| Cache-misses | 149k | 26k | -82% |
+-----------------------------+---------+---------+---------+
Future work
-----------
This was by-design an NFC change to compare apples-to-apples.
The next PR aims to add STL-like iterators to the EnumSet to allow
using it with STL algorithms, and range-based for loops.
Signed-off-by: Nathan Gauër <brioche@google.com>
* Roll external/googletest/ f269e15c5..687c58994 (2 commits)
f269e15c5c...687c589949
$ git log f269e15c5..687c58994 --date=short --no-merges --format='%ad %ae %s'
2023-06-28 absl-team Print stack traces on SEH exceptions on Windows
2023-06-27 dmauro On platforms without a file system, don't log an error when no alternative output format is requested.
Created with:
roll-dep external/googletest
* Roll external/re2/ 9ea3effad..231c11764 (1 commit)
9ea3effadf...231c117648
$ git log 9ea3effad..231c11764 --date=short --no-merges --format='%ad %ae %s'
2023-06-28 junyer Print command lines for build commands.
Created with:
roll-dep external/re2
---------
Co-authored-by: GitHub Actions[bot] <>
* Validate layouts for PhysicalStorageBuffer pointers
Fixes#5282
* These pointers may not orginate from a variable so standard layout
validation misses them
* Now checks every instructions that results in a physical storage
buffer pointer
* May not start from a Block-decorated struct so that part is fudged
with a valid layout
* formatting
* SPV_KHR_cooperative_matrix
* Update DEPS with headers
* Update according to review recommendations
* Bugfix and formatting
* Formatting missed or damaged by VS2022
We currently add the abseil in the external/CMakeLists.txt. However, it
is not needed by spirv-tools directly. Instead we set effcee's variable
with the abseil source directory, and let effcee add it.
This will mean that abseil will be checked out only if effcee is
used. We currently get a few reports from people that use to only checkout
spirv-headers, and now get errors because abseil is missing.
Simplify what we add to user code by moving most of it into a function
that checks both that the descriptor index is in bounds and the
initialization state. Move error logging into this function as
well.
Remove many options to turn off parts of the instrumentation,
because there were far too many permutations to keep working and
test properly.
Combine Buffer and TexBuffer error checking. This requires that VVL
set the length of TexBuffers in the descriptor input state, rather
than relying on the instrumentation code to call OpImageQuerySize.
Since the error log includes the descriptor set and binding numbers
we can use a single OOB error code rather than having 4 per-type
error codes.
Since the error codes are getting renumbered, make them start at 1
rather than 0 so it is easier to determine if the error code was
actually set by the instrumentation.
From the 3.27 release notes:
The FindPythonInterp and FindPythonLibs modules, which have been
deprecated since CMake 3.12, have been removed by policy CMP0148.
Port projects to FindPython3, FindPython2, or FindPython.
closes#4145
The length of a uvec3 was assumed to be 16 bytes, but it is 12.
Sometimes the stride might be 16 bytes though, which is probably the
source of the confusion.
Redo structure length to be the offset + length of the last member.
Add tests to cover arrays of uvec3s and uvec3 struct members.
Fixes https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5691
