[PATCH] D74226: [mlir][spirv] Introduce spv.function

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVOps.h:48
+
+// spirv::Function ops hash just like pointers.
+template <> struct DenseMapInfo<mlir::spirv::FuncOp> {
----------------
///


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVOps.h:49
+// spirv::Function ops hash just like pointers.
+template <> struct DenseMapInfo<mlir::spirv::FuncOp> {
+  static mlir::spirv::FuncOp getEmptyKey() {
----------------
This needs to be formatted.


================
Comment at: mlir/include/mlir/IR/FunctionSupport.h:141
+  /// Adds an entry block to an empty function, and set up the block arguments
+  /// to match the signature of the function. The newly inserted entry block
+  /// is returned.
----------------
None of these are valid if the type of the function isn't the builtin `FunctionType`, i.e. LLVMFuncOp doesn't work with any of these methods.


================
Comment at: mlir/lib/Dialect/SPIRV/SPIRVOps.cpp:1741
+      if (fnType.getNumResults() != 0) {
+        retOp.emitOpError("cannot be used in functions returning value");
+        return WalkResult::interrupt();
----------------
You can return the errors directly. `failure` implicitly converts to WalkResult::interrupt. You just need to set the return type on the lambda to WalkResult.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74226