diff --git a/src/ccall.cpp b/src/ccall.cpp index 47496a3a91ba6..46834d323925b 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -1413,7 +1413,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs) if (jl_is_long(argi_root)) continue; jl_cgval_t arg_root = emit_expr(ctx, argi_root); - Value *gc_root = get_gc_root_for(arg_root); + Value *gc_root = get_gc_root_for(ctx, arg_root); if (gc_root) gc_uses.push_back(gc_root); } diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 69f620869df93..7b15ef366777c 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -305,19 +305,34 @@ static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V) return Call; } -static Value *get_gc_root_for(const jl_cgval_t &x) +static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt); +static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, unsigned alignment, bool isVolatile=false); + +static Value *get_gc_root_for(jl_codectx_t &ctx, const jl_cgval_t &x) { - if (x.Vboxed) + if (x.constant || x.typ == jl_bottom_type) + return nullptr; + if (x.Vboxed) // superset of x.isboxed return x.Vboxed; - if (x.ispointer() && !x.constant) { + assert(!x.isboxed); +#ifndef NDEBUG + if (x.ispointer()) { assert(x.V); if (PointerType *T = dyn_cast(x.V->getType())) { - if (T->getAddressSpace() == AddressSpace::Tracked || - T->getAddressSpace() == AddressSpace::Derived) { - return x.V; + assert(T->getAddressSpace() != AddressSpace::Tracked); + if (T->getAddressSpace() == AddressSpace::Derived) { + // n.b. this IR would not be valid after LLVM-level inlining, + // since codegen does not have a way to determine the whether + // this argument value needs to be re-rooted } } } +#endif + if (jl_is_concrete_immutable(x.typ) && !jl_is_pointerfree(x.typ)) { + Type *T = julia_type_to_llvm(ctx, x.typ); + return emit_unbox(ctx, T, x, x.typ); + } + // nothing here to root, move along return nullptr; } @@ -1795,9 +1810,6 @@ static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_v return im1; } -static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt); -static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, unsigned alignment, bool isVolatile=false); - static void emit_write_barrier(jl_codectx_t&, Value*, ArrayRef); static void emit_write_barrier(jl_codectx_t&, Value*, Value*); static void emit_write_multibarrier(jl_codectx_t&, Value*, Value*, jl_value_t*); diff --git a/src/codegen.cpp b/src/codegen.cpp index f72c2ae2056b9..09fc465e93fca 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3028,9 +3028,12 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t a varg2 = emit_pointer_from_objref(ctx, varg2); Value *gc_uses[2]; int nroots = 0; - if ((gc_uses[nroots] = get_gc_root_for(arg1))) + // these roots may seem a bit overkill, but we want to make sure + // that a!=b implies (a,)!=(b,) even if a and b are unused and + // therefore could be freed and then the memory for a reused for b + if ((gc_uses[nroots] = get_gc_root_for(ctx, arg1))) nroots++; - if ((gc_uses[nroots] = get_gc_root_for(arg2))) + if ((gc_uses[nroots] = get_gc_root_for(ctx, arg2))) nroots++; OperandBundleDef OpBundle("jl_roots", makeArrayRef(gc_uses, nroots)); auto answer = ctx.builder.CreateCall(prepare_call(memcmp_func), { @@ -5790,16 +5793,9 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_ } std::vector vals; for (size_t i = 0; i < nargs; ++i) { - const jl_cgval_t &ai = argv[i]; - if (ai.constant || ai.typ == jl_bottom_type) - continue; - if (ai.isboxed) { - vals.push_back(ai.Vboxed); - } - else if (jl_is_concrete_immutable(ai.typ) && !jl_is_pointerfree(ai.typ)) { - Type *at = julia_type_to_llvm(ctx, ai.typ); - vals.push_back(emit_unbox(ctx, at, ai, ai.typ)); - } + Value *gc_root = get_gc_root_for(ctx, argv[i]); + if (gc_root) + vals.push_back(gc_root); } Value *token = vals.empty() ? (Value*)ConstantTokenNone::get(ctx.builder.getContext()) diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl index 26d9f0c08e797..9b34ae7321ff6 100644 --- a/test/compiler/codegen.jl +++ b/test/compiler/codegen.jl @@ -574,12 +574,12 @@ end end @test occursin("llvm.julia.gc_preserve_begin", get_llvm(f3, Tuple{Bool}, true, false, false)) - # unions of immutables (JuliaLang/julia#39501) + # PhiNode of unions of immutables (JuliaLang/julia#39501) function f2(cond) - val = cond ? 1 : 1f0 + val = cond ? 1 : "" GC.@preserve val begin end end - @test !occursin("llvm.julia.gc_preserve_begin", get_llvm(f2, Tuple{Bool}, true, false, false)) + @test occursin("llvm.julia.gc_preserve_begin", get_llvm(f2, Tuple{Bool}, true, false, false)) # make sure the fix for the above doesn't regress #34241 function f4(cond) val = cond ? ([1],) : ([1f0],)