Skip to content

Commit

Permalink
lowering: Keep track of which :enter correspond to which :leave (#51590)
Browse files Browse the repository at this point in the history
This should be NFC and is intended to allow the optimizer to delete
:enter statements (by replacing them with `nothing`), without leaving
dangling `:leave`s around. This is accomplished by having `leave` take
(a variable number of) `:enter` tokens (that are already being used by
`:pop_exception`). The semantics are that a literal `nothing` or an
SSAValue pointing to a `nothing` statement are ignored, and one
exception handler is popped for each remaining argument. The actual
value of the token is ignored, except that the verifier asserts that it
belongs to an `:enter`.

Note that we don't need to do the same for :pop_exception, because the
token generated by an `:enter` is semantically only in scope for
:pop_exception during its catch block. If we determine the `:enter` is
dead, then its catch block is guaranteed to not be executed and will be
deleted wholesale by cfg liveness.

I was considering doing something fancier where :leave is changed back
to taking an integer after optimization, but the case where the IR size
is bigger after this change (when we are `:leave`ing many handlers) is
fairly rare and likely not worth the additional complexity or time cost
to do anything special. If it does show up in size benchmarks, I'd
rather give `:leave` a special, compact encoding.
  • Loading branch information
Keno authored Oct 11, 2023
1 parent 8180240 commit 3711749
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 59 deletions.
21 changes: 18 additions & 3 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ function compute_trycatch(code::Vector{Any}, ip::BitSet)
# The goal initially is to record the frame like this for the state at exit:
# 1: (enter 3) # == 0
# 3: (expr) # == 1
# 3: (leave 1) # == 1
# 3: (leave %1) # == 1
# 4: (expr) # == 0
# then we can find all trys by walking backwards from :enter statements,
# and all catches by looking at the statement after the :enter
Expand Down Expand Up @@ -386,7 +386,20 @@ function compute_trycatch(code::Vector{Any}, ip::BitSet)
if head === :enter
cur_hand = pc
elseif head === :leave
l = stmt.args[1]::Int
l = 0
for j = 1:length(stmt.args)
arg = stmt.args[j]
if arg === nothing
continue
else
enter_stmt = code[(arg::SSAValue).id]
if enter_stmt === nothing
continue
end
@assert isexpr(enter_stmt, :enter) "malformed :leave"
end
l += 1
end
for i = 1:l
cur_hand = handler_at[cur_hand]
end
Expand All @@ -396,7 +409,9 @@ function compute_trycatch(code::Vector{Any}, ip::BitSet)

pc´ > n && break # can't proceed with the fast-path fall-through
if handler_at[pc´] != cur_hand
@assert handler_at[pc´] == 0 "unbalanced try/catch"
if handler_at[pc´] != 0
@assert false "unbalanced try/catch"
end
handler_at[pc´] = cur_hand
elseif !in(pc´, ip)
break # already visited
Expand Down
1 change: 1 addition & 0 deletions base/compiler/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ function is_relevant_expr(e::Expr)
:foreigncall, :isdefined, :copyast,
:throw_undef_if_not,
:cfunction, :method, :pop_exception,
:leave,
:new_opaque_closure)
end

