This commits starts to plumb PDLL down into MLIR and adds an initial
PDL generator. After this commit, we will have conceptually support
end-to-end execution of PDLL. Followups will add CPP generation to
match the current DRR setup, and begin to add various end-to-end
tests to test PDLL execution.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice, looking forward to using this!
I'd still be interested in seeing PDLL grammar + doc materialized somewhere, this would have helped review some of the mutual recursions.
Still, the tests are nice and help infer some of this back into working memory.
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
210 | Here and below. I find this insertion point change across function boundaries to almost always be a source of pain. OpBuilder::InsertionGuard g(builder); checkAndNestUnderRewriteOp(builder, rootExpr, loc); here and everywhere ? | |
269 | Is there an assertion we can use ? | |
275 | Thanks! | |
282 | nit: is there a lookupOrNull in this API to avoid 2 calls ? |
Nice thanks
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
47 | It would be nice if we had a range Location :) | |
59 | Sort? | |
73 | Comment ? | |
113 | s/mlir/MLIR/ | |
291 | Seems this is only place loc is used, could we compute it here only? | |
373 | Given this is an offline generator, I'd rather this produce an error in non-debug builds too as very few people actually build tools with assertions on. (unless no user input can ever get here/would have been checked before here). Same for other asserts. |
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
373 | This is supposed to be handled by the frontend though, the verification of the AST and diagnostic hopefully happens there and then the assert is just checking this contract. |
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
373 | +1 to defensive programming; if it ever crashes, the frontend should be fixed with a proper error message. |
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
269 | I opted for just a FIXME as I have a patch in the stack that handles the PDL plumbing, and dropping it won't cause anything to mis-compile. | |
282 | Thanks, I kept looking for a "find" but didn't notice that it's called "begin". | |
373 | Exactly, this is codifying an expectation that should be verified by the frontend. If we hit a crash here, it's an indication of a bug that needs to be fixed. We shouldn't be surfacing errors from this point, the constructs should have been verified above this layer. |
It would be nice if we had a range Location :)