This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Add basic definitions for supporting availability
ClosedPublic

Authored by antiagainst on Dec 27 2019, 7:52 AM.

Details

Summary

SPIR-V has a few mechanisms to control op availability: version,
extension, and capabilities. These mechanisms are considered as
different availability classes.

This commit introduces basic definitions for modelling SPIR-V
availability classes. Specifically, an Availability class is
added to SPIRVBase.td, along with two subclasses: MinVersion
and MaxVersion for versioning. SPV_Op is extended to take a
list of Availability. Each Availability instance carries
information for generating op interfaces for the corresponding
availability class and also the concrete availability
requirements.

With the availability spec on ops, we can now auto-generate the
op interfaces of all SPIR-V availability classes and also
synthesize the op's implementations of these interfaces. The
interface generation is done via new TableGen backends
-gen-avail-interface-{decls|defs}. The op's implementation is
done via -gen-spirv-avail-impls.

Diff Detail

Unit TestsFailed

Event Timeline

antiagainst created this revision.Dec 27 2019, 7:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 27 2019, 7:52 AM

Update commit message

antiagainst retitled this revision from [MLIR][spirv] Add basic definitions for supporting versions to [mlir][spirv] Add basic definitions for supporting availability.Dec 27 2019, 7:56 AM

Unit tests: fail. 61129 tests passed, 1 failed and 728 were skipped.

failed: Clang.SemaOpenCL/numbered-address-space.cl

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

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

Unit tests: fail. 61129 tests passed, 1 failed and 728 were skipped.

failed: Clang.SemaOpenCL/numbered-address-space.cl

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

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

mravishankar requested changes to this revision.Dec 27 2019, 11:54 AM

The SPIR-V part looks fine to me. I have no major comments on the tblgen part. Maybe get some one with more knowledge to review it.

mlir/include/mlir/Dialect/SPIRV/SPIRVBase.td
49

I am assuming these will move into a different file some time. Can we put them in a different td file for separation?

165

nit : neted -> nested

mlir/test/Dialect/SPIRV/TestAvailability.cpp
19

Should this be a module pass? Do we need to look at ops at "module-level" to arrive at capabilities?

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

Is there a reason to not have them in a separate file. The additions are completely independent here right?

This revision now requires changes to proceed.Dec 27 2019, 11:54 AM
antiagainst marked 3 inline comments as done.

Addressed Mahesh's comments

antiagainst marked 4 inline comments as done.Dec 27 2019, 1:09 PM
antiagainst added inline comments.
mlir/include/mlir/Dialect/SPIRV/SPIRVBase.td
49

Yes this will eventually go into OpBase.td after graduation from the SPIR-V dialect. Moved to SPIRVAvailability.td for now.

mlir/test/Dialect/SPIRV/TestAvailability.cpp
19

This pass is meant for testing. Using FunctionPass as the base allows us to put a test case inside a function so it's a nice boundary for tests and FileCheck. (spv.module can also be put in a func.)

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

Wanted to keep all SPIR-V specific TableGen backend in one place. I think this is okay for now.

Unit tests: pass. 61132 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

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

mravishankar accepted this revision.Dec 27 2019, 1:15 PM
mravishankar added inline comments.
mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
1

Ok, wont try to push it. Wouldn't most of these methods to go into TblGen core infra?

This revision is now accepted and ready to land.Dec 27 2019, 1:15 PM
antiagainst marked 4 inline comments as done.Dec 27 2019, 1:24 PM
antiagainst added inline comments.
mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
1

Yup before that they will stay scoped to the SPIR-V dialect. We already have a few SPIR-V specific TableGen backends; there is no clear cut rule as which to put in which file, if we have multiple source files for SPIR-V; so rather to put all of them in one file until it becomes too large to tolerate. :)

This revision was automatically updated to reflect the committed changes.
antiagainst marked an inline comment as done.

What is the relationship between this and the RFC?

What is the relationship between this and the RFC?

This CL mostly follows the RFC in order to allow a try-out-and-then-graduate approach for the op availability feature. This CL only touches SPIR-V dialect related files, so it's not possible to follow exactly the RFC (which requires touching OpBase.td) and I've made deviations like forcing all ops to implement all availability interfaces.