Skip to content

proper termination, take 2#117

Merged
timholy merged 3 commits into
masterfrom
avi/implicit-return
Jun 11, 2026
Merged

proper termination, take 2#117
timholy merged 3 commits into
masterfrom
avi/implicit-return

Conversation

@aviatesk

@aviatesk aviatesk commented Sep 10, 2024

Copy link
Copy Markdown
Member

This PR is an alternative to #99. This is built on top of #116.

With this PR, the following test cases now pass correctly:

    # Final block is not a `return`: Need to use `controller::SelectiveEvalController` explicitly
    ex = quote
        x = 1
        yy = 7
        @label loop
        x += 1
        x < 5 || return yy
        @goto loop
    end
    frame = Frame(ModSelective, ex)
    src = frame.framecode.src
    edges = CodeEdges(ModSelective, src)
    controller = SelectiveEvalController()
    isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges, controller)
    selective_eval_fromstart!(controller, frame, isrequired, true)
    @test ModSelective.x == 5
    @test !isdefined(ModSelective, :yy)

The basic approach is overloading JuliaInterpreter.step_expr! and LoweredCodeUtils.next_or_nothing! for the new SelectiveEvalController type, as described below, to perform correct selective execution.

When SelectiveEvalController is passed as the recurse argument of selective_eval!, the selective execution is adjusted as follows:

  • Implicit return: In Julia's IR representation (CodeInfo), the final block does not necessarily return and may goto another block. And if the return statement is not included in the slice in such cases, it is necessary to terminate selective_eval! when execution reaches such implicit return statements. controller.implicit_returns records the PCs of such return statements, and selective_eval! will return when reaching those statements. This is the core part of the fix for the test cases in Add failing test for proper termination #99.

  • CFG short-cut: When the successors of a conditional branch are inactive, and it is safe to move the program counter from the conditional branch to the nearest common post-dominator of those successors, this short-cut is taken. This short-cut is not merely an optimization but is actually essential for the correctness of the selective execution. This is because, in CodeInfo, even if we simply fall-through dead blocks (i.e., increment the program counter without executing the statements of those blocks), it does not necessarily lead to the nearest common post-dominator block.

And now lines_required or lines_required! will update the SelectiveEvalController passed as their argument to be appropriate for the program slice generated.

One thing to note is that currently, the controller is not be recursed. That said, in Revise, which is the main consumer of LCU, there is no need for recursive selective execution, and so selective_eval! does not provide a system for inter-procedural selective evaluation. Accordingly SelectiveEvalController does not recurse too, but this can be left as a future extension.

@aviatesk aviatesk requested a review from timholy September 10, 2024 16:21
@aviatesk aviatesk force-pushed the avi/implicit-return branch 2 times, most recently from 0dfe9a3 to e92ca73 Compare September 10, 2024 16:26
Base automatically changed from avi/new-add_control_flow! to master September 11, 2024 04:32
@aviatesk aviatesk force-pushed the avi/implicit-return branch 2 times, most recently from 78c2db7 to 475dc90 Compare September 11, 2024 04:48
@aviatesk aviatesk force-pushed the avi/implicit-return branch from cef292f to aa4e09d Compare October 11, 2024 05:42

@timholy timholy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great, and far simpler than what I was trying. I only noted a couple of places that might benefit from further clarifications. (Overall your docs are excellent, many thanks!)

If you know this works with both Revise and JET, let's get this merged.

Comment thread src/codeedges.jl Outdated
Comment thread src/codeedges.jl Outdated
Comment thread src/codeedges.jl Outdated
Comment thread src/codeedges.jl Outdated
Comment thread src/codeedges.jl Outdated
@aviatesk aviatesk force-pushed the avi/implicit-return branch 4 times, most recently from b11e388 to 04b843a Compare July 31, 2025 19:28
Comment thread src/codeedges.jl Outdated
@timholy

timholy commented Aug 9, 2025

Copy link
Copy Markdown
Member

