Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Substrait] Implement simple extensions. #853

Merged

Conversation

ingomueller-net
Copy link
Collaborator

@ingomueller-net ingomueller-net commented Jul 18, 2024

This is a mechanism of Substrait to define functions and types external
to the core specification. This PR models the declarations as Symbols
and references to them as symbol references in MLIR, which allows to use
MLIR tooling around symbols (such as SymbolTables, iterating over
symbol uses, code navigation via the LSP server, etc.). The PR does not
yet implement a mechanism to use the functions and types, which
subsequent PRs for ScalarFunction and similar are going to do.

While touching the PlanOp-related test cases, this op also merges the
tests of that op, which were previously split into a "simple" and a
"version" file, which probably predates the possibility to tell
mlir-translate (and custom variants) to use output splitters.

@ingomueller-net ingomueller-net force-pushed the substrait-simple-extensions branch 6 times, most recently from df3bd1e to 9b32ee5 Compare July 19, 2024 09:56
@ingomueller-net ingomueller-net changed the title WIP: [Substrait] Implement simple extensions. [Substrait] Implement simple extensions. Jul 19, 2024
@ingomueller-net ingomueller-net requested a review from jpienaar July 19, 2024 09:57
@ingomueller-net ingomueller-net force-pushed the substrait-simple-extensions branch from 9b32ee5 to 8fe00d2 Compare July 19, 2024 10:35
@ingomueller-net ingomueller-net force-pushed the substrait-simple-extensions branch 2 times, most recently from 5f7e841 to 662a78d Compare July 22, 2024 10:21
ingomueller-net added a commit to ingomueller-net/iree-llvm-sandbox that referenced this pull request Jul 22, 2024
This allows to use the externally declared functions from iree-org#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
This is a mechanism of Substrait to define functions and types external
to the core specification. This PR models the declarations as `Symbol`s
and references to them as symbol references in MLIR, which allows to use
MLIR tooling around symbols (such as `SymbolTable`s, iterating over
symbol uses, code navigation via the LSP server, etc.). The PR does not
yet implement a mechanism to use the functions and types, which
subsequent PRs for `ScalarFunction` and similar are going to do.

While touching the `PlanOp`-related test cases, this op also merges the
tests of that op, which were previously split into a "simple" and a
"version" file, which probably predates the possibility to tell
`mlir-translate` (and custom variants) to use output splitters.

Signed-off-by: Ingo Müller <[email protected]>
In the meantime, `main` contains tests that used `func.call` ops to
prevent further pattern application. This breaks with this PR, which
makes `plan`s a `SymbolTable`, which become the closest symbol table of
the `func.call`s, such that those symbols cannot be found anymore. As a
solution, this commit changes the unit tests to use an opaque op from an
unregistered dialect.

Signed-off-by: Ingo Müller <[email protected]>
@ingomueller-net ingomueller-net force-pushed the substrait-simple-extensions branch from 662a78d to 35b8f45 Compare July 31, 2024 08:30
ingomueller-net added a commit to ingomueller-net/iree-llvm-sandbox that referenced this pull request Jul 31, 2024
This allows to use the externally declared functions from iree-org#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
@ingomueller-net ingomueller-net merged commit c5428ad into iree-org:main Jul 31, 2024
3 checks passed
@ingomueller-net ingomueller-net deleted the substrait-simple-extensions branch July 31, 2024 10:12
ingomueller-net added a commit to ingomueller-net/iree-llvm-sandbox that referenced this pull request Jul 31, 2024
This allows to use the externally declared functions from iree-org#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit to ingomueller-net/iree-llvm-sandbox that referenced this pull request Jul 31, 2024
This allows to use the externally declared functions from iree-org#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit that referenced this pull request Jul 31, 2024
This allows to use the externally declared functions from #853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit to substrait-io/substrait-mlir-contrib that referenced this pull request Oct 15, 2024
)

This is a mechanism of Substrait to define functions and types external
to the core specification. This PR models the declarations as `Symbol`s
and references to them as symbol references in MLIR, which allows to use
MLIR tooling around symbols (such as `SymbolTable`s, iterating over
symbol uses, code navigation via the LSP server, etc.). The PR does not
yet implement a mechanism to use the functions and types, which
subsequent PRs for `ScalarFunction` and similar are going to do.

In the meantime, `main` contains tests that used `func.call` ops to
prevent further pattern application. This breaks with this PR, which
makes `plan`s a `SymbolTable`, which become the closest symbol table of
the `func.call`s, such that those symbols cannot be found anymore. As a
solution, this PR also changes the unit tests to use an opaque op from an
unregistered dialect.

While touching the `PlanOp`-related test cases, this op also merges the
tests of that op, which were previously split into a "simple" and a
"version" file, which probably predates the possibility to tell
`mlir-translate` (and custom variants) to use output splitters.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit to substrait-io/substrait-mlir-contrib that referenced this pull request Oct 15, 2024
…g/iree-llvm-sandbox#855)

This allows to use the externally declared functions from iree-org/iree-llvm-sandbox#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit to substrait-io/substrait-mlir-contrib that referenced this pull request Oct 15, 2024
)

This is a mechanism of Substrait to define functions and types external
to the core specification. This PR models the declarations as `Symbol`s
and references to them as symbol references in MLIR, which allows to use
MLIR tooling around symbols (such as `SymbolTable`s, iterating over
symbol uses, code navigation via the LSP server, etc.). The PR does not
yet implement a mechanism to use the functions and types, which
subsequent PRs for `ScalarFunction` and similar are going to do.

In the meantime, `main` contains tests that used `func.call` ops to
prevent further pattern application. This breaks with this PR, which
makes `plan`s a `SymbolTable`, which become the closest symbol table of
the `func.call`s, such that those symbols cannot be found anymore. As a
solution, this PR also changes the unit tests to use an opaque op from an
unregistered dialect.

While touching the `PlanOp`-related test cases, this op also merges the
tests of that op, which were previously split into a "simple" and a
"version" file, which probably predates the possibility to tell
`mlir-translate` (and custom variants) to use output splitters.

Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net added a commit to substrait-io/substrait-mlir-contrib that referenced this pull request Oct 15, 2024
…g/iree-llvm-sandbox#855)

This allows to use the externally declared functions from iree-org/iree-llvm-sandbox#853 as scalar
function calls (which have the semantics of a typical function call).
The current design anticipates that aggregate and window functions can
be modelled with the same op, but future PR will need to show if and how
that is possible.

Signed-off-by: Ingo Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants