* opt: fix StorageInputOutput16 trimming.
While integrating this pass into DXC, I found a lot of missing
cases. This PR fixes a few issues centered around this capability
while laying out fondations for more fixes.
1. The grammar can define extensions in operand & opcode tables.
- opcode can rely on common capabilities, but require a new
extension.
- opcode can also rely on a capability which requires an extension.
Sometimes, the extension is listed twice, in the opcode, and
capability. But this redundancy is not guaranteed.
2. minVersion check. The condition was flipped: we added the extension
when the minVersion was less than current.
Didn't noticed the issue as I only tests on the default env.
3. Capability/Extension instructions were not ignored.
- `OpCapability Foo` will require the `Foo` capability.
- it doesn't mean the module requires the `Foo` capability.
Same for extensions.
This commit adds disabled tests, for fixes which are too large to
be brought into this already large PR.
* Roll external/googletest/ 6f6ab4212..46db91ef6 (9 commits)
6f6ab4212a...46db91ef6f
$ git log 6f6ab4212..46db91ef6 --date=short --no-merges --format='%ad %ae %s'
2023-08-07 dinor Make references to `#include`s consistent across docs
2023-08-02 robert.shade Avoid unreachable code warning
2023-08-02 dmauro Update documentation to refer to v1.14
2023-08-02 dmauro Bump version to v1.14 in preparation for release
2023-08-02 dmauro Remove the GTEST_HAS_DOWNCAST_ customization point.
2023-08-02 dmauro Add googletest-message-test to the Bazel tests It appears to have been unintentionally left out
2023-08-01 phoebeliang Make testing::Message support streamed AbslStringify values
2023-08-01 dmauro Update GoogleTest dependencies
2023-07-27 patryk gtest: Supress warning about set unused variable
Created with:
roll-dep external/googletest
* Roll external/re2/ 960c86176..9dc7ae7b5 (1 commit)
960c861764...9dc7ae7b52
$ git log 960c86176..9dc7ae7b5 --date=short --no-merges --format='%ad %ae %s'
2023-08-04 junyer Minor Bazel cleanups.
Created with:
roll-dep external/re2
---------
Co-authored-by: GitHub Actions[bot] <>
The autoroll workflow is currently failing due to being unable to find
some depot_tools executables. This is due to a limitation in Go os/exec
which effectively rejects all relative paths in PATH, and is exposed by
a recent update to depot_tools
(https://crrev.com/43083529de5802a83f53f1d53d7f5f9615999996).
Multiple calls to this function were causing vkCreateGraphicsPipelines
to be 3x slower on some driver. I suspect this was because each
call had to be inlined separately which bloated the code and caused
more work in the driver's SPIRV -> native instruction compilation.
* Roll external/googletest/ 40412d851..6f6ab4212 (2 commits)
40412d8512...6f6ab4212a
$ git log 40412d851..6f6ab4212 --date=short --no-merges --format='%ad %ae %s'
2023-07-27 julien.combattelli Use #if and not #ifdef to check filesystem support
2023-07-28 absl-team Adjust includes to use <> instead of "", consistent with quickstart pages.
Created with:
roll-dep external/googletest
* Roll external/re2/ e66463312..960c86176 (10 commits)
e66463312e...960c861764
$ git log e66463312..960c86176 --date=short --no-merges --format='%ad %ae %s'
2023-07-28 junyer Don't try to support ARM64 on Windows yet.
2023-07-28 junyer Try again to make cross-compiling on Windows work.
2023-07-28 junyer `bazelbuild/setup-bazelisk` doesn't work for some reason.
2023-07-28 junyer Bazelisk isn't installed with Chocolatey, apparently.
2023-07-28 junyer Avoid the Chocolatey install of Bazel(isk) getting in the way.
2023-07-28 junyer Try using `bazelbuild/setup-bazelisk` everywhere.
2023-07-28 junyer Tell the Python build where Bazelisk is.
2023-07-28 junyer Explicitly invoke Bazelisk rather than Bazel.
2023-07-28 junyer Avoid `Conflicts: python3-lldb-x.y` between packages.
2023-07-28 junyer Prepare to release `google-re2` 1.1.
Created with:
roll-dep external/re2
* Roll external/spirv-headers/ 51b106461..ae89923fa (1 commit)
51b1064617...ae89923fa7
$ git log 51b106461..ae89923fa --date=short --no-merges --format='%ad %ae %s'
2023-07-26 kevin.petit Add KHR suffix to Cooperative Matrix Operands
Created with:
roll-dep external/spirv-headers
---------
Co-authored-by: GitHub Actions[bot] <>
This commit adds a new optimization which tries to remove unnecessary
capabilities from a SPIR-V module.
When compiling a SPIR-V module, you may have some dead-code using
features gated by a capability.
DCE will remove this code, but the capability will remain. This means
your module would still require some capability, even if it doesn't
require it. Calling this pass on your module would remove obsolete
capabilities.
This pass wouldn't be enabled by default, and would only be usable
from the API (at least for now).
NOTE: this commit only adds the basic skeleton/structure, and
doesn't mark as supported many capabilities it could support.
I'll add them as supported as I write tests.
Signed-off-by: Nathan Gauër <brioche@google.com>
The code currently tries to get the value of the floating point constant
to see if it is -0.0. However, we are not able to get the value for
16-bit floating point value, and we hit an assert.
To avoid this, we add an early check for the width to make sure it is
either 32 or 64.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5413.
Expanding a bit the EnumSet API to have iterator-based
insert and constructors (like the STL).
This is also a pre-requisite from the capability-trimming pass as
it allows to build a const set from a constexpr std::array easily.
Signed-off-by: Nathan Gauër <brioche@google.com>
GetCapabilities returned a const*, and GetExtensions did not exist.
This commit adds GetExtensions, and changes the return value to
be a const&.
This commit also removes the overload to GetCapabilities which returns
a mutable set, as it is unused.
Signed-off-by: Nathan Gauër <brioche@google.com>
Work around a problem in CMake 3.22.1 in setting -std=c++17.
Instead -std=c++11 is in effect for targets in test/*, but
those targets require C++17.
Fixes: #5340
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] <>