Expand Down
14 changes: 14 additions & 0 deletions base/compiler/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,20 @@ function verify_ir(ir::IRCode, print::Bool=true,
# TODO: these are not yet linearized
continue
end
elseif stmt.head === :leave
for i in 1:length(stmt.args)
arg = stmt.args[i]
if !isa(arg, Union{Nothing, SSAValue})
@verify_error "Malformed :leave - Expected `Nothing` or SSAValue"
error()
elseif isa(arg, SSAValue)
enter_stmt = ir[arg::SSAValue][:stmt]
if !isa(enter_stmt, Nothing) && !isexpr(enter_stmt, :enter)
@verify_error "Malformed :leave - argument ssavalue should point to `nothing` or :enter"
error()
end
end
end
end
end
n = 1
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/validation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const VALID_EXPR_HEADS = IdDict{Symbol,UnitRange{Int}}(
:splatnew => 2:2,
:the_exception => 0:0,
:enter => 1:1,
:leave => 1:1,
:leave => 1:typemax(Int),
:pop_exception => 1:1,
:inbounds => 1:1,
:inline => 1:1,
Expand Down
2 changes: 1 addition & 1 deletion doc/src/devdocs/ssair.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ The corresponding IR (with irrelevant types stripped) is:
4 ┄ %13 = φᶜ (%3, %6, %9)::Bool
│ %14 = φᶜ (%4, %7, %10)::Core.Compiler.MaybeUndef(Int64)
│ %15 = φᶜ (%5)::Core.Const(1)
└── $(Expr(:leave, 1))
└── $(Expr(:leave, Core.SSAValue(2)))
5 ─ $(Expr(:pop_exception, :(%2)))::Any
│ $(Expr(:throw_undef_if_not, :y, :(%13)))::Any
│ %19 = Core.tuple(%15, %14)
Expand Down
14 changes: 12 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5386,9 +5386,19 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
return;
}
else if (head == jl_leave_sym) {
assert(jl_is_long(args[0]));
int hand_n_leave = 0;
for (size_t i = 0; i < jl_expr_nargs(ex); ++i) {
jl_value_t *arg = args[i];
if (arg == jl_nothing)
continue;
assert(jl_is_ssavalue(arg));
jl_value_t *enter_stmt = jl_array_ptr_ref(ctx.code, ((jl_ssavalue_t*)arg)->id - 1);
if (enter_stmt == jl_nothing)
continue;
hand_n_leave += 1;
}
ctx.builder.CreateCall(prepare_call(jlleave_func),
ConstantInt::get(getInt32Ty(ctx.builder.getContext()), jl_unbox_long(args[0])));
ConstantInt::get(getInt32Ty(ctx.builder.getContext()), hand_n_leave));
}
else if (head == jl_pop_exception_sym) {
jl_cgval_t excstack_state = emit_expr(ctx, jl_exprarg(expr, 0));
Expand Down
36 changes: 24 additions & 12 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -554,18 +554,30 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
}
}
else if (head == jl_leave_sym) {
int hand_n_leave = jl_unbox_long(jl_exprarg(stmt, 0));
assert(hand_n_leave > 0);
// equivalent to jl_pop_handler(hand_n_leave), but retaining eh for longjmp:
jl_handler_t *eh = ct->eh;
while (--hand_n_leave > 0)
eh = eh->prev;
jl_eh_restore_state(eh);
// leave happens during normal control flow, but we must
// longjmp to pop the eval_body call for each enter.
s->continue_at = next_ip;
asan_unpoison_task_stack(ct, &eh->eh_ctx);
jl_longjmp(eh->eh_ctx, 1);
int hand_n_leave = 0;
for (int i = 0; i < jl_expr_nargs(stmt); ++i) {
jl_value_t *arg = jl_exprarg(stmt, i);
if (arg == jl_nothing)
continue;
assert(jl_is_ssavalue(arg));
jl_value_t *enter_stmt = jl_array_ptr_ref(stmts, ((jl_ssavalue_t*)arg)->id - 1);
if (enter_stmt == jl_nothing)
continue;
hand_n_leave += 1;
}
if (hand_n_leave > 0) {
assert(hand_n_leave > 0);
// equivalent to jl_pop_handler(hand_n_leave), but retaining eh for longjmp:
jl_handler_t *eh = ct->eh;
while (--hand_n_leave > 0)
eh = eh->prev;
jl_eh_restore_state(eh);
// leave happens during normal control flow, but we must
// longjmp to pop the eval_body call for each enter.
s->continue_at = next_ip;
asan_unpoison_task_stack(ct, &eh->eh_ctx);
jl_longjmp(eh->eh_ctx, 1);
}
}
else if (head == jl_pop_exception_sym) {
size_t prev_state = jl_unbox_ulong(eval_value(jl_exprarg(stmt, 0), s));
Expand Down
76 changes: 43 additions & 33 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -4291,7 +4291,7 @@ f(x) = yt(x)
;; be emitted at the exit of the block. Code
;; should enter the finally block via `enter-finally-block`.
(handler-goto-fixups '()) ;; `goto`s that might need `leave` exprs added
(handler-level 0) ;; exception handler nesting depth
(handler-token-stack '()) ;; tokens identifying handler stack while active
(catch-token-stack '())) ;; tokens identifying handler enter for current catch blocks
(define (emit c)
(or c (raise "missing value in IR"))
Expand Down Expand Up @@ -4322,12 +4322,13 @@ f(x) = yt(x)
(emit `(= ,(car finally-handler) ,tag))
(if need-goto
(let ((label (cadr finally-handler))
(dest-handler-level (cadddr finally-handler))
(dest-tokens (caddddr finally-handler)))
(dest-handler-tokens (cadddr finally-handler))
(dest-catch-tokens (caddddr finally-handler)))
;; Leave current exception handling scope and jump to finally block
(let ((pexc (pop-exc-expr catch-token-stack dest-tokens)))
(let ((pexc (pop-exc-expr catch-token-stack dest-catch-tokens)))
(if pexc (emit pexc)))
(emit `(leave ,(+ 1 (- handler-level dest-handler-level))))
(let ((plist (pop-handler-list handler-token-stack (cdr dest-handler-tokens) '())))
(emit `(leave ,@plist)))
(emit `(goto ,label))))
tag))
(define (pop-exc-expr src-tokens dest-tokens)
Expand All @@ -4340,6 +4341,18 @@ f(x) = yt(x)
(car s)
(loop (cdr s))))))
`(pop_exception ,restore-token))))
(define (pop-handler-list src-tokens dest-tokens lab)
(if (eq? src-tokens dest-tokens)
#f
(let loop ((s src-tokens)
(l '()))
(if (not (pair? s))
(if (null? lab)
(error "Attempt to jump into catch block")
(error (string "cannot goto label \"" lab "\" inside try/catch block"))))
(if (eq? (cdr s) dest-tokens)
(cons (car s) l)
(loop (cdr s) (cons (car s) l))))))
(define (emit-return x)
(define (emit- x)
(let* ((tmp (if ((if (null? catch-token-stack) valid-ir-return? simple-atom?) x)
Expand All @@ -4357,27 +4370,27 @@ f(x) = yt(x)
(if pexc (emit pexc)))
(emit `(return ,x))))
(if x
(if (> handler-level 0)
(if (null? handler-token-stack)
(actually-return x)
(let ((tmp (cond ((and (simple-atom? x) (or (not (ssavalue? x)) (not finally-handler))) #f)
(finally-handler (new-mutable-var))
(else (make-ssavalue)))))
(if tmp (emit `(= ,tmp ,x)))
(if finally-handler
(enter-finally-block `(return ,(or tmp x)))
(begin (emit `(leave ,handler-level))
(begin (emit `(leave ,@handler-token-stack))
(actually-return (or tmp x))))
(or tmp x))
(actually-return x))))
(or tmp x)))))
(define (emit-break labl)
(let ((lvl (caddr labl))
(let ((dest-handler-tokens (caddr labl))
(dest-tokens (cadddr labl)))
(if (and finally-handler (> (cadddr finally-handler) lvl))
(if (and finally-handler (> (length (cadddr finally-handler)) (length dest-handler-tokens)))
(enter-finally-block `(break ,labl))
(begin
(let ((pexc (pop-exc-expr catch-token-stack dest-tokens)))
(if pexc (emit pexc)))
(if (> handler-level lvl)
(emit `(leave ,(- handler-level lvl))))
(let ((plist (pop-handler-list handler-token-stack dest-handler-tokens '())))
(if plist (emit `(leave ,@plist))))
(emit `(goto ,(cadr labl)))))))
(define (new-mutable-var . name)
(let ((g (if (null? name) (gensy) (named-gensy (car name)))))
Expand Down Expand Up @@ -4654,7 +4667,7 @@ f(x) = yt(x)
((break-block)
(let ((endl (make-label)))
(compile (caddr e)
(cons (list (cadr e) endl handler-level catch-token-stack)
(cons (list (cadr e) endl handler-token-stack catch-token-stack)
break-labels)
#f #f)
(mark-label endl))
Expand All @@ -4668,7 +4681,7 @@ f(x) = yt(x)
(if (eq? (car e) 'symboliclabel)
(if (has? label-nesting (cadr e))
(error (string "label \"" (cadr e) "\" defined multiple times"))
(put! label-nesting (cadr e) (list handler-level catch-token-stack))))
(put! label-nesting (cadr e) (list handler-token-stack catch-token-stack))))
(let ((m (get label-map (cadr e) #f)))
(if m
(emit `(label ,m))
Expand All @@ -4684,7 +4697,7 @@ f(x) = yt(x)
(emit `(null)) ;; save space for `leave` that might be needed
(emit `(goto ,m))
(set! handler-goto-fixups
(cons (list code handler-level catch-token-stack (cadr e)) handler-goto-fixups))
(cons (list code handler-token-stack catch-token-stack (cadr e)) handler-goto-fixups))
#f))

;; exception handlers are lowered using
Expand All @@ -4701,8 +4714,8 @@ f(x) = yt(x)
(my-finally-handler #f))
;; handler block entry
(emit `(= ,handler-token (enter ,catch)))
(set! handler-level (+ handler-level 1))
(if finally (begin (set! my-finally-handler (list finally endl '() handler-level catch-token-stack))
(set! handler-token-stack (cons handler-token handler-token-stack))
(if finally (begin (set! my-finally-handler (list finally endl '() handler-token-stack catch-token-stack))
(set! finally-handler my-finally-handler)
(emit `(= ,finally -1))))
(let* ((v1 (compile (cadr e) break-labels value #f)) ;; emit try block code
Expand All @@ -4713,12 +4726,12 @@ f(x) = yt(x)
(if tail
(begin (if els
(begin (if (and (not val) v1) (emit v1))
(emit '(leave 1)))
(emit `(leave ,handler-token)))
(if v1 (emit-return v1)))
(if (not finally) (set! endl #f)))
(begin (emit '(leave 1))
(begin (emit `(leave ,handler-token))
(emit `(goto ,(or els endl)))))
(set! handler-level (- handler-level 1))
(set! handler-token-stack (cdr handler-token-stack))
;; emit else block
(if els
(begin (mark-label els)
Expand All @@ -4727,7 +4740,7 @@ f(x) = yt(x)
(if endl (emit `(goto ,endl)))))
;; emit either catch or finally block
(mark-label catch)
(emit `(leave 1))
(emit `(leave ,handler-token))
(if finally
(begin (enter-finally-block '(call (top rethrow)) #f) ;; enter block via exception
(mark-label endl) ;; non-exceptional control flow enters here
Expand Down Expand Up @@ -4885,21 +4898,18 @@ f(x) = yt(x)
(compile e '() #t #t)
(for-each (lambda (x)
(let ((point (car x))
(hl (cadr x))
(src-tokens (caddr x))
(src-handler-tokens (cadr x))
(src-catch-tokens (caddr x))
(lab (cadddr x)))
(let ((target-nesting (get label-nesting lab #f)))
(if (not target-nesting)
(error (string "label \"" lab "\" referenced but not defined")))
(let ((target-level (car target-nesting)))
(cond ((> target-level hl)
(error (string "cannot goto label \"" lab "\" inside try/catch block")))
((= target-level hl)
(set-cdr! point (cddr point))) ;; remove empty slot
(else
(set-car! (cdr point) `(leave ,(- hl target-level))))))
(let ((pexc (pop-exc-expr src-tokens (cadr target-nesting))))
(if pexc (set-cdr! point (cons pexc (cdr point))))))))
(let ((target-handler-tokens (car target-nesting))
(target-catch-tokens (cadr target-nesting)))
(let ((plist (pop-handler-list src-handler-tokens target-handler-tokens lab)))
(if plist (set-car! (cdr point) `(leave ,@plist))))
(let ((pexc (pop-exc-expr src-catch-tokens target-catch-tokens)))
(if pexc (set-cdr! point (cons pexc (cdr point)))))))))
handler-goto-fixups)
(if global-const-error
(error (string "`global const` declaration not allowed inside function" (format-loc global-const-error))))
Expand Down
10 changes: 5 additions & 5 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4371,18 +4371,18 @@ let x = Tuple{Int,Any}[
#= 4=# (2, Expr(:enter, 12))
#= 5=# (4, Expr(:(=), Core.SlotNumber(3), '3'))
#= 6=# (4, Core.GotoIfNot(Core.SlotNumber(2), 9))
#= 7=# (4, Expr(:leave, 2))
#= 7=# (4, Expr(:leave, Core.SSAValue(4), Core.SSAValue(2)))
#= 8=# (0, Core.ReturnNode(1))
#= 9=# (4, Expr(:call, GlobalRef(Main, :throw)))
#=10=# (4, Expr(:leave, 1))
#=10=# (4, Expr(:leave, Core.SSAValue(4)))
#=11=# (2, Core.GotoNode(16))
#=12=# (4, Expr(:leave, 1))
#=12=# (4, Expr(:leave, Core.SSAValue(4)))
#=13=# (2, Expr(:(=), Core.SlotNumber(4), Expr(:the_exception)))
#=14=# (2, Expr(:call, GlobalRef(Main, :rethrow)))
#=15=# (2, Expr(:pop_exception, Core.SSAValue(4)))
#=16=# (2, Expr(:leave, 1))
#=16=# (2, Expr(:leave, Core.SSAValue(2)))
#=17=# (0, Core.GotoNode(22))
#=18=# (2, Expr(:leave, 1))
#=18=# (2, Expr(:leave, Core.SSAValue(2)))
#=19=# (0, Expr(:(=), Core.SlotNumber(5), Expr(:the_exception)))
#=20=# (0, nothing)
#=21=# (0, Expr(:pop_exception, Core.SSAValue(2)))
Expand Down
2 changes: 1 addition & 1 deletion test/compiler/interpreter_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ let m = Meta.@lower 1 + 1
# block 6
Core.PhiCNode(Any[Core.SSAValue(5), Core.SSAValue(7), Core.SSAValue(9)]), # NULL, :a, :b
Core.PhiCNode(Any[Core.SSAValue(6)]), # NULL
Expr(:leave, 1),
Expr(:leave, Core.SSAValue(4)),
# block 7
ReturnNode(Core.SSAValue(11)),
]
Expand Down
2 changes: 1 addition & 1 deletion test/compiler/ssair.jl
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ let
Expr(:enter, 11),
Expr(:call, :+, SSAValue(3), 1),
Expr(:throw_undef_if_not, :expected, false),
Expr(:leave, 1),
Expr(:leave, Core.SSAValue(1)),
Expr(:(=), SSAValue(1), Expr(:call, :+, SSAValue(3), 1)),
UpsilonNode(),
UpsilonNode(SSAValue(2)),
Expand Down

2 comments on commit 3711749

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.