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 | ||
---|---|---|
209 | 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 ? | |
268 | Is there an assertion we can use ? | |
274 | Thanks! | |
281 | nit: is there a lookupOrNull in this API to avoid 2 calls ? |
Nice thanks
mlir/lib/Tools/PDLL/CodeGen/MLIRGen.cpp | ||
---|---|---|
46 | It would be nice if we had a range Location :) | |
58 | Sort? | |
72 | Comment ? | |
112 | s/mlir/MLIR/ | |
290 | Seems this is only place loc is used, could we compute it here only? | |
372 | 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 | ||
---|---|---|
372 | 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 | ||
---|---|---|
372 | +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 | ||
---|---|---|
268 | 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. | |
281 | Thanks, I kept looking for a "find" but didn't notice that it's called "begin". | |
372 | 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 :)