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

proper termination, take 2 #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

proper termination, take 2 #117

wants to merge 1 commit into from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Sep 10, 2024

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
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.
@aviatesk aviatesk force-pushed the avi/implicit-return branch from cef292f to aa4e09d Compare October 11, 2024 05:42
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.

1 participant