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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In D74226#1865003 <https://reviews.llvm.org/D74226#1865003>, @rriddle wrote:

> I would actually prefer that we call this FuncOp('func'). It is consistent with all of the other functions that have been defined, and seems a little weird to deviate.


+1



================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVStructureOps.td:277
+    ///    parameters they won't have any attributes.
+    void setType(FunctionType newType);
+
----------------
The 4 methods above all seem like they could be added to FunctionLike trait?


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVStructureOps.td:287
+    /// Returns the number of results. Hook for OpTrait::FunctionLike.
+    unsigned getNumFuncResults() { return getType().getResults().size(); }
+
----------------
getType().getNumResults(); is shorter?


================
Comment at: mlir/lib/Dialect/SPIRV/SPIRVOps.cpp:1745
+  if (getType().getNumResults() > 1)
+    return emitOpError("cannot have more than one result");
+
----------------
Seems like this should be checked in `verifyType()`?

I'd expect verifyBody to actually find the return ops and check for the matching, as well as other possible SPIRV specific properties.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74226