Skip to content

Fix higher order codegen - #2161

Merged
wsmoses merged 7 commits into
mainfrom
deffix
Dec 7, 2024
Merged

wsmoses merged 7 commits into
mainfrom
deffix

Conversation

@wsmoses

@wsmoses wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member

No description provided.

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

vchuravy commented Dec 3, 2024

Copy link
Copy Markdown
Member

What's the GPU story here?

@wsmoses

wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

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

@github-actions

github-actions Bot commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

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

vchuravy commented Dec 3, 2024

Copy link
Copy Markdown
Member

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

wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

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

wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

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

@vchuravy

vchuravy commented Dec 3, 2024

Copy link
Copy Markdown
Member

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

vchuravy commented Dec 3, 2024

Copy link
Copy Markdown
Member

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

wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

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

wsmoses commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

[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