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

Fix higher order codegen #2161

Merged
merged 7 commits into from
Dec 7, 2024
Merged

Fix higher order codegen #2161

merged 7 commits into from
Dec 7, 2024

Conversation

wsmoses
Copy link
Member

@wsmoses wsmoses commented Dec 3, 2024

No description provided.

@wsmoses wsmoses requested a review from vchuravy December 3, 2024 14:22
@vchuravy
Copy link
Member

vchuravy commented Dec 3, 2024

What's the GPU story here?

@wsmoses
Copy link
Member Author

wsmoses commented Dec 3, 2024

So the follow up there is basically to also have a similar handler that looks for deferred_(id) and we'll just run gpucompiler codegen there and inline the module similarly to here where we do the mapping variant from the pointer

Copy link
Contributor

github-actions bot commented Dec 3, 2024

Benchmark Results

main d629c30... main/d629c3057119ec...
basics/overhead 4.33 ± 0.01 ns 4.34 ± 0.01 ns 0.998
time_to_load 1.14 ± 0.0098 s 1.2 ± 0.043 s 0.952

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@vchuravy
Copy link
Member

vchuravy commented Dec 3, 2024

I want to remove the deferred_id buisness from GPUCompiler so that we can actually precompile GPU kernels,
and I am not keen on maintaining two code-paths.

@wsmoses
Copy link
Member Author

wsmoses commented Dec 3, 2024

Hm how is pre compilation blocked?

but in any case yeah this PR primarily just fixes the “Enzyme.autodiff is the outermost compiler job” context

@wsmoses
Copy link
Member Author

wsmoses commented Dec 3, 2024

More thought/design is merited for when that is not the case

@vchuravy
Copy link
Member

vchuravy commented Dec 3, 2024

So this PR is the proposed new API for deferred compilation in GPUCompiler JuliaGPU/GPUCompiler.jl#582 this is without any changes for Enzyme.

The rest of the stack is one possible way to add the necessary bits and bobs to make it Enzyme compatible.
Generally speaking, the rule for precompilation is "you shan't depend on runtime data" so pointers or incrementing counters are out.

@vchuravy
Copy link
Member

vchuravy commented Dec 3, 2024

Hm how is pre compilation blocked?

We will serialize a Julia CodeInfo that contains a magical pointer/id? That is then used for a dictionary that only exists during the runtime of a particular code.

@wsmoses
Copy link
Member Author

wsmoses commented Dec 3, 2024

hm yeah fair enough.

In any case even if not a final resolution, would you be opposed to merging?

This net deletes code (including getting rid of a lot of runtime pointers) and removes the autodiff_deferred upgrade in absint, and thus fixes issues like #2147

@wsmoses
Copy link
Member Author

wsmoses commented Dec 3, 2024

[to be clear we would still need to follow up/continue here, but it also at least unblocks the CPU side higher order stuff]

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