MLIR: support CALL of registered procedures end to end - #759
Conversation
40cd558 to
1a12967
Compare
| void Procedure::setIndices(bool indices) { | ||
| _indices = indices; | ||
| } | ||
|
|
There was a problem hiding this comment.
I do this slightly differently in #754 , we should compare and see which pattern we want
| // Collect those columns. One bound in another block is skipped: an op here can only | ||
| // take what this block binds. | ||
| void collectLiveColumns(LiveColumns& live); |
There was a problem hiding this comment.
What are "those" columns?
| // A chunk holds the rows of the loop whose body binds it, so only a column bound in this | ||
| // block is row-aligned with the rows flowing past this point. One bound in an enclosing | ||
| // block - or in a loop the dataflow has already left - holds a different row set, and an | ||
| // op that consumes the whole row set (a filter, a call's carry set) would pair its rows | ||
| // with unrelated ones. Note this is stricter than dominance on purpose: an outer block's | ||
| // value does dominate here, it is just the wrong rows. | ||
| const auto isLive = [&](const mlir::Value column) { |
There was a problem hiding this comment.
If I understand correctly this comment can be simplified to: "a column is live if it is defined in the current block"
| func.func @main() { | ||
| %call = nl.procedure("db.labels") yields ["id", "label"] | ||
| %rows = nl.procedure_init(%call, (), {}) : (!nl.procedure_state) -> !nl.iter<!nl.chunk<!storage.label_id>, !nl.chunk<!storage.string>> | ||
| nl.for %ids, %labels in %rows : !nl.iter<!nl.chunk<!storage.label_id>, !nl.chunk<!storage.string>> { | ||
| nl.output(%ids, %labels) : !nl.chunk<!storage.label_id>, !nl.chunk<!storage.string> | ||
| } | ||
| return | ||
| } | ||
| )mlir"; |
There was a problem hiding this comment.
Can we have some example NL and DB programs as samples in samples/mlir?
| // even when the projection returns none of their columns, or the query would lose | ||
| // their cardinality. | ||
| if (!live._columns.empty() && inputs.empty()) { | ||
| generateCrossedCall(procedureName, yieldedNames, yieldedVariables, live); |
There was a problem hiding this comment.
Do we want separate logic for cross product of a MATCH and a CALL? Ideally I would want centralised cross product logic to join islands regardless of whether they are from a MATCH or a CALL. I think it would involve maintaining CALLs in the connected components (we already tracked connected components in generateTraversal
There was a problem hiding this comment.
Yes it would be a better shape to have one unified procedure for the cross product.
But today in this PR, the cross product is decided before CALL statements are processed, in generateTraversal. So this is where it would require integration of the CALLs with the VDG to use the same logic based on components identification in the VDG. For now we do codegen of calls afterwards to minimise changes to the VDG until we can come with a better approach.
| // A standalone CALL has no projection of its own: what it emits is the columns it | ||
| // yielded, in the order the procedure declares them. A call yielding none - a | ||
| // procedure declaring no return value - emits nothing at all, so the query is the | ||
| // drive and no output op is generated. |
| const llvm::StringRef name = mlir::cast<mlir::StringAttr>(yield).getValue(); | ||
| const size_t returnIndex = procedure->getReturnValueIndex(std::string_view(name.data(), name.size())); | ||
|
|
||
| chunkTypes.push_back(procedureChunkType(_builder, procedure->getReturnValueType(returnIndex))); |
There was a problem hiding this comment.
I think this could benefit from being split onto a few lines
79c2d3e to
2394f4f
Compare
7b92414 to
0090189
Compare
ff441cf to
2459549
Compare
Runs a Cypher CALL of a registered procedure through the MLIR engine, from the frontend down to the interpreter.
A procedure that declares a row-aligned argument now has to report the input row of the rows it emits: registration refuses one that does not, and an argument read once per call rather than per row is declared as constant.
Works today:
Added a test for GNN-like sampling:
Added a test for a call crossing and a call joining a hop chain: