add gc_safe to :foreigncall expr

vchuravy · vchuravy · commit 47a08bbe263f · 2023-10-09T16:30:49.000-04:00
diff --git a/base/boot.jl b/base/boot.jl
@@ -507,7 +507,7 @@ AbstractArray{T}(A::AbstractArray{S,N}) where {T,S,N} = AbstractArray{T,N}(A)
 ## Helper for proper GC rooting without unsafe_convert
 eval(Core, quote
     _Symbol(ptr::Ptr{UInt8}, sz::Int, root::Any) = $(Expr(:foreigncall, QuoteNode(:jl_symbol_n),
-        Ref{Symbol}, svec(Ptr{UInt8}, Int), 0, QuoteNode(:ccall), :ptr, :sz, :root))
+        Ref{Symbol}, svec(Ptr{UInt8}, Int), 0, false, QuoteNode(:ccall), :ptr, :sz, :root))
 end)
 
 function Symbol(s::String)
diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl
@@ -2588,7 +2588,7 @@ function abstract_eval_foreigncall(interp::AbstractInterpreter, e::Expr, vtypes:
     effects = foreigncall_effects(e) do @nospecialize x
         abstract_eval_value(interp, x, vtypes, sv)
     end
-    cconv = e.args[5]
+    cconv = e.args[6]
     if isa(cconv, QuoteNode) && (v = cconv.value; isa(v, Tuple{Symbol, UInt8}))
         override = decode_effects_override(v[2])
         effects = Effects(effects;
diff --git a/base/compiler/ssair/EscapeAnalysis/EscapeAnalysis.jl b/base/compiler/ssair/EscapeAnalysis/EscapeAnalysis.jl
@@ -745,18 +745,18 @@ function compute_frameinfo(ir::IRCode)
             ndims = alloc_array_ndims(nn)
             ndims === nothing && @goto next_stmt
             if ndims ≠ 0
-                length(args) ≥ ndims+6 || @goto next_stmt
+                length(args) ≥ ndims+7 || @goto next_stmt
                 dims = Int[]
                 for i in 1:ndims
-                    dim = argextype(args[i+6], ir)
+                    dim = argextype(args[i+7], ir)
                     isa(dim, Const) || @goto next_stmt
                     dim = dim.val
                     isa(dim, Int) || @goto next_stmt
                     push!(dims, dim)
                 end
             else
-                length(args) ≥ 7 || @goto next_stmt
-                dims = argextype(args[7], ir)
+                length(args) ≥ 8 || @goto next_stmt
+                dims = argextype(args[8], ir)
                 if isa(dims, Const)
                     dims = dims.val
                     isa(dims, Tuple{Vararg{Int}}) || @goto next_stmt
@@ -1153,7 +1153,7 @@ end
 # TODO: we can apply a similar strategy like builtin calls to specialize some foreigncalls
 function escape_foreigncall!(astate::AnalysisState, pc::Int, args::Vector{Any})
     nargs = length(args)
-    if nargs < 6
+    if nargs < 7
         # invalid foreigncall, just escape everything
         add_conservative_changes!(astate, pc, args)
         return
@@ -1196,7 +1196,7 @@ function escape_foreigncall!(astate::AnalysisState, pc::Int, args::Vector{Any})
         add_escape_change!(astate, args[5+i], arg_info)
         add_liveness_change!(astate, args[5+i], pc)
     end
-    for i = (5+nargs):length(args)
+    for i = (6+nargs):length(args)
         arg = args[i]
         add_escape_change!(astate, arg, ⊥)
         add_liveness_change!(astate, arg, pc)
diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl
@@ -1802,7 +1802,7 @@ function late_inline_special_case!(
         return SomeCase(typevar_call)
     elseif f === UnionAll && length(argtypes) == 3 && ⊑(optimizer_lattice(state.interp), argtypes[2], TypeVar)
         unionall_call = Expr(:foreigncall, QuoteNode(:jl_type_unionall), Any, svec(Any, Any),
-            0, QuoteNode(:ccall), stmt.args[2], stmt.args[3])
+            0, false, QuoteNode(:ccall), stmt.args[2], stmt.args[3])
         return SomeCase(unionall_call)
     elseif is_return_type(f)
         if isconstType(type)
diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl
@@ -1041,7 +1041,7 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
             nccallargs = length(stmt.args[3]::SimpleVector)
             preserved = Int[]
             new_preserves = Any[]
-            for pidx in (6+nccallargs):length(stmt.args)
+            for pidx in (7+nccallargs):length(stmt.args)
                 preserved_arg = stmt.args[pidx]
                 isa(preserved_arg, SSAValue) || continue
                 let intermediaries = SPCSet()
@@ -1613,10 +1613,10 @@ end
 function form_new_preserves(origex::Expr, intermediates::Vector{Int}, new_preserves::Vector{Any})
     newex = Expr(:foreigncall)
     nccallargs = length(origex.args[3]::SimpleVector)
-    for i in 1:(6+nccallargs-1)
+    for i in 1:(7+nccallargs-1)
         push!(newex.args, origex.args[i])
     end
-    for i in (6+nccallargs):length(origex.args)
+    for i in (7+nccallargs):length(origex.args)
         x = origex.args[i]
         # don't need to preserve intermediaries
         if isa(x, SSAValue) && x.id in intermediates
diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl
@@ -2905,7 +2905,7 @@ end
 # N.B. the `abstract_eval` callback below allows us to use these queries
 # both during abstract interpret and optimization
 
-const FOREIGNCALL_ARG_START = 6
+const FOREIGNCALL_ARG_START = 7
 
 function foreigncall_effects(@specialize(abstract_eval), e::Expr)
     args = e.args
diff --git a/base/compiler/validation.jl b/base/compiler/validation.jl
@@ -23,7 +23,7 @@ const VALID_EXPR_HEADS = IdDict{Symbol,UnitRange{Int}}(
     :copyast => 1:1,
     :meta => 0:typemax(Int),
     :global => 1:1,
-    :foreigncall => 5:typemax(Int), # name, RT, AT, nreq, (cconv, effects), args..., roots...
+    :foreigncall => 6:typemax(Int), # name, RT, AT, nreq, gc_safe, (cconv, effects), args..., roots...
     :cfunction => 5:5,
     :isdefined => 1:1,
     :code_coverage_effect => 0:0,
diff --git a/base/meta.jl b/base/meta.jl
@@ -403,7 +403,9 @@ function _partially_inline!(@nospecialize(x), slot_replacements::Vector{Any},
                 elseif i == 4
                     @assert isa(x.args[4], Int)
                 elseif i == 5
-                    @assert isa((x.args[5]::QuoteNode).value, Union{Symbol, Tuple{Symbol, UInt8}})
+                    @assert isa(x.ars[5], Bool)
+                elseif i == 6
+                    @assert isa((x.args[6]::QuoteNode).value, Union{Symbol, Tuple{Symbol, UInt8}})
                 else
                     x.args[i] = _partially_inline!(x.args[i], slot_replacements,
                                                    type_signature, static_param_values,
diff --git a/base/strings/string.jl b/base/strings/string.jl
@@ -87,7 +87,7 @@ end
 
 # This is @assume_effects :effect_free :nothrow :terminates_globally @ccall jl_alloc_string(n::Csize_t)::Ref{String},
 # but the macro is not available at this time in bootstrap, so we write it manually.
-@eval _string_n(n::Integer) = $(Expr(:foreigncall, QuoteNode(:jl_alloc_string), Ref{String}, Expr(:call, Expr(:core, :svec), :Csize_t), 1, QuoteNode((:ccall,0xe)), :(convert(Csize_t, n))))
+@eval _string_n(n::Integer) = $(Expr(:foreigncall, QuoteNode(:jl_alloc_string), Ref{String}, Expr(:call, Expr(:core, :svec), :Csize_t), 1, false, QuoteNode((:ccall,0xe)), :(convert(Csize_t, n))))
 
 """
     String(s::AbstractString)
diff --git a/doc/src/devdocs/ast.md b/doc/src/devdocs/ast.md
@@ -498,15 +498,19 @@ These symbols appear in the `head` field of [`Expr`](@ref)s in lowered form.
 
         The number of required arguments for a varargs function definition.
 
-      * `args[5]::QuoteNode{Symbol}` : calling convention
+      * `args[5]::Bool` : gc_safe
+
+        If this foreigncall is safe to execute concurrently to GC.
+
+      * `args[6]::QuoteNode{Symbol}` : calling convention
 
         The calling convention for the call.
 
-      * `args[6:5+length(args[3])]` : arguments
+      * `args[7:6+length(args[3])]` : arguments
 
         The values for all the arguments (with types of each given in args[3]).
 
-      * `args[6+length(args[3])+1:end]` : gc-roots
+      * `args[7+length(args[3])+1:end]` : gc-roots
 
         The additional objects that may need to be gc-rooted for the duration of the call.
         See [Working with LLVM](@ref Working-with-LLVM) for where these are derived from and how they get handled.
diff --git a/doc/src/devdocs/llvm.md b/doc/src/devdocs/llvm.md
@@ -344,7 +344,7 @@ need to make sure that the array does stay alive while we're doing the
 [`ccall`](@ref). To understand how this is done, first recall the lowering of the
 above code:
 ```julia
-return $(Expr(:foreigncall, :(:foo), Cvoid, svec(Ptr{Float64}), 0, :(:ccall), Expr(:foreigncall, :(:jl_array_ptr), Ptr{Float64}, svec(Any), 0, :(:ccall), :(A)), :(A)))
+return $(Expr(:foreigncall, :(:foo), Cvoid, svec(Ptr{Float64}), 0, false, :(:ccall), Expr(:foreigncall, :(:jl_array_ptr), Ptr{Float64}, svec(Any), 0, false, :(:ccall), :(A)), :(A)))
 ```
 The last `:(A)`, is an extra argument list inserted during lowering that informs
 the code generator which Julia level values need to be kept alive for the
diff --git a/src/ccall.cpp b/src/ccall.cpp
@@ -1029,6 +1029,7 @@ class function_sig_t {
     AttributeList attributes; // vector of function call site attributes
     Type *lrt; // input parameter of the llvm return type (from julia_struct_to_llvm)
     bool retboxed; // input parameter indicating whether lrt is jl_value_t*
+    bool gc_safe; // input parameter indicating whether the call is safe to execute concurrently to GC
     Type *prt; // out parameter of the llvm return type for the function signature
     int sret; // out parameter for indicating whether return value has been moved to the first argument position
     std::string err_msg;
@@ -1041,8 +1042,8 @@ class function_sig_t {
     size_t nreqargs; // number of required arguments in ccall function definition
     jl_codegen_params_t *ctx;
 
-    function_sig_t(const char *fname, Type *lrt, jl_value_t *rt, bool retboxed, jl_svec_t *at, jl_unionall_t *unionall_env, size_t nreqargs, CallingConv::ID cc, bool llvmcall, jl_codegen_params_t *ctx)
-      : lrt(lrt), retboxed(retboxed),
+    function_sig_t(const char *fname, Type *lrt, jl_value_t *rt, bool retboxed, bool gc_safe, jl_svec_t *at, jl_unionall_t *unionall_env, size_t nreqargs, CallingConv::ID cc, bool llvmcall, jl_codegen_params_t *ctx)
+      : lrt(lrt), retboxed(retboxed), gc_safe(gc_safe),
         prt(NULL), sret(0), cc(cc), llvmcall(llvmcall),
         at(at), rt(rt), unionall_env(unionall_env),
         nccallargs(jl_svec_len(at)), nreqargs(nreqargs),
@@ -1064,8 +1065,7 @@ class function_sig_t {
             const native_sym_arg_t &symarg,
             jl_cgval_t *argv,
             SmallVector<Value*, 16> &gc_uses,
-            bool static_rt,
-            bool gc_safe) const;
+            bool static_rt) const;
 
 private:
 std::string generate_func_sig(const char *fname)
@@ -1190,6 +1190,9 @@ std::string generate_func_sig(const char *fname)
         RetAttrs = RetAttrs.addAttribute(LLVMCtx, Attribute::NonNull);
     if (rt == jl_bottom_type)
         FnAttrs = FnAttrs.addAttribute(LLVMCtx, Attribute::NoReturn);
+    if (gc_safe)
+        FnAttrs = FnAttrs.addAttribute(LLVMCtx, "julia.gc_safe");
+
     assert(attributes.isEmpty());
     attributes = AttributeList::get(LLVMCtx, FnAttrs, RetAttrs, paramattrs);
     return "";
@@ -1305,32 +1308,29 @@ static const std::string verify_ccall_sig(jl_value_t *&rt, jl_value_t *at,
     return "";
 }
 
-const int fc_args_start = 6;
+const int fc_args_start = 7;
 
-// Expr(:foreigncall, pointer, rettype, (argtypes...), nreq, [cconv | (cconv, effects)], args..., roots...)
+// Expr(:foreigncall, pointer, rettype, (argtypes...), nreq, gc_safe, [cconv | (cconv, effects)], args..., roots...)
 static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
 {
-    JL_NARGSV(ccall, 5);
+    JL_NARGSV(ccall, 6);
     args -= 1;
     jl_value_t *rt = args[2];
     jl_value_t *at = args[3];
     size_t nccallargs = jl_svec_len(at);
     size_t nreqargs = jl_unbox_long(args[4]); // if vararg
-    assert(jl_is_quotenode(args[5]));
-    jl_value_t *jlcc = jl_quotenode_value(args[5]);
+    assert(jl_is_quotenode(args[6]));
+    jl_value_t *jlcc = jl_quotenode_value(args[6]);
     jl_sym_t *cc_sym = NULL;
     if (jl_is_symbol(jlcc)) {
         cc_sym = (jl_sym_t*)jlcc;
     }
     else if (jl_is_tuple(jlcc)) {
         cc_sym = (jl_sym_t*)jl_get_nth_field_noalloc(jlcc, 0);
     }
-    // TODO: Can we detect calls to the runtime and not mark them gc_safe,
-    //       but for all other ccall we transition? We could add a new cconv
-    //       for runtime calls.
     // TODO: Can we introduce a intrinisc token = @julia.gc_safe_begin()
     //       and "grow" gc safe regions so that we minimize the overhead?
-    bool gc_safe = false;
+    bool gc_safe = jl_unbox_bool(args[5]);
     assert(jl_is_symbol(cc_sym));
     native_sym_arg_t symarg = {};
     JL_GC_PUSH3(&rt, &at, &symarg.gcroot);
@@ -1450,7 +1450,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
     }
     if (rt != args[2] && rt != (jl_value_t*)jl_any_type)
         rt = jl_ensure_rooted(ctx, rt);
-    function_sig_t sig("ccall", lrt, rt, retboxed,
+    function_sig_t sig("ccall", lrt, rt, retboxed, gc_safe,
                        (jl_svec_t*)at, unionall, nreqargs,
                        cc, llvmcall, &ctx.emission_context);
     for (size_t i = 0; i < nccallargs; i++) {
@@ -1920,8 +1920,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
             symarg,
             argv.data(),
             gc_uses,
-            static_rt,
-            gc_safe);
+            static_rt);
     JL_GC_POP();
     return retval;
 }
@@ -1931,8 +1930,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
         const native_sym_arg_t &symarg,
         jl_cgval_t *argv,
         SmallVector<Value*, 16> &gc_uses,
-        bool static_rt,
-        bool gc_safe) const
+        bool static_rt) const
 {
     ++EmittedCCalls;
     if (!err_msg.empty()) {
@@ -2113,11 +2111,11 @@ jl_cgval_t function_sig_t::emit_a_ccall(
         }
     }
 
-    Value *last_gc_state = NULL;
-    Value *ptls = get_current_ptls(ctx); // Do we need to reload this after the ccall?
+    // Value *last_gc_state = NULL;
+    // Value *ptls = get_current_ptls(ctx); // Do we need to reload this after the ccall?
 
-    if (gc_safe)
-        last_gc_state = emit_gc_safe_enter(ctx.builder, ctx.types().T_size, ptls, false);
+    // if (gc_safe)
+    //    last_gc_state = emit_gc_safe_enter(ctx.builder, ctx.types().T_size, ptls, false);
     OperandBundleDef OpBundle("jl_roots", gc_uses);
     // the actual call
     CallInst *ret = ctx.builder.CreateCall(functype, llvmf,
@@ -2133,8 +2131,11 @@ jl_cgval_t function_sig_t::emit_a_ccall(
         ctx.f->addFnAttr(Attribute::StackProtectReq);
     }
 
-    if (gc_safe)
-        emit_gc_safe_leave(ctx.builder, ctx.types().T_size, ptls, last_gc_state, false);
+    // if (gc_safe)
+        // ret->addAttribute("julia.gc_safe");
+
+    //if (gc_safe)
+    //    emit_gc_safe_leave(ctx.builder, ctx.types().T_size, ptls, last_gc_state, false);
 
     if (rt == jl_bottom_type) {
         CreateTrap(ctx.builder);
diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -6706,7 +6706,7 @@ static jl_cgval_t emit_cfunction(jl_codectx_t &ctx, jl_value_t *output_type, con
     if (rt != declrt && rt != (jl_value_t*)jl_any_type)
         rt = jl_ensure_rooted(ctx, rt);
 
-    function_sig_t sig("cfunction", lrt, rt, retboxed, argt, unionall_env, false, CallingConv::C, false, &ctx.emission_context);
+    function_sig_t sig("cfunction", lrt, rt, retboxed, false, argt, unionall_env, false, CallingConv::C, false, &ctx.emission_context);
     assert(sig.fargt.size() + sig.sret == sig.fargt_sig.size());
     if (!sig.err_msg.empty()) {
         emit_error(ctx, sig.err_msg);
@@ -6846,7 +6846,7 @@ const char *jl_generate_ccallable(LLVMOrcThreadSafeModuleRef llvmmod, void *sysi
     }
     jl_value_t *err;
     { // scope block for sig
-        function_sig_t sig("cfunction", lcrt, crt, toboxed,
+        function_sig_t sig("cfunction", lcrt, crt, toboxed, false,
                            argtypes, NULL, false, CallingConv::C, false, &params);
         if (sig.err_msg.empty()) {
             size_t world = jl_atomic_load_acquire(&jl_world_counter);
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
@@ -1122,6 +1122,7 @@
                          ;; 0 or number of arguments before ... in definition
                          ,(or nreq
                               (if isseq (- (length atypes) 1) 0))
+                          ,'(false)
                          ',cconv
                          ,.(reverse! C)
                          ,@GC)) ; GC root ordering is arbitrary
@@ -4502,13 +4503,13 @@ f(x) = yt(x)
                    (not (tuple-call? fptr))))
              (let* ((args
                      (cond ((eq? (car e) 'foreigncall)
-                            ;; NOTE: 2nd to 5th arguments of ccall must be left in place
+                            ;; NOTE: 2nd to 6th arguments of ccall must be left in place
                             ;;       the 1st should be compiled if an atom.
                             (append (if (atom-or-not-tuple-call? (cadr e))
                                         (compile-args (list (cadr e)) break-labels)
                                         (list (cadr e)))
                                     (list-head (cddr e) 4)
-                                    (compile-args (list-tail e 6) break-labels)))
+                                    (compile-args (list-tail e 7) break-labels)))
                            ;; NOTE: arguments of cfunction must be left in place
                            ;;       except for argument 2 (fptr)
                            ((eq? (car e) 'cfunction)
diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp
@@ -2481,16 +2481,21 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) {
                 NewCall->copyMetadata(*CI);
                 CI->replaceAllUsesWith(NewCall);
                 UpdatePtrNumbering(CI, NewCall, S);
-            } else if (CI->arg_size() == CI->getNumOperands()) {
-                /* No operand bundle to lower */
-                ++it;
-                continue;
             } else {
-                CallInst *NewCall = CallInst::Create(CI, None, CI);
-                NewCall->takeName(CI);
-                NewCall->copyMetadata(*CI);
-                CI->replaceAllUsesWith(NewCall);
-                UpdatePtrNumbering(CI, NewCall, S);
+                if (CI->getFnAttr("julia.gc_safe").isValid())
+                    dbgs() << "Found julia.gc_safe attribute on" << *CI;
+                if (CI->arg_size() == CI->getNumOperands()) {
+                    /* No operand bundle to lower */
+                    ++it;
+                    continue;
+                } else {
+                    // remove operand bundle
+                    CallInst *NewCall = CallInst::Create(CI, None, CI);
+                    NewCall->takeName(CI);
+                    NewCall->copyMetadata(*CI);
+                    CI->replaceAllUsesWith(NewCall);
+                    UpdatePtrNumbering(CI, NewCall, S);
+                }
             }
             if (!CI->use_empty()) {
                 CI->replaceAllUsesWith(UndefValue::get(CI->getType()));
diff --git a/src/method.c b/src/method.c
@@ -155,7 +155,7 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
                 return expr;
             }
             if (e->head == jl_foreigncall_sym) {
-                JL_NARGSV(ccall method definition, 5); // (fptr, rt, at, nreq, (cc, effects))
+                JL_NARGSV(ccall method definition, 6); // (fptr, rt, at, nreq, gc_safe, (cc, effects))
                 jl_value_t *rt = jl_exprarg(e, 1);
                 jl_value_t *at = jl_exprarg(e, 2);
                 if (!jl_is_type(rt)) {
@@ -184,8 +184,9 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
                 }
                 check_c_types("ccall method definition", rt, at);
                 JL_TYPECHK(ccall method definition, long, jl_exprarg(e, 3));
-                JL_TYPECHK(ccall method definition, quotenode, jl_exprarg(e, 4));
-                jl_value_t *cc = jl_quotenode_value(jl_exprarg(e, 4));
+                JL_TYPECHK(ccall method definition, bool, jl_exprarg(e, 4));
+                JL_TYPECHK(ccall method definition, quotenode, jl_exprarg(e, 5));
+                jl_value_t *cc = jl_quotenode_value(jl_exprarg(e, 5));
                 if (!jl_is_symbol(cc)) {
                     JL_TYPECHK(ccall method definition, tuple, cc);
                     if (jl_nfields(cc) != 2) {
diff --git a/test/ccall.jl b/test/ccall.jl
@@ -36,7 +36,7 @@ end
         Core.svec(Ptr{Ptr{Cchar}}, Cstring,
             UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8,
             Cfloat, Cfloat, Cfloat, Cfloat, Cfloat, Cfloat, Cfloat, Cfloat, Cfloat),
-            2, :(:cdecl),
+            2, false, :(:cdecl),
             :(Base.unsafe_convert(Ptr{Ptr{Cchar}}, strp)), :(Base.unsafe_convert(Cstring, fmt)),
             0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
             Cfloat(1.1), Cfloat(2.2), Cfloat(3.3), Cfloat(4.4), Cfloat(5.5), Cfloat(6.6), Cfloat(7.7), Cfloat(8.8), Cfloat(9.9),
diff --git a/test/compiler/irpasses.jl b/test/compiler/irpasses.jl
