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

allow CodeInfo to be returned directly from generated function generator #22440

Merged
merged 3 commits into from
Aug 2, 2017

Conversation

jrevels
Copy link
Member

@jrevels jrevels commented Jun 19, 2017

My pitiable attempt at resolving #21146. I'm not a C programmer by any means, so I've probably done something stupid/silly.

I would appreciate any direction here, since if I can get it working, this would be quite useful for tracing generic Julia calls via dispatch-interceptable function wrappers.

@kshyatt kshyatt added the compiler:codegen Generation of LLVM IR and native code label Jun 21, 2017
@jrevels
Copy link
Member Author

jrevels commented Jul 5, 2017

What work items are left for this PR (besides tests)? I know there needs to be some sort of validation pass over the emitted CodeInfo, but I'm not sure what the pass should be checking for exactly. The only guarantees we normally ascribe to Expr generated bodies are that they're locally syntactically valid, yeah?

@StefanKarpinski
Copy link
Sponsor Member

@JeffBezanson, any thoughts on this? Even if just uncertainty, would be good to know...

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 19, 2017

This is legal for inference to do, so that basically makes it certain that generated functions are allowed to do it. We haven't done it before mostly because creating a valid CodeInfo object is fairly difficult (it's easy to trick yourself into thinking some transform is valid which is not).

@JeffBezanson
Copy link
Sponsor Member

JeffBezanson commented Jul 19, 2017

Agreed, the concept is legit.

@StefanKarpinski
Copy link
Sponsor Member

Does that mean we should merge this?

@jrevels jrevels added needs tests Unit tests are required for this change and removed needs tests Unit tests are required for this change labels Jul 20, 2017
@jrevels
Copy link
Member Author

jrevels commented Jul 20, 2017

Added a fairly simple test for this. If anybody has other test ideas, let me know and I can take a crack at implementing them.

I'm planning on implementing a (naive) IR validator in a separate PR, which would make this feature somewhat more robust/error-prone. I'm fine with the IR validator being either a prerequisite or follow-up PR to this PR; otherwise, this PR is ready to merge if CI comes up green.

@jrevels jrevels changed the title [WIP/RFC] allow CodeInfo to be returned directly from generated function generator allow CodeInfo to be returned directly from generated function generator Jul 27, 2017
@jrevels jrevels force-pushed the jr/cinfofromgenerated branch 2 times, most recently from 2159281 to 5d8929e Compare July 27, 2017 19:09
@jrevels
Copy link
Member Author

jrevels commented Jul 31, 2017

Given that the only issue here seems to be a lack of CodeInfo validation, my plan is to merge this as soon as #22938 is merged (assuming nobody expresses dissent).

@JeffBezanson
Copy link
Sponsor Member

I'm ok with merging in either order.

@jrevels
Copy link
Member Author

jrevels commented Aug 2, 2017

I'll merge this now, then, since we've decided in #22938 the code validator won't be enabled by default anyway.

@jrevels jrevels merged commit 898d650 into master Aug 2, 2017
@jrevels jrevels deleted the jr/cinfofromgenerated branch August 2, 2017 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants