[PATCH] D71930: [mlir][spirv] Add basic definitions for supporting availability

[Lei Zhang via Phabricator via llvm-commits]

antiagainst marked 4 inline comments as done.
antiagainst added inline comments.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVBase.td:49
+//===----------------------------------------------------------------------===//
+// Op availability definitions
+//===----------------------------------------------------------------------===//
----------------
mravishankar wrote:
> I am assuming these will move into a different file some time. Can we put them in a different td file for separation?
Yes this will eventually go into OpBase.td after graduation from the SPIR-V dialect. Moved to SPIRVAvailability.td for now.


================
Comment at: mlir/test/Dialect/SPIRV/TestAvailability.cpp:19
+struct TestAvailability : public FunctionPass<TestAvailability> {
+  void runOnFunction() override;
+};
----------------
mravishankar wrote:
> Should this be a module pass? Do we need to look at ops at "module-level" to arrive at capabilities?
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.) 


================
Comment at: mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp:1
 //===- SPIRVSerializationGen.cpp - SPIR-V serialization utility generator -===//
 //
----------------
mravishankar wrote:
> Is there a reason to not have them in a separate file. The additions are completely independent here right?
Wanted to keep all SPIR-V specific TableGen backend in one place. I think this is okay for now. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71930/new/

https://reviews.llvm.org/D71930