[PATCH] D72336: [mlir] Create a gpu.module operation for the GPU Dialect.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/GPU/IR/GPUDialect.cpp:822
+  OperationState state(loc, getOperationName());
+  Builder builder(loc->getContext());
+  GpuModuleOp::build(&builder, state, name);
----------------
tpopp wrote:
> herhut wrote:
> > rriddle wrote:
> > > Why is this method necessary?
> > It is the equivalent of ModuleOp::create and FuncOp::create to allow creation of modules outside the context of a surrounding module. We could refactor the code here to use a builder, as the module is inserted into a surrounding module but in general that might not be the case. Or is there a way to have create methods generated?
> We need GPUModuleOp::build because we have to ensure a terminator, we can't use a generated create method as I understand it.  The create method is to be consistent with similar ops like those that Stephan referenced.
ModuleOp has a create because it is the top level operation(also partially legacy), FuncOp only has one because it was a part of the transition from separate class to operation that was never removed. Realistically, I don't see why GPUModuleOp needs one given that it is not any more difficult for a user to just have:

OpBuilder builder(context);
builder.create<GPUModuleOp>(...);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72336