-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Description
The problem
While looking at #48918 with @vtjnash, we discussed that the @GC.preserve
lowering is probably bad. There's at least three related, but separate problems.
Consider:
julia> function foo(x)
while true
@GC.preserve x (rand() < 0.5 && continue)
break
end
end
foo (generic function with 1 method)
julia> @code_lowered foo("abc")
CodeInfo(
1 ┄ goto #9 if not true
2 ─ %2 = $(Expr(:gc_preserve_begin, :(x)))
│ %3 = Main.:<
│ %4 = Main.rand
│ %5 = dynamic (%4)()
│ %6 = dynamic (%3)(%5, 0.5)
└── goto #6 if not %6
3 ─ goto #8
4 ─ %9 = goto #8
5 ─ @_3 = %9
└── goto #7
6 ─ @_3 = false
7 ┄ @_3
│ $(Expr(:gc_preserve_end, :(%2)))
└── goto #9
8 ┄ goto #1
9 ┄ return nothing
)
The gc_preserve_end
here is outside of the loop body, so semantically, we're only ending the lifetime of the value in the last loop.
Consider
julia> function bar(x)
@goto foo
@GC.preserve x begin
@label foo
ccall(:jl_, Cvoid, (Any,), x)
end
end
bar (generic function with 1 method)
julia> @code_llvm bar("abc")
ERROR: AssertionError: val.id > 0
Stacktrace:
[1] renumber_ssa(stmt::Core.SSAValue, ssanums::Vector{Core.SSAValue}, new_ssa::Bool)
@ Compiler ./../usr/share/julia/Compiler/src/ssair/slot2ssa.jl:56
[2] #renumber_ssa!##0
@ ./../usr/share/julia/Compiler/src/ssair/slot2ssa.jl:62 [inlined]
[3] ssamap(f::Compiler.var"#renumber_ssa!##0#renumber_ssa!##1"{Vector{Core.SSAValue}, Bool}, stmt::Any)
@ Compiler ./../usr/share/julia/Compiler/src/utilities.jl:245
[4] renumber_ssa!
@ ./../usr/share/julia/Compiler/src/ssair/slot2ssa.jl:62 [inlined]
-
Inlining can extend the scope of a
@GC.preserve
if there's a return inside the region. -
The scope of GC.preserve is not entirely correct in the presence of try/catch:
Lines 6098 to 6108 in fde79f5
// TODO: The semantics of `gc_preserve` are not perfect here. An `Expr(:enter, ...)` block may // have multiple exits, but effects of `preserve_end` are only extended to the end of the // dominance of each `Expr(:leave, ...)`. // // That means that a scope object can suddenly end up preserved again outside of an // `Expr(:enter, ...)` region where it ought to be dead. It'd be preferable if the effects // of gc_preserve_end propagated through a control-flow joins as long as all incoming // agree about the preserve state. // // This is correct as-is anyway - it just means the scope lives longer than it needs to // if the `Expr(:enter, ...)` has multiple exits.
Root cause
The core problem is that GC.preserve
as lexical, but rather dynamic. Contrast this with something like inbounds
which sets a flag on every (lexical) call inside the annotated region.
Proposed solution
Ideally, we'd attach the GC preserve to every call lexically, like we do for the other lexically scoped options. Unfortunately, it's not quite as simple as setting a flag, because the GC preserve takes arguments. My proposal is something like the following. Turn @GC.preserve p f(arg)
into:
f2 = Core.gc_preserve_during(f, p)
f2(arg)
where gc_preserve_during
is a new intrinsic that semantically wraps the f
in a closure that preserves p
during the execution of the call. Obviously inference/codegen would ensure that this never actually needs to be allocated. For :invoke
, we could rename :invoke_modify
to :invoke_special
and use it here as well to annotate the edge.