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 methodinstance usage and backedges #2199

Merged
merged 17 commits into from
Dec 15, 2024
Merged

Fix methodinstance usage and backedges #2199

merged 17 commits into from
Dec 15, 2024

Conversation

wsmoses
Copy link
Member

@wsmoses wsmoses commented Dec 14, 2024

Hopefully now that I understand backedges, this fixes pending issues.

Simultaneously this gets rid of a bunch of inner generated functions so hopefully compile time comes down.

Time to first derivative (post precompile) is now:

julia> @time autodiff(Forward, sin, Duplicated(1.0, 2.0))
  0.000014 seconds
(1.0806046117362795,)

julia> @time autodiff(Forward, sin, Duplicated(1.0, 2.0))
  0.000003 seconds
(1.0806046117362795,)

julia> @time autodiff(Forward, sin, Duplicated(1.0, 2.0))
  0.000002 seconds
(1.0806046117362795,)

Copy link
Contributor

github-actions bot commented Dec 14, 2024

Benchmark Results

main 4dbca9f... main/4dbca9f3a41ab7...
basics/overhead 4.34 ± 0.01 ns 4.34 ± 0.92 ns 1
time_to_load 1.15 ± 0.0054 s 1.15 ± 0.0089 s 0.997

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).

@wsmoses wsmoses merged commit ee21090 into main Dec 15, 2024
18 of 29 checks passed
@wsmoses wsmoses deleted the metinst branch December 15, 2024 06:47
@@ -1258,6 +1272,8 @@ Create the methodinstance pair, and lookup the primal return type.
@nospecialize(TT::Type),
world::Union{UInt,Nothing} = nothing,
)

fdsafdsafsa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Comment on lines +5599 to +5614
fwd_sig = Tuple{typeof(EnzymeRules.forward), <:EnzymeRules.FwdConfig, <:Enzyme.EnzymeCore.Annotation, Type{<:Enzyme.EnzymeCore.Annotation},Vararg{Enzyme.EnzymeCore.Annotation}}
push!(edges, ccall(:jl_method_table_for, Any, (Any,), fwd_sig)::Core.MethodTable)
push!(edges, fwd_sig)
else
push!(edges, GPUCompiler.methodinstance(typeof(Compiler.Interpreter.rule_backedge_holder), Tuple{typeof(EnzymeRules.augmented_primal)}, world))
rev_sig = Tuple{typeof(EnzymeRules.augmented_primal), <:EnzymeRules.RevConfig, <:Enzyme.EnzymeCore.Annotation, Type{<:Enzyme.EnzymeCore.Annotation},Vararg{Enzyme.EnzymeCore.Annotation}}
push!(edges, ccall(:jl_method_table_for, Any, (Any,), rev_sig)::Core.MethodTable)
push!(edges, rev_sig)

rev_sig = Tuple{typeof(EnzymeRules.reverse), <:EnzymeRules.RevConfig, <:Enzyme.EnzymeCore.Annotation, Union{Type{<:Enzyme.EnzymeCore.Annotation}, Enzyme.EnzymeCore.Active}, Any, Vararg{Enzyme.EnzymeCore.Annotation}}
push!(edges, ccall(:jl_method_table_for, Any, (Any,), rev_sig)::Core.MethodTable)
push!(edges, rev_sig)
end

ina_sig = Tuple{typeof(EnzymeRules.inactive), Vararg{Any}}
push!(edges, ccall(:jl_method_table_for, Any, (Any,), ina_sig)::Core.MethodTable)
push!(edges, ina_sig)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does look much better.

@@ -110,7 +94,7 @@ function rule_backedge_holder_generator(world::UInt, source, self, ft::Type)
new_ci.slotflags = UInt8[0x00 for i = 1:2]

# return the codegen world age
push!(new_ci.code, Core.Compiler.ReturnNode(0))
push!(new_ci.code, Core.Compiler.ReturnNode(world))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm this, on the other hand, looks horribly illegal.

@@ -198,6 +176,37 @@ function EnzymeInterpreter(
else
InferenceParams(; unoptimize_throw_blocks=false)
end

@static if HAS_INTEGRATED_CACHE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also contemplating that Enzyme for 1.11 could simply not use the integrated cache...
But we might have removed some interface...

@@ -2,30 +2,32 @@ import EnzymeCore: Annotation
import EnzymeCore.EnzymeRules: FwdConfig, RevConfig, forward, augmented_primal, inactive, _annotate_tt

function has_frule_from_sig(@nospecialize(interp::Core.Compiler.AbstractInterpreter),
@nospecialize(TT), sv::Core.Compiler.AbsIntState)::Bool
@nospecialize(TT::Type), sv::Core.Compiler.AbsIntState, partialedge::Bool=true)::Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a partialedge? It seems not used?

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