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

Add a "context" hook for generic calls during codegen #36398

Merged
merged 1 commit into from
Jun 28, 2020
Merged

Conversation

Keno
Copy link
Member

@Keno Keno commented Jun 23, 2020

This adds a new context field that rewrites generic calls like
apply_generic(f, (args...,)) to apply_generic(context, (f, args...))
during codegen. The intention here is to allow external AbstractInterpreters
to provide custom implementations of apply_generic (usually recursing
analysis using the same interpreter, but other behavior may be
desired). This is a bit of a stopgap solution. I think in the fullness
of time, we'll probably want completely custom codegen for generic
callsites, to avoid the potential of a double-dispatch impact, but
for the moment this allows prototyping.

@vtjnash
Copy link
Member

vtjnash commented Jun 23, 2020

This seems likely to break rather often, since it's already only one of many places where the internal system may call into jl_apply_generic (any invoke site may get turned back into a trampoline sometimes, as well as the reverse may happen). It's not bad, but I wonder why you don't do it as a post-processing step. Regardless, since I assume this to be documented as unstable/experimental, that doesn't need to block merging.

It looks like the only thing it requires for merging is to be added to the list of all parameters inside compare_cgparams.

@Keno
Copy link
Member Author

Keno commented Jun 26, 2020

It's not bad, but I wonder why you don't do it as a post-processing step.

I was hoping to be able to avoid relying on LLVM.jl at least for the simple cases. It'll likely be required in full generality, but it's pretty nice to be able to just write the hook in regular Julia without having to do all the low level code generation for prototyping.

This adds a new context field that rewrites generic calls like
`apply_generic(f, (args...,))` to `apply_generic(context, (f, args...))`
during codegen. The intention here is to allow external AbstractInterpreters
to provide custom implementations of apply_generic (usually recursing
analysis using the same interpreter, but other behavior may be
desired). This is a bit of a stopgap solution. I think in the fullness
of time, we'll probably want completely custom codegen for generic
callsites, to avoid the potential of a double-dispatch impact, but
for the moment this allows prototyping.
@vtjnash
Copy link
Member

vtjnash commented Jun 26, 2020

True, I suppose a pre-processing step is more sensible, while semantic meaning is still clearer in the IR?

@Keno
Copy link
Member Author

Keno commented Jun 26, 2020

Right, I don't want to transform it in the Julia IR, because the fact that it's a generic call doesn't matter there. From the julia perspective it's just a regular call - it's about the implementation detail of how that call gets dispatched and for a custom AbstractInterpreter that may require something other than the Julia default - but it's semantically still just a regular abstract call. It does make sense to transform at the LLVM level of course, but this just seems like an easier way to achieve that end.

@Keno Keno merged commit 6185d24 into master Jun 28, 2020
@Keno Keno deleted the kf/hooks4 branch June 28, 2020 22:42
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
This adds a new context field that rewrites generic calls like
`apply_generic(f, (args...,))` to `apply_generic(context, (f, args...))`
during codegen. The intention here is to allow external AbstractInterpreters
to provide custom implementations of apply_generic (usually recursing
analysis using the same interpreter, but other behavior may be
desired). This is a bit of a stopgap solution. I think in the fullness
of time, we'll probably want completely custom codegen for generic
callsites, to avoid the potential of a double-dispatch impact, but
for the moment this allows prototyping.
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