Page MenuHomePhabricator

[mlir][spirv] Support implied extensions and capabilities
ClosedPublic

Authored by antiagainst on Jan 15 2020, 6:13 AM.

Details

Summary

In SPIR-V, when a new version is introduced, it is possible some
existing extensions will be incorporated into it so that it becomes
implicitly declared if targeting the new version. This affects
conversion target specification because we need to take this into
account when allowing what extensions to use.

For a capability, it may also implies some other capabilities,
for example, the Shader capability implies Matrix the capability.
This should also be taken into consideration when preparing the
conversion target: when we specify an capability is allowed, all
its recursively implied capabilities are also allowed.

This commit adds utility functions to query implied extensions for
a given version and implied capabilities for a given capability
and updated SPIRVConversionTarget to use them.

This commit also fixes a bug in availability spec. When a symbol
(op or enum case) can be enabled by an extension, we should drop
it's minimal version requirement. Being enabled by an extension
naturally means the symbol can be used by *any* SPIR-V version
as long as the extension is supported. The grammar still encodes
the 'version' field for such cases, but it should be interpreted
as a different way: rather than meaning a minimal version
requirement, it says the symbol becomes core at that specific
version.

Diff Detail

Event Timeline

antiagainst created this revision.Jan 15 2020, 6:13 AM

Unit tests: pass. 61884 tests passed, 0 failed and 782 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle added inline comments.Jan 15 2020, 9:09 AM
mlir/lib/Dialect/SPIRV/SPIRVTypes.cpp
90

I would imagine this would be much simpler/efficient if you used a SetVector instead.

mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
1307

This seems unnecessarily inefficient. Can you just split the format below and use interleaveComma over the defs instead.

benvanik accepted this revision.Jan 15 2020, 9:11 AM
benvanik added inline comments.
mlir/include/mlir/Dialect/SPIRV/SPIRVTypes.h
53

nit: wouldn't this just be std::vector?

This revision is now accepted and ready to land.Jan 15 2020, 9:11 AM
antiagainst marked 6 inline comments as done.

Address comments

antiagainst added inline comments.Jan 16 2020, 6:50 PM
mlir/include/mlir/Dialect/SPIRV/SPIRVTypes.h
53

According to LLVM programmer's manual, SmallVector<..., 0> has advantages over std::vector: http://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h

mlir/lib/Dialect/SPIRV/SPIRVTypes.cpp
90

Indeed. Thanks for the suggestion! :)

mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
1307

Good point! Done.

Unit tests: pass. 61936 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

This revision was automatically updated to reflect the committed changes.