From 3d50855280cb26c7b055d05352e2d130679fe3c5 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Wed, 25 Oct 2023 02:02:03 +0000 Subject: [PATCH 01/12] Properly guard UpsilonNode unboxed store In #51852, we are coercing a boxed `Union{@NamedTuple{progress::String}, @NamedTuple{progress::Float64}}` to `@NamedTuple{progress::String}` via convert_julia_type. This results in a jl_cgval_t that has a Vboxed that points to a boxed `@NamedTuple{progress::Float64}` but with a `@NamedTuple{progress::String}` type tag that the up upsilonnode code then tries to unbox into the typed PhiC slot. This ends up treating the Float64 as a pointer and crashing in GC. Avoid this by adding a runtime check that the converted value is actually compatible (we already had this kind of check for the non-boxed cases) and then making the unboxing runtime-conditional on the type of the union matching the type of the phic. Fixes #51852 --- test/compiler/inference.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 20fdaaf297d82..361d004a7a92d 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -5450,7 +5450,6 @@ end @test Base.return_types(phic_type8) |> only === Int @test phic_type8() === 2 - function phic_type9() local a try From 81cced1945c68d7b777f0ee8f595539fa064cd4a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Wed, 1 Nov 2023 02:00:28 +0000 Subject: [PATCH 02/12] WIP: Outline potentially undefined globals during lowering Currently [1] it is illegal [2] in IRCode to have a GlobalRef in value position that could potentially throw. This is because in IRCode, we want to assign flags to every statement and if there are multiple things with effects in a statement, we lose precision in tracking which they apply to. However, we currently do allow this in `CodeInfo`. Now that we're starting to make more use of flags in inference also, this is becoming annoying (as it did for IRCode), so I would like to do this transformation earlier. This is an attempt to do this during lowering. It is not entirely clear that this is precisely the correct place for it. We could alternatively consider doing it during the global resolve pass in method.c, but that currently does not renumber SSAValues, so doing it during the renumbering inside lowering may be easier. [1] https://github.com/JuliaLang/julia/commit/39c278b728deb04c3a32d70e3e35dcef7822c0c0 [2] https://github.com/JuliaLang/julia/blob/2f63cc99fb134fb4adb7f11ba86a4e2ab5adcd48/base/compiler/ssair/verify.jl#L54-L58 --- base/reflection.jl | 2 ++ src/ast.c | 12 ++++++++++++ src/jlfrontend.scm | 1 + src/julia-syntax.scm | 20 ++++++++++++++++---- test/compiler/inference.jl | 2 ++ test/core.jl | 33 +++++++++++++++++---------------- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/base/reflection.jl b/base/reflection.jl index e0764ee761a3e..5e32d6e68a58e 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -2081,6 +2081,8 @@ function bodyfunction(basemethod::Method) else return nothing end + elseif isa(fsym, Core.SSAValue) + fsym = ast.code[fsym.id] else return nothing end diff --git a/src/ast.c b/src/ast.c index 9e22369d836b9..2ba5f89a2cb27 100644 --- a/src/ast.c +++ b/src/ast.c @@ -164,6 +164,17 @@ static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint return (b != NULL && jl_atomic_load_relaxed(&b->owner) == b) ? fl_ctx->T : fl_ctx->F; } +static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) +{ + // tells whether a var is defined, in the sense that accessing it is nothrow + argcount(fl_ctx, "nothrow-julia-global", nargs, 1); + (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global"); + jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); + jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); + jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); + return b != NULL ? fl_ctx->T : fl_ctx->F; +} + static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT { jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); @@ -210,6 +221,7 @@ static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *m static const builtinspec_t julia_flisp_ast_ext[] = { { "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint + { "nothrow-julia-global", fl_nothrow_julia_global }, { "current-julia-module-counter", fl_current_module_counter }, { "julia-scalar?", fl_julia_scalar }, { "julia-current-file", fl_julia_current_file }, diff --git a/src/jlfrontend.scm b/src/jlfrontend.scm index 34a8f5405676a..b46663c560346 100644 --- a/src/jlfrontend.scm +++ b/src/jlfrontend.scm @@ -31,6 +31,7 @@ ;; this is overwritten when we run in actual julia (define (defined-julia-global v) #f) +(define (nothrow-julia-global v) #f) (define (julia-current-file) 'none) (define (julia-current-line) 0) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 6afb8afaffbea..bd1ba6ce1863d 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4255,7 +4255,7 @@ f(x) = yt(x) (define (valid-ir-argument? e) (or (simple-atom? e) (symbol? e) (and (pair? e) - (memq (car e) '(quote inert top core globalref outerref + (memq (car e) '(quote inert top core outerref slot static_parameter))))) (define (valid-ir-rvalue? lhs e) @@ -5006,6 +5006,15 @@ f(x) = yt(x) (set! code (cons e code)) (set! i (+ i 1)) (set! locs (cons current-loc locs))) + (define (maybe-outline-outerref e) + (if (and (outerref? e) (not (nothrow-julia-global (cadr e)))) + (let ((ssav (make-ssavalue))) + (put! ssavtable (cadr ssav) i) + (emit e) + ssav) + e)) + (define (maybe-outline-outerref-expr e) + (cons (car e) (map maybe-outline-outerref (cdr e)))) (let loop ((stmts (cdr body))) (if (pair? stmts) (let ((e (car stmts))) @@ -5045,11 +5054,14 @@ f(x) = yt(x) ;; if both lhs and rhs are ssavalues, merge them (if idx (put! ssavtable (cadr (cadr e)) idx) - (begin + (let* ((e_rhs (caddr e)) + (rhs (if (and (pair? e_rhs) (eq? (car e_rhs) 'call)) (maybe-outline-outerref-expr e_rhs) e_rhs))) (put! ssavtable (cadr (cadr e)) i) - (emit (caddr e)))))) + (emit rhs))))) + ((eq? (car e) 'call) + (emit (maybe-outline-outerref-expr e))) (else - (emit e))) + (emit e))) (loop (cdr stmts))))) (vector (reverse code) (reverse locs) (reverse linetable) ssavtable labltable))) diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 361d004a7a92d..3e3b35324cb89 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -2998,6 +2998,8 @@ end @test ig27907(Int, Int, 1, 0) == 0 # issue #28279 +# ensure that lowering doesn't move these into statement position, which would require renumbering +using Base: +, - function f28279(b::Bool) i = 1 while i > b diff --git a/test/core.jl b/test/core.jl index 72b9f435d21a1..d113399e5889f 100644 --- a/test/core.jl +++ b/test/core.jl @@ -7356,22 +7356,23 @@ let fc = FieldConvert(1.0, [2.0], 0x3, 0x4, 0x5) @test fc.e === UInt32(0x5) end @test ftype_eval[] == 1 -let code = code_lowered(FieldConvert)[1].code - @test code[1] == Expr(:call, GlobalRef(Core, :apply_type), GlobalRef(@__MODULE__, :FieldConvert), GlobalRef(@__MODULE__, :FieldTypeA), Expr(:static_parameter, 1)) - @test code[2] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(1), 1) - @test code[7] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(2), Core.SlotNumber(10))) - @test code[8] == Core.SlotNumber(10) - @test code[9] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(1), 2) - @test code[14] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(9), Core.SlotNumber(9))) - @test code[15] == Core.SlotNumber(9) - @test code[16] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(1), 4) - @test code[21] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(16), Core.SlotNumber(8))) - @test code[22] == Core.SlotNumber(8) - @test code[23] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(1), 5) - @test code[28] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(23), Core.SlotNumber(7))) - @test code[29] == Core.SlotNumber(7) - @test code[30] == Expr(:new, Core.SSAValue(1), Core.SSAValue(8), Core.SSAValue(15), Core.SlotNumber(4), Core.SSAValue(22), Core.SSAValue(29)) - @test code[31] == Core.ReturnNode(Core.SSAValue(30)) +let code = code_lowered(FieldConvert)[1].code, fc_ssa = 2 + @test code[1] == GlobalRef(@__MODULE__, :FieldConvert) + @test code[2] == Expr(:call, GlobalRef(Core, :apply_type), Core.SSAValue(1), GlobalRef(@__MODULE__, :FieldTypeA), Expr(:static_parameter, 1)) + @test code[3] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 1) + @test code[8] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(3), Core.SlotNumber(10))) + @test code[9] == Core.SlotNumber(10) + @test code[10] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 2) + @test code[15] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(10), Core.SlotNumber(9))) + @test code[16] == Core.SlotNumber(9) + @test code[17] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 4) + @test code[22] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(17), Core.SlotNumber(8))) + @test code[23] == Core.SlotNumber(8) + @test code[24] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 5) + @test code[29] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(24), Core.SlotNumber(7))) + @test code[30] == Core.SlotNumber(7) + @test code[31] == Expr(:new, Core.SSAValue(fc_ssa), Core.SSAValue(9), Core.SSAValue(16), Core.SlotNumber(4), Core.SSAValue(23), Core.SSAValue(30)) + @test code[32] == Core.ReturnNode(Core.SSAValue(31)) end # Issue #32820 From 8aa5302c375b389771b1adeba59f44dd9acf3879 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 2 Nov 2023 05:13:08 +0000 Subject: [PATCH 03/12] interpreter: Fix stacktrace for global incorrectly assumed to be in phi block We don't fully set up the interpreter state when evaluating the phi block, because we expect all statements to be valid in value-position (using the IR definition of value position). However, the interpreter was overapproximating the size of the phi-block, leading to the possibility that the evaluation may throw. Correct that by truncating the phi block to the actual last phi. Will be tested in existing tests after #51970. --- src/julia-syntax.scm | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index bd1ba6ce1863d..6afb8afaffbea 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4255,7 +4255,7 @@ f(x) = yt(x) (define (valid-ir-argument? e) (or (simple-atom? e) (symbol? e) (and (pair? e) - (memq (car e) '(quote inert top core outerref + (memq (car e) '(quote inert top core globalref outerref slot static_parameter))))) (define (valid-ir-rvalue? lhs e) @@ -5006,15 +5006,6 @@ f(x) = yt(x) (set! code (cons e code)) (set! i (+ i 1)) (set! locs (cons current-loc locs))) - (define (maybe-outline-outerref e) - (if (and (outerref? e) (not (nothrow-julia-global (cadr e)))) - (let ((ssav (make-ssavalue))) - (put! ssavtable (cadr ssav) i) - (emit e) - ssav) - e)) - (define (maybe-outline-outerref-expr e) - (cons (car e) (map maybe-outline-outerref (cdr e)))) (let loop ((stmts (cdr body))) (if (pair? stmts) (let ((e (car stmts))) @@ -5054,14 +5045,11 @@ f(x) = yt(x) ;; if both lhs and rhs are ssavalues, merge them (if idx (put! ssavtable (cadr (cadr e)) idx) - (let* ((e_rhs (caddr e)) - (rhs (if (and (pair? e_rhs) (eq? (car e_rhs) 'call)) (maybe-outline-outerref-expr e_rhs) e_rhs))) + (begin (put! ssavtable (cadr (cadr e)) i) - (emit rhs))))) - ((eq? (car e) 'call) - (emit (maybe-outline-outerref-expr e))) + (emit (caddr e)))))) (else - (emit e))) + (emit e))) (loop (cdr stmts))))) (vector (reverse code) (reverse locs) (reverse linetable) ssavtable labltable))) From 74173509fe2e6b2bf6104128dc75dc0c1b02ce3a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 2 Nov 2023 05:22:27 +0000 Subject: [PATCH 04/12] WIP --- base/compiler/abstractinterpretation.jl | 16 +++-- src/ast.c | 4 +- src/julia-syntax.scm | 79 ++++++++++++++----------- test/compiler/inference.jl | 11 ++-- test/core.jl | 39 ++++++------ test/syntax.jl | 2 +- 6 files changed, 85 insertions(+), 66 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 21ff1c951bf85..d7fa58212d195 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -320,7 +320,7 @@ function from_interprocedural!(interp::AbstractInterpreter, @nospecialize(rt), s arginfo::ArgInfo, @nospecialize(maybecondinfo)) rt = collect_limitations!(rt, sv) if isa(rt, InterMustAlias) - rt = from_intermustalias(rt, arginfo) + rt = from_intermustalias(rt, arginfo, sv) elseif is_lattice_bool(ipo_lattice(interp), rt) if maybecondinfo === nothing rt = widenconditional(rt) @@ -340,10 +340,10 @@ function collect_limitations!(@nospecialize(typ), sv::InferenceState) return typ end -function from_intermustalias(rt::InterMustAlias, arginfo::ArgInfo) +function from_intermustalias(rt::InterMustAlias, arginfo::ArgInfo, sv::AbsIntState) fargs = arginfo.fargs if fargs !== nothing && 1 โ‰ค rt.slot โ‰ค length(fargs) - arg = fargs[rt.slot] + arg = ssa_def_slot(fargs[rt.slot], sv) if isa(arg, SlotNumber) argtyp = widenslotwrapper(arginfo.argtypes[rt.slot]) if rt.vartyp โŠ‘ argtyp @@ -1334,6 +1334,9 @@ function ssa_def_slot(@nospecialize(arg), sv::InferenceState) return arg end +# No slots in irinterp +ssa_def_slot(@nospecialize(arg), sv::IRInterpretationState) = nothing + struct AbstractIterationResult cti::Vector{Any} info::MaybeAbstractIterationInfo @@ -1743,7 +1746,7 @@ function abstract_call_builtin(interp::AbstractInterpreter, f::Builtin, (; fargs a3 = argtypes[3] if isa(a3, Const) if rt !== Bottom && !isalreadyconst(rt) - var = fargs[2] + var = ssa_def_slot(fargs[2], sv) if isa(var, SlotNumber) vartyp = widenslotwrapper(argtypes[2]) fldidx = maybe_const_fldidx(vartyp, a3.val) @@ -3047,6 +3050,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState) @goto branch elseif isa(stmt, GotoIfNot) condx = stmt.cond + condxslot = ssa_def_slot(condx, frame) condt = abstract_eval_value(interp, condx, currstate, frame) if condt === Bottom ssavaluetypes[currpc] = Bottom @@ -3054,10 +3058,10 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState) @goto find_next_bb end orig_condt = condt - if !(isa(condt, Const) || isa(condt, Conditional)) && isa(condx, SlotNumber) + if !(isa(condt, Const) || isa(condt, Conditional)) && isa(condxslot, SlotNumber) # if this non-`Conditional` object is a slot, we form and propagate # the conditional constraint on it - condt = Conditional(condx, Const(true), Const(false)) + condt = Conditional(condxslot, Const(true), Const(false)) end condval = maybe_extract_const_bool(condt) nothrow = (condval !== nothing) || โŠ‘(๐•ƒแตข, orig_condt, Bool) diff --git a/src/ast.c b/src/ast.c index 2ba5f89a2cb27..da47d69524366 100644 --- a/src/ast.c +++ b/src/ast.c @@ -171,8 +171,8 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global"); jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); - jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); - return b != NULL ? fl_ctx->T : fl_ctx->F; + jl_binding_t *b = jl_get_binding_if_bound(ctx->module, var); + return b != NULL && b->value != NULL ? fl_ctx->T : fl_ctx->F; } static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 6afb8afaffbea..91fd6763391c6 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4252,10 +4252,13 @@ f(x) = yt(x) (else (for-each linearize (cdr e)))) e) +;; N.B.: This assumes that resolve-scopes has run, so outerref is equivalent to +;; a global in the current scope. (define (valid-ir-argument? e) - (or (simple-atom? e) (symbol? e) + (or (simple-atom? e) + (and (outerref? e) (nothrow-julia-global (cadr e))) (and (pair? e) - (memq (car e) '(quote inert top core globalref outerref + (memq (car e) '(quote inert top core slot static_parameter))))) (define (valid-ir-rvalue? lhs e) @@ -4408,48 +4411,54 @@ f(x) = yt(x) (else (string "\"" h "\" expression")))) (if (not (null? (cadr lam))) (error (string (head-to-text (car e)) " not at top level")))) + (define (valid-body-ir-argument? aval) + (or (valid-ir-argument? aval) + (and (symbol? aval) ; Arguments are always defined slots + (or (memq aval (lam:args lam)) + (let ((vi (get vinfo-table aval #f))) + (and vi (vinfo:never-undef vi))))))) + (define (single-assign-var? aval) + (and (symbol? aval) ; Arguments are always sa + (or (memq aval (lam:args lam)) + (let ((vi (get vinfo-table aval #f))) + (and vi (vinfo:sa vi)))))) ;; evaluate the arguments of a call, creating temporary locations as needed (define (compile-args lst break-labels) (if (null? lst) '() - (let ((simple? (every (lambda (x) (or (simple-atom? x) (symbol? x) - (and (pair? x) - (memq (car x) '(quote inert top core globalref outerref))))) - lst))) - (let loop ((lst lst) - (vals '())) - (if (null? lst) - (reverse! vals) - (let* ((arg (car lst)) - (aval (or (compile arg break-labels #t #f) - ;; TODO: argument exprs that don't yield a value? - '(null)))) - (loop (cdr lst) - (cons (if (and (not simple?) - (not (simple-atom? arg)) - (not (simple-atom? aval)) - (not (and (pair? arg) - (memq (car arg) '(quote inert top core)))) - (not (and (symbol? aval) ;; function args are immutable and always assigned - (memq aval (lam:args lam)))) - (not (and (or (symbol? arg) - (and (pair? arg) - (memq (car arg) '(globalref outerref)))) - (or (null? (cdr lst)) - (null? vals))))) - (let ((tmp (make-ssavalue))) - (emit `(= ,tmp ,aval)) - tmp) - aval) - vals)))))))) + ;; First check if all the arguments as simple (and therefore side-effect free). + ;; Otherwise, we need to use ssa values for all arguments to ensure proper + ;; left-to-right evaluation semantics. + (let ((simple? (every (lambda (x) (or (simple-atom? x) (symbol? x) + (and (pair? x) + (memq (car x) '(quote inert top core globalref outerref))))) + lst))) + (let loop ((lst lst) + (vals '())) + (if (null? lst) + (reverse! vals) + (let* ((arg (car lst)) + (aval (or (compile arg break-labels #t #f) + ;; TODO: argument exprs that don't yield a value? + '(null)))) + (loop (cdr lst) + (cons (if (and + ;; Even if we have side effects, we know that singly-assigned + ;; locals cannot be affected them, so we can inline them anyway. + (or simple? (single-assign-var? aval)) + (valid-body-ir-argument? aval)) + aval + (let ((tmp (make-ssavalue))) + (emit `(= ,tmp ,aval)) + tmp)) + vals)))))))) (define (compile-cond ex break-labels) (let ((cnd (or (compile ex break-labels #t #f) ;; TODO: condition exprs that don't yield a value? '(null)))) - (if (not (valid-ir-argument? cnd)) + (if (valid-body-ir-argument? cnd) cnd (let ((tmp (make-ssavalue))) (emit `(= ,tmp ,cnd)) - tmp) - cnd))) + tmp)))) (define (emit-cond cnd break-labels endl) (let* ((cnd (if (and (pair? cnd) (eq? (car cnd) 'block)) (flatten-ex 'block cnd) diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 3e3b35324cb89..ed23da715da69 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -3001,12 +3001,13 @@ end # ensure that lowering doesn't move these into statement position, which would require renumbering using Base: +, - function f28279(b::Bool) - i = 1 - while i > b - i -= 1 + let i = 1 + while i > b + i -= 1 + end + if b end + return i + 1 end - if b end - return i + 1 end code28279 = code_lowered(f28279, (Bool,))[1].code oldcode28279 = deepcopy(code28279) diff --git a/test/core.jl b/test/core.jl index d113399e5889f..d8eb682bbdfcf 100644 --- a/test/core.jl +++ b/test/core.jl @@ -7356,23 +7356,28 @@ let fc = FieldConvert(1.0, [2.0], 0x3, 0x4, 0x5) @test fc.e === UInt32(0x5) end @test ftype_eval[] == 1 -let code = code_lowered(FieldConvert)[1].code, fc_ssa = 2 - @test code[1] == GlobalRef(@__MODULE__, :FieldConvert) - @test code[2] == Expr(:call, GlobalRef(Core, :apply_type), Core.SSAValue(1), GlobalRef(@__MODULE__, :FieldTypeA), Expr(:static_parameter, 1)) - @test code[3] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 1) - @test code[8] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(3), Core.SlotNumber(10))) - @test code[9] == Core.SlotNumber(10) - @test code[10] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 2) - @test code[15] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(10), Core.SlotNumber(9))) - @test code[16] == Core.SlotNumber(9) - @test code[17] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 4) - @test code[22] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(17), Core.SlotNumber(8))) - @test code[23] == Core.SlotNumber(8) - @test code[24] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(fc_ssa), 5) - @test code[29] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(24), Core.SlotNumber(7))) - @test code[30] == Core.SlotNumber(7) - @test code[31] == Expr(:new, Core.SSAValue(fc_ssa), Core.SSAValue(9), Core.SSAValue(16), Core.SlotNumber(4), Core.SSAValue(23), Core.SSAValue(30)) - @test code[32] == Core.ReturnNode(Core.SSAValue(31)) +let code = code_lowered(FieldConvert)[1].code + local fc_global_ssa, sp1_ssa, apply_type_ssa, field_type_ssa, + field_type2_ssa, field_type4_ssa, field_type5_ssa, + slot_read_1, slot_read_2, slot_read_3, slot_read_4, + new_ssa + @test code[(fc_global_ssa = 1;)] == GlobalRef(@__MODULE__, :FieldConvert) + @test code[(sp1_ssa = 2;)] == Expr(:static_parameter, 1) + @test code[(apply_type_ssa = 3;)] == Expr(:call, GlobalRef(Core, :apply_type), Core.SSAValue(fc_global_ssa), GlobalRef(@__MODULE__, :FieldTypeA), Core.SSAValue(sp1_ssa)) + @test code[(field_type_ssa = 4;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 1) + @test code[10] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type_ssa), Core.SlotNumber(10))) + @test code[(slot_read_1 = 11;)] == Core.SlotNumber(10) + @test code[(field_type2_ssa = 12;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 2) + @test code[18] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type2_ssa), Core.SlotNumber(9))) + @test code[(slot_read_2 = 19;)] == Core.SlotNumber(9) + @test code[(field_type4_ssa = 20;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 4) + @test code[26] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type4_ssa), Core.SlotNumber(8))) + @test code[(slot_read_3 = 27;)] == Core.SlotNumber(8) + @test code[(field_type5_ssa = 28;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 5) + @test code[34] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type5_ssa), Core.SlotNumber(7))) + @test code[(slot_read_4 = 35;)] == Core.SlotNumber(7) + @test code[(new_ssa = 36;)] == Expr(:new, Core.SSAValue(apply_type_ssa), Core.SSAValue(slot_read_1), Core.SSAValue(slot_read_2), Core.SlotNumber(4), Core.SSAValue(slot_read_3), Core.SSAValue(slot_read_4)) + @test code[37] == Core.ReturnNode(Core.SSAValue(new_ssa)) end # Issue #32820 diff --git a/test/syntax.jl b/test/syntax.jl index 106a8f296ced2..1c5d0c92d28f5 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -553,7 +553,7 @@ for (str, tag) in Dict("" => :none, "\"" => :string, "#=" => :comment, "'" => :c end # meta nodes for optional positional arguments -let src = Meta.lower(Main, :(@inline f(p::Int=2) = 3)).args[1].code[end-1].args[3] +let src = Meta.lower(Main, :(@inline f(p::Int=2) = 3)).args[1].code[end-2].args[3] @test Core.Compiler.is_declared_inline(src) end From 8f28497694476118a3d0455ef76ffe32df29c89a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 2 Nov 2023 07:51:25 +0000 Subject: [PATCH 05/12] WIP --- src/toplevel.c | 11 +++++++---- test/meta.jl | 10 +++++----- test/syntax.jl | 11 +++++++++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/toplevel.c b/src/toplevel.c index 5b67ab6061c51..750f34aaa9fed 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -352,7 +352,7 @@ JL_DLLEXPORT jl_module_t *jl_base_relative_to(jl_module_t *m) return jl_top_module; } -static void expr_attributes(jl_value_t *v, int *has_ccall, int *has_defs, int *has_opaque) +static void expr_attributes(jl_value_t *v, jl_array_t *body, int *has_ccall, int *has_defs, int *has_opaque) { if (!jl_is_expr(v)) return; @@ -390,6 +390,9 @@ static void expr_attributes(jl_value_t *v, int *has_ccall, int *has_defs, int *h else if (head == jl_call_sym && jl_expr_nargs(e) > 0) { jl_value_t *called = NULL; jl_value_t *f = jl_exprarg(e, 0); + if (jl_is_ssavalue(f)) { + f = jl_array_ptr_ref(body, ((jl_ssavalue_t*)f)->id - 1); + } if (jl_is_globalref(f)) { jl_module_t *mod = jl_globalref_mod(f); jl_sym_t *name = jl_globalref_name(f); @@ -417,7 +420,7 @@ static void expr_attributes(jl_value_t *v, int *has_ccall, int *has_defs, int *h for (i = 0; i < jl_array_nrows(e->args); i++) { jl_value_t *a = jl_exprarg(e, i); if (jl_is_expr(a)) - expr_attributes(a, has_ccall, has_defs, has_opaque); + expr_attributes(a, body, has_ccall, has_defs, has_opaque); } } @@ -431,7 +434,7 @@ int jl_code_requires_compiler(jl_code_info_t *src, int include_force_compile) return 1; for(i=0; i < jl_array_nrows(body); i++) { jl_value_t *stmt = jl_array_ptr_ref(body,i); - expr_attributes(stmt, &has_ccall, &has_defs, &has_opaque); + expr_attributes(stmt, body, &has_ccall, &has_defs, &has_opaque); if (has_ccall) return 1; } @@ -454,7 +457,7 @@ static void body_attributes(jl_array_t *body, int *has_ccall, int *has_defs, int *has_loops = 1; } } - expr_attributes(stmt, has_ccall, has_defs, has_opaque); + expr_attributes(stmt, body, has_ccall, has_defs, has_opaque); } *forced_compile = jl_has_meta(body, jl_force_compile_sym); } diff --git a/test/meta.jl b/test/meta.jl index 36a8acbfe08dd..856462fedd9fa 100644 --- a/test/meta.jl +++ b/test/meta.jl @@ -254,14 +254,14 @@ end f(::T) where {T} = T ci = code_lowered(f, Tuple{Int})[1] @test Meta.partially_inline!(ci.code, [], Tuple{typeof(f),Int}, Any[Int], 0, 0, :propagate) == - Any[Core.ReturnNode(QuoteNode(Int))] + Any[QuoteNode(Int), Core.ReturnNode(Core.SSAValue(1))] g(::Val{x}) where {x} = x ? 1 : 0 ci = code_lowered(g, Tuple{Val{true}})[1] -@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 0, :propagate)[1] == - Core.GotoIfNot(QuoteNode(true), 3) -@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 2, :propagate)[1] == - Core.GotoIfNot(QuoteNode(true), 5) +@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 0, :propagate)[2] == + Core.GotoIfNot(Core.SSAValue(1), 4) +@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 2, :propagate)[2] == + Core.GotoIfNot(Core.SSAValue(3), 6) @testset "inlining with isdefined" begin isdefined_slot(x) = @isdefined(x) diff --git a/test/syntax.jl b/test/syntax.jl index 1c5d0c92d28f5..4d204f3e29364 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -713,7 +713,7 @@ m1_exprs = get_expr_list(Meta.lower(@__MODULE__, quote @m1 end)) let low3 = Meta.lower(@__MODULE__, quote @m3 end) m3_exprs = get_expr_list(low3) ci = low3.args[1]::Core.CodeInfo - @test ci.codelocs == [4, 2] + @test ci.codelocs in ([4, 4, 2], [4, 2]) @test is_return_ssavalue(m3_exprs[end]) end @@ -2511,7 +2511,14 @@ end function ncalls_in_lowered(ex, fname) lowered_exprs = Meta.lower(Main, ex).args[1].code return count(lowered_exprs) do ex - Meta.isexpr(ex, :call) && ex.args[1] == fname + if Meta.isexpr(ex, :call) + arg = ex.args[1] + if isa(arg, Core.SSAValue) + arg = lowered_exprs[arg.id] + end + return arg == fname + end + return false end end From e30f3403c0c6d8bea63cc0dbe8e958b9d3f57673 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 2 Nov 2023 08:20:43 +0000 Subject: [PATCH 06/12] WIP --- base/compiler/utilities.jl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/base/compiler/utilities.jl b/base/compiler/utilities.jl index f46f9f19ee629..3d1fe35453060 100644 --- a/base/compiler/utilities.jl +++ b/base/compiler/utilities.jl @@ -446,9 +446,12 @@ function find_ssavalue_uses(e::PhiNode, uses::Vector{BitSet}, line::Int) end end -function is_throw_call(e::Expr) +function is_throw_call(e::Expr, code::Vector{Any}) if e.head === :call f = e.args[1] + if isa(f, SSAValue) + f = code[f.id] + end if isa(f, GlobalRef) ff = abstract_eval_globalref_type(f) if isa(ff, Const) && ff.val === Core.throw @@ -478,7 +481,7 @@ function find_throw_blocks(code::Vector{Any}, handler_at::Vector{Int}) end elseif s.head === :return # see `ReturnNode` handling - elseif is_throw_call(s) + elseif is_throw_call(s, code) if handler_at[i] == 0 push!(stmts, i) end From 14c848dfcce9d7b743a404a70254c0cccf24443d Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Fri, 3 Nov 2023 03:59:20 +0000 Subject: [PATCH 07/12] Fix various misc checks --- doc/src/base/reflection.md | 11 ++++++----- src/ast.c | 2 +- stdlib/Test/src/Test.jl | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/src/base/reflection.md b/doc/src/base/reflection.md index e5c70175e5f9a..945f469cb474b 100644 --- a/doc/src/base/reflection.md +++ b/doc/src/base/reflection.md @@ -97,14 +97,15 @@ Finally, the [`Meta.lower`](@ref) function gives the `lowered` form of any expre particular interest for understanding how language constructs map to primitive operations such as assignments, branches, and calls: -```jldoctest +```jldoctest; setup = (using Base: +, sin) julia> Meta.lower(@__MODULE__, :( [1+2, sin(0.5)] )) :($(Expr(:thunk, CodeInfo( @ none within `top-level scope` -1 โ”€ %1 = 1 + 2 -โ”‚ %2 = sin(0.5) -โ”‚ %3 = Base.vect(%1, %2) -โ””โ”€โ”€ return %3 +1 โ”€ %1 = Base.vect +โ”‚ %2 = 1 + 2 +โ”‚ %3 = sin(0.5) +โ”‚ %4 = (%1)(%2, %3) +โ””โ”€โ”€ return %4 )))) ``` diff --git a/src/ast.c b/src/ast.c index da47d69524366..20e99bdab99b3 100644 --- a/src/ast.c +++ b/src/ast.c @@ -172,7 +172,7 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); jl_binding_t *b = jl_get_binding_if_bound(ctx->module, var); - return b != NULL && b->value != NULL ? fl_ctx->T : fl_ctx->F; + return b != NULL && jl_atomic_load_relaxed(&b->value) != NULL ? fl_ctx->T : fl_ctx->F; } static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT diff --git a/stdlib/Test/src/Test.jl b/stdlib/Test/src/Test.jl index 7e416df91864e..9742975d7c464 100644 --- a/stdlib/Test/src/Test.jl +++ b/stdlib/Test/src/Test.jl @@ -1814,7 +1814,7 @@ matches the inferred type modulo `AllowedType`, or when the return type is a sub `AllowedType`. This is useful when testing type stability of functions returning a small union such as `Union{Nothing, T}` or `Union{Missing, T}`. -```jldoctest; setup = :(using InteractiveUtils), filter = r"begin\\n(.|\\n)*end" +```jldoctest; setup = :(using InteractiveUtils; using Base: >), filter = r"begin\\n(.|\\n)*end" julia> f(a) = a > 1 ? 1 : 1.0 f (generic function with 1 method) From 97bff513844b01b050fbd2e458d5b627064a01ae Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Tue, 7 Nov 2023 17:03:43 -0500 Subject: [PATCH 08/12] fix some indenting --- src/julia-syntax.scm | 46 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 91fd6763391c6..824a8e8e543bb 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4413,15 +4413,15 @@ f(x) = yt(x) (error (string (head-to-text (car e)) " not at top level")))) (define (valid-body-ir-argument? aval) (or (valid-ir-argument? aval) - (and (symbol? aval) ; Arguments are always defined slots - (or (memq aval (lam:args lam)) - (let ((vi (get vinfo-table aval #f))) - (and vi (vinfo:never-undef vi))))))) + (and (symbol? aval) ; Arguments are always defined slots + (or (memq aval (lam:args lam)) + (let ((vi (get vinfo-table aval #f))) + (and vi (vinfo:never-undef vi))))))) (define (single-assign-var? aval) (and (symbol? aval) ; Arguments are always sa - (or (memq aval (lam:args lam)) - (let ((vi (get vinfo-table aval #f))) - (and vi (vinfo:sa vi)))))) + (or (memq aval (lam:args lam)) + (let ((vi (get vinfo-table aval #f))) + (and vi (vinfo:sa vi)))))) ;; evaluate the arguments of a call, creating temporary locations as needed (define (compile-args lst break-labels) (if (null? lst) '() @@ -4429,28 +4429,28 @@ f(x) = yt(x) ;; Otherwise, we need to use ssa values for all arguments to ensure proper ;; left-to-right evaluation semantics. (let ((simple? (every (lambda (x) (or (simple-atom? x) (symbol? x) - (and (pair? x) - (memq (car x) '(quote inert top core globalref outerref))))) - lst))) + (and (pair? x) + (memq (car x) '(quote inert top core globalref outerref))))) + lst))) (let loop ((lst lst) - (vals '())) + (vals '())) (if (null? lst) (reverse! vals) (let* ((arg (car lst)) - (aval (or (compile arg break-labels #t #f) - ;; TODO: argument exprs that don't yield a value? - '(null)))) + (aval (or (compile arg break-labels #t #f) + ;; TODO: argument exprs that don't yield a value? + '(null)))) (loop (cdr lst) (cons (if (and - ;; Even if we have side effects, we know that singly-assigned - ;; locals cannot be affected them, so we can inline them anyway. - (or simple? (single-assign-var? aval)) - (valid-body-ir-argument? aval)) - aval - (let ((tmp (make-ssavalue))) - (emit `(= ,tmp ,aval)) - tmp)) - vals)))))))) + ;; Even if we have side effects, we know that singly-assigned + ;; locals cannot be affected them, so we can inline them anyway. + (or simple? (single-assign-var? aval)) + (valid-body-ir-argument? aval)) + aval + (let ((tmp (make-ssavalue))) + (emit `(= ,tmp ,aval)) + tmp)) + vals)))))))) (define (compile-cond ex break-labels) (let ((cnd (or (compile ex break-labels #t #f) ;; TODO: condition exprs that don't yield a value? From 688b1e1c3d38482f3b4c97666e1b3e7c13b3d733 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 9 Nov 2023 03:14:57 +0000 Subject: [PATCH 09/12] Fix rebase --- src/ast.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ast.c b/src/ast.c index 20e99bdab99b3..f18eb2713cc40 100644 --- a/src/ast.c +++ b/src/ast.c @@ -171,7 +171,8 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global"); jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); - jl_binding_t *b = jl_get_binding_if_bound(ctx->module, var); + jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); + b = b ? jl_atomic_load_relaxed(&b->owner) : NULL; return b != NULL && jl_atomic_load_relaxed(&b->value) != NULL ? fl_ctx->T : fl_ctx->F; } From a0c6204cba2f8a844c29ae92d364badd01cfda4a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 9 Nov 2023 04:20:26 +0000 Subject: [PATCH 10/12] Address review comments --- src/ast.c | 55 +++++++++++++++++++++++++------------- src/julia-syntax.scm | 14 +++++++--- test/compiler/inference.jl | 5 ++++ test/core.jl | 12 +++++++++ 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/ast.c b/src/ast.c index f18eb2713cc40..3a4b84044eea9 100644 --- a/src/ast.c +++ b/src/ast.c @@ -153,13 +153,26 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo static value_t julia_to_scm(fl_context_t *fl_ctx, jl_value_t *v); static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel, size_t world, int throw_load_error); +static jl_sym_t *scmsym_to_julia(fl_context_t *fl_ctx, value_t s) +{ + assert(issymbol(s)); + if (fl_isgensym(fl_ctx, s)) { + char gsname[16]; + char *n = uint2str(&gsname[1], sizeof(gsname)-1, + ((gensym_t*)ptr(s))->id, 10); + *(--n) = '#'; + return jl_symbol(n); + } + return jl_symbol(symbol_name(fl_ctx, s)); +} + static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) { // tells whether a var is defined in and *by* the current module argcount(fl_ctx, "defined-julia-global", nargs, 1); (void)tosymbol(fl_ctx, args[0], "defined-julia-global"); jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); - jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); + jl_sym_t *var = scmsym_to_julia(fl_ctx, args[0]); jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); return (b != NULL && jl_atomic_load_relaxed(&b->owner) == b) ? fl_ctx->T : fl_ctx->F; } @@ -167,11 +180,29 @@ static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) { // tells whether a var is defined, in the sense that accessing it is nothrow - argcount(fl_ctx, "nothrow-julia-global", nargs, 1); - (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global"); + // can take either a symbol or a module and a symbol jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); - jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0])); - jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); + jl_module_t *mod = ctx->module; + jl_sym_t *var = NULL; + if (nargs == 1) { + (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global"); + var = scmsym_to_julia(fl_ctx, args[0]); + } + else { + argcount(fl_ctx, "nothrow-julia-global", nargs, 2); + value_t argmod = args[0]; + if (iscvalue(argmod) && cv_class((cvalue_t*)ptr(argmod)) == jl_ast_ctx(fl_ctx)->jvtype) { + mod = *(jl_module_t**)cv_data((cvalue_t*)ptr(argmod)); + } else { + (void)tosymbol(fl_ctx, argmod, "nothrow-julia-global"); + if (scmsym_to_julia(fl_ctx, argmod) != jl_thismodule_sym) { + lerrorf(fl_ctx, fl_ctx->ArgError, "nothrow-julia-global: Unknown globalref module kind"); + } + } + (void)tosymbol(fl_ctx, args[1], "nothrow-julia-global"); + var = scmsym_to_julia(fl_ctx, args[1]); + } + jl_binding_t *b = jl_get_module_binding(mod, var, 0); b = b ? jl_atomic_load_relaxed(&b->owner) : NULL; return b != NULL && jl_atomic_load_relaxed(&b->value) != NULL ? fl_ctx->T : fl_ctx->F; } @@ -434,20 +465,6 @@ JL_DLLEXPORT void fl_profile(const char *fname) jl_ast_ctx_leave(ctx); } - -static jl_sym_t *scmsym_to_julia(fl_context_t *fl_ctx, value_t s) -{ - assert(issymbol(s)); - if (fl_isgensym(fl_ctx, s)) { - char gsname[16]; - char *n = uint2str(&gsname[1], sizeof(gsname)-1, - ((gensym_t*)ptr(s))->id, 10); - *(--n) = '#'; - return jl_symbol(n); - } - return jl_symbol(symbol_name(fl_ctx, s)); -} - static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mod) { jl_value_t *v = NULL; diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 824a8e8e543bb..ceb567f7f2eb9 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4257,6 +4257,7 @@ f(x) = yt(x) (define (valid-ir-argument? e) (or (simple-atom? e) (and (outerref? e) (nothrow-julia-global (cadr e))) + (and (globalref? e) (nothrow-julia-global (cadr e) (caddr e))) (and (pair? e) (memq (car e) '(quote inert top core slot static_parameter))))) @@ -4265,7 +4266,7 @@ f(x) = yt(x) (or (ssavalue? lhs) (valid-ir-argument? e) (and (symbol? lhs) (pair? e) - (memq (car e) '(new splatnew the_exception isdefined call invoke foreigncall cfunction gc_preserve_begin copyast new_opaque_closure))))) + (memq (car e) '(new splatnew the_exception isdefined call invoke foreigncall cfunction gc_preserve_begin copyast new_opaque_closure globalref outerref))))) (define (valid-ir-return? e) ;; returning lambda directly is needed for @generated @@ -4422,6 +4423,13 @@ f(x) = yt(x) (or (memq aval (lam:args lam)) (let ((vi (get vinfo-table aval #f))) (and vi (vinfo:sa vi)))))) + ;; TODO: We could also allow const globals here + (define (const-read-arg? x) + ;; Even if we have side effects, we know that singly-assigned + ;; locals cannot be affected them, so we can inline them anyway. + (or (simple-atom? x) (single-assign-var? x) + (and (pair? x) + (memq (car x) '(quote inert top core))))) ;; evaluate the arguments of a call, creating temporary locations as needed (define (compile-args lst break-labels) (if (null? lst) '() @@ -4442,9 +4450,7 @@ f(x) = yt(x) '(null)))) (loop (cdr lst) (cons (if (and - ;; Even if we have side effects, we know that singly-assigned - ;; locals cannot be affected them, so we can inline them anyway. - (or simple? (single-assign-var? aval)) + (or simple? (const-read-arg? aval)) (valid-body-ir-argument? aval)) aval (let ((tmp (make-ssavalue))) diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index ed23da715da69..b057786282973 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -5491,6 +5491,11 @@ end @test Base.return_types(phic_type10) |> only === Int @test phic_type10() === 2 +undef_trycatch() = try (a_undef_trycatch = a_undef_trycatch, b = 2); return 1 catch end +# `global a_undef_trycatch` could be defined dynamically, so both paths must be allowed +@test Base.return_types(undef_trycatch) |> only === Union{Nothing, Int} +@test undef_trycatch() === nothing + # Test that `exit` returns `Union{}` (issue #51856) function test_exit_bottom(s) n = tryparse(Int, s) diff --git a/test/core.jl b/test/core.jl index d8eb682bbdfcf..505c5d03a6484 100644 --- a/test/core.jl +++ b/test/core.jl @@ -8048,3 +8048,15 @@ end let lin = Core.LineInfoNode(Base, first(methods(convert)), :foo, Int32(5), Int32(0)) @test convert(LineNumberNode, lin) == LineNumberNode(5, :foo) end + +# Test that a nothrow-globalref doesn't get outlined during lowering +module WellKnownGlobal + global well_known = 1 +end +macro insert_global() + Expr(:call, GlobalRef(Base, :println), GlobalRef(WellKnownGlobal, :well_known)) +end +check_globalref_lowering() = @insert_global +let src = code_lowered(check_globalref_lowering)[1] + @test length(src.code) == 2 +end From 7fa80c1d5f065e92f0aec01d32547252e73bc390 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 9 Nov 2023 04:29:32 +0000 Subject: [PATCH 11/12] Add GC root annotation --- src/ast.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ast.c b/src/ast.c index 3a4b84044eea9..e6adbdc4d5aac 100644 --- a/src/ast.c +++ b/src/ast.c @@ -193,6 +193,7 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint value_t argmod = args[0]; if (iscvalue(argmod) && cv_class((cvalue_t*)ptr(argmod)) == jl_ast_ctx(fl_ctx)->jvtype) { mod = *(jl_module_t**)cv_data((cvalue_t*)ptr(argmod)); + JL_GC_PROMISE_ROOTED(mod); } else { (void)tosymbol(fl_ctx, argmod, "nothrow-julia-global"); if (scmsym_to_julia(fl_ctx, argmod) != jl_thismodule_sym) { From 3ec0c87a7dbb2b425f9a60b843d7e0294a867a7d Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 9 Nov 2023 06:42:46 +0000 Subject: [PATCH 12/12] Update doctest --- doc/src/base/reflection.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/src/base/reflection.md b/doc/src/base/reflection.md index 945f469cb474b..d44bc474abbd2 100644 --- a/doc/src/base/reflection.md +++ b/doc/src/base/reflection.md @@ -101,11 +101,10 @@ as assignments, branches, and calls: julia> Meta.lower(@__MODULE__, :( [1+2, sin(0.5)] )) :($(Expr(:thunk, CodeInfo( @ none within `top-level scope` -1 โ”€ %1 = Base.vect -โ”‚ %2 = 1 + 2 -โ”‚ %3 = sin(0.5) -โ”‚ %4 = (%1)(%2, %3) -โ””โ”€โ”€ return %4 +1 โ”€ %1 = 1 + 2 +โ”‚ %2 = sin(0.5) +โ”‚ %3 = Base.vect(%1, %2) +โ””โ”€โ”€ return %3 )))) ```