Documenter test should be fixed before merging, though

aviatesk and others added 3 commits June 4, 2026 07:38
This PR is an alternative to #99. This is
built on top of #116.

With this PR, the following test cases now pass correctly: ```julia
    #Final block is not a `return`: Need to use
    #`config::SelectiveEvalRecurse` explicitly
    ex = quote x = 1 yy = 7 @Label loop x += 1 x < 5 || return yy @goto
    loop end frame = Frame(ModSelective, ex) src = frame.framecode.src
    edges = CodeEdges(ModSelective, src) config = SelectiveEvalRecurse()
    isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges,
    config) selective_eval_fromstart!(config, frame, isrequired, true)
    @test ModSelective.x == 5 @test !isdefined(ModSelective, :yy) ```

The basic approach is overloading `JuliaInterpreter.step_expr!` and
`LoweredCodeUtils.next_or_nothing!` for the new
`SelectiveEvalController` type, as described below, to perform correct
selective execution.

When `SelectiveEvalController` is passed as the `recurse` argument of
`selective_eval!`, the selective execution is adjusted as follows:

- **Implicit return**: In Julia's IR representation (`CodeInfo`), the
final block does not necessarily return and may `goto` another block.
And if the `return` statement is not included in the slice in such
cases, it is necessary to terminate `selective_eval!` when execution
reaches such implicit return statements. `controller.implicit_returns`
records the PCs of such return statements, and `selective_eval!` will
return when reaching those statements. This is the core part of the fix
for the test cases in #99.

- **CFG short-cut**: When the successors of a conditional branch are
inactive, and it is safe to move the program counter from the
conditional branch to the nearest common post-dominator of those
successors, this short-cut is taken. This short-cut is not merely an
optimization but is actually essential for the correctness of the
selective execution. This is because, in `CodeInfo`, even if we simply
fall-through dead blocks (i.e., increment the program counter without
executing the statements of those blocks), it does not necessarily lead
to the nearest common post-dominator block.

And now [`lines_required`](@ref) or [`lines_required!`](@ref) will
update the `SelectiveEvalController` passed as their argument to be
appropriate for the program slice generated.

One thing to note is that currently, the `controller` is not be
recursed. That said, in Revise, which is the main consumer of LCU, there
is no need for recursive selective execution, and so `selective_eval!`
does not provide a system for inter-procedural selective evaluation.
Accordingly `SelectiveEvalController` does not recurse too, but this can
be left as a future extension.
Complete the work in PR #117 (proper termination for selective
evaluation, addressing #99). The branch had drifted across three names
for the CFG-control object; settle on `SelectiveEvalController`
throughout the code, exports, docstrings, and tests. This also repairs
the Documenter build, which failed because `docs/src/api.md` referenced
`SelectiveEvalController` while the code defined `SelectiveEvalInfo`.

Also fix docstring typos introduced by an earlier rename
(`inforeter`/`inforocedural`, `record_implcit_return!`), restore the
`SelectiveInterpreter` docstring, rewrite the `selective_eval!`
docstring to describe how to obtain fully-correct selective evaluation
(populate a controller via `lines_required` and run a matching
`SelectiveInterpreter`), and drop a leftover commented-out debug line.

The default `selective_eval!` path is unchanged: it constructs an empty
controller, so existing consumers (Revise, JET) see identical behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Minor bump for the new exported API (`SelectiveEvalController`,
`SelectiveInterpreter`) added in PR #117.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@timholy timholy force-pushed the avi/implicit-return branch from 04b843a to b14c419 Compare June 4, 2026 12:39
@timholy

timholy commented Jun 4, 2026

Copy link
Copy Markdown
Member

@aviatesk I've rebased and updated this PR. Let me know if you have any objections to merging, otherwise I'll merge it within a week or so.

@timholy timholy merged commit 77d6e95 into master Jun 11, 2026
9 checks passed
@timholy timholy deleted the avi/implicit-return branch June 11, 2026 10:21
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