Defining our own function operation allows for the PDL interpreter
to be more self contained, and also removes any dependency on FuncOp;
which is moving out of the Builtin dialect.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td | ||
---|---|---|
666 | Can you document this builder as creating the entry block. | |
672 | I think there was a TODO in the review of the FunctionOpInterface to rename this API from getType to getFunctionType? | |
677–682 | Shouldn't these have a default implementation in the interface trait? | |
687–690 | Isn't this included in the ODS generated verifier? Can you have a test hit this condition? |
mlir/include/mlir/Dialect/PDLInterp/IR/PDLInterpOps.td | ||
---|---|---|
666 | Yeah, given that we always want an entry block. These functions don't have an "external" equivalent, so we always have a non-empty region. Lower down I marked skipDefaultBuilders, so this should be the only builder generated. | |
672 | Do you have a strong preference of doing now or later in the current commit stack? I still need to flip PDL over to use the prefixed accessors. | |
677–682 | No. FunctionOpInterface doesn't assume that you use FunctionType, though we might want to consider a helper trait that stubs everything out for the FunctionType case. | |
687–690 | Good point, removed. This was copy over from FuncOp before I added TypeAttrOf to support providing the concrete type to ODS. | |
mlir/lib/Dialect/PDLInterp/IR/PDLInterp.cpp | ||
190–191 | Thanks for pointing this out, I don't understand why it doesn't. Let me just move this to the interface. |
Can you document this builder as creating the entry block.
Also does this remove the default generated builders? I'm concerned we'd have some builders generating a block but not all of them.
Does it really pulls it weight when the client just has to call funcOp.addEntryBlock(); post creation?