From bf6a90b6078125e6a9635ea6995de28a525b88b1 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sat, 28 Jul 2018 18:58:43 -0400 Subject: [PATCH] Add a special GC AS for array ptrs (#28251) The array data pointer is somewhat special. It points to a chunk for memory that is effectively managed by the GC, but is not itself a GC-tracked value. However, it is also not quite an interior pointer into the array, since it may be an external allocation (or at the more immediate IR level it is derived using a load rather than a gep). We could try to make Derived do both, but the semantics turn out to be rather different, so add a new kind of AS `Loaded`, that handles precisely this situation: It roots the object that it was loaded from while it is live. Fixes #27955 --- doc/src/devdocs/llvm.md | 5 +++ src/ccall.cpp | 3 +- src/cgutils.cpp | 56 ++++++++++++++++++++++-------- src/codegen.cpp | 4 +-- src/codegen_shared.h | 7 ++-- src/jitlayers.cpp | 2 +- src/llvm-gc-invariant-verifier.cpp | 2 ++ src/llvm-late-gc-lowering.cpp | 26 +++++++++++++- test/llvmpasses/gcroots.ll | 16 +++++++++ 9 files changed, 99 insertions(+), 22 deletions(-) diff --git a/doc/src/devdocs/llvm.md b/doc/src/devdocs/llvm.md index 10af27ff45f62c..9ed7da79ba6995 100644 --- a/doc/src/devdocs/llvm.md +++ b/doc/src/devdocs/llvm.md @@ -211,6 +211,11 @@ three different address spaces (their numbers are defined in `src/codegen_shared future), but unlike the other pointers need not be rooted if passed to a call (they do still need to be rooted if they are live across another safepoint between the definition and the call). +- Pointers loaded from tracked object (currently 13): This is used by arrays, + which themselves contain a pointer to the managed data. This data area is owned + by the array, but is not a GC-tracked object by itself. The compiler guarantees + that as long as this pointer is live, the object that this pointer was loaded + from will keep being live. ### Invariants diff --git a/src/ccall.cpp b/src/ccall.cpp index 33ec24fcfe47d1..e52a57acbb5f0f 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -1638,9 +1638,8 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs) assert(!isVa && !llvmcall && nargt == 1); assert(!addressOf.at(0)); const jl_cgval_t &ary = argv[0]; - jl_value_t *aryex = ccallarg(0); JL_GC_POP(); - return mark_or_box_ccall_result(ctx, ctx.builder.CreatePtrToInt(emit_arrayptr(ctx, ary, aryex), lrt), + return mark_or_box_ccall_result(ctx, ctx.builder.CreatePtrToInt(emit_unsafe_arrayptr(ctx, ary), lrt), retboxed, rt, unionall, static_rt); } else if (is_libjulia_func(jl_value_ptr)) { diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 8f031440935ded..265a95e31b4e3a 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -238,20 +238,28 @@ static DIType *julia_type_to_di(jl_value_t *jt, DIBuilder *dbuilder, bool isboxe return (llvm::DIType*)jdt->ditype; } -static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V) +static Value *emit_pointer_from_objref_internal(jl_codectx_t &ctx, Value *V) { - unsigned AS = cast(V->getType())->getAddressSpace(); - if (AS != AddressSpace::Tracked && AS != AddressSpace::Derived) - return ctx.builder.CreatePtrToInt(V, T_size); - V = ctx.builder.CreateBitCast(decay_derived(V), - PointerType::get(T_jlvalue, AddressSpace::Derived)); CallInst *Call = ctx.builder.CreateCall(prepare_call(pointer_from_objref_func), V); #if JL_LLVM_VERSION >= 50000 Call->addAttribute(AttributeList::FunctionIndex, Attribute::ReadNone); #else Call->addAttribute(AttributeSet::FunctionIndex, Attribute::ReadNone); #endif - return ctx.builder.CreatePtrToInt(Call, T_size); + return Call; +} + +static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V) +{ + unsigned AS = cast(V->getType())->getAddressSpace(); + if (AS != AddressSpace::Tracked && AS != AddressSpace::Derived) + return ctx.builder.CreatePtrToInt(V, T_size); + V = ctx.builder.CreateBitCast(decay_derived(V), + PointerType::get(T_jlvalue, AddressSpace::Derived)); + + return ctx.builder.CreatePtrToInt( + emit_pointer_from_objref_internal(ctx, V), + T_size); } // --- emitting pointers directly into code --- @@ -1705,17 +1713,24 @@ static Value *emit_arraylen(jl_codectx_t &ctx, const jl_cgval_t &tinfo) return emit_arraylen_prim(ctx, tinfo); } -static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isboxed = false) +static Value *emit_arrayptr_internal(jl_codectx_t &ctx, const jl_cgval_t &tinfo, Value *t, unsigned AS, bool isboxed) { - Value *t = boxed(ctx, tinfo); - Value *addr = ctx.builder.CreateStructGEP(jl_array_llvmt, - emit_bitcast(ctx, decay_derived(t), jl_parray_llvmt), - 0); //index (not offset) of data field in jl_parray_llvmt - + Value *addr = + ctx.builder.CreateStructGEP(jl_array_llvmt, + emit_bitcast(ctx, t, jl_parray_llvmt), + 0); // index (not offset) of data field in jl_parray_llvmt MDNode *tbaa = arraytype_constshape(tinfo.typ) ? tbaa_const : tbaa_arrayptr; + PointerType *PT = cast(addr->getType()); + PointerType *PPT = cast(PT->getElementType()); if (isboxed) { addr = ctx.builder.CreateBitCast(addr, - PointerType::get(T_pprjlvalue, cast(addr->getType())->getAddressSpace())); + PointerType::get(PointerType::get(T_prjlvalue, AS), + PT->getAddressSpace())); + } else if (AS != PPT->getAddressSpace()) { + addr = ctx.builder.CreateBitCast(addr, + PointerType::get( + PointerType::get(PPT->getElementType(), AS), + PT->getAddressSpace())); } auto LI = ctx.builder.CreateLoad(addr); LI->setMetadata(LLVMContext::MD_nonnull, MDNode::get(jl_LLVMContext, None)); @@ -1723,6 +1738,19 @@ static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isb return LI; } +static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isboxed = false) +{ + Value *t = boxed(ctx, tinfo); + return emit_arrayptr_internal(ctx, tinfo, decay_derived(t), AddressSpace::Loaded, isboxed); +} + +static Value *emit_unsafe_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isboxed = false) +{ + Value *t = boxed(ctx, tinfo); + t = emit_pointer_from_objref_internal(ctx, decay_derived(t)); + return emit_arrayptr_internal(ctx, tinfo, t, 0, isboxed); +} + static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, jl_value_t *ex, bool isboxed = false) { return emit_arrayptr(ctx, tinfo, isboxed); diff --git a/src/codegen.cpp b/src/codegen.cpp index a63d1fb62ffebc..50faf038028dae 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -6830,7 +6830,7 @@ static void init_julia_llvm_env(Module *m) jl_func_sig = FunctionType::get(T_prjlvalue, ftargs, false); assert(jl_func_sig != NULL); - Type *vaelts[] = {T_pint8 + Type *vaelts[] = {PointerType::get(T_int8, AddressSpace::Loaded) #ifdef STORE_ARRAY_LEN , T_size #endif @@ -7476,7 +7476,7 @@ extern "C" void *jl_init_llvm(void) // Mark our address spaces as non-integral #if JL_LLVM_VERSION >= 40000 jl_data_layout = jl_ExecutionEngine->getDataLayout(); - std::string DL = jl_data_layout.getStringRepresentation() + "-ni:10:11:12"; + std::string DL = jl_data_layout.getStringRepresentation() + "-ni:10:11:12:13"; jl_data_layout.reset(DL); #endif diff --git a/src/codegen_shared.h b/src/codegen_shared.h index c274fbdeb69585..d93b2de8e8a9ce 100644 --- a/src/codegen_shared.h +++ b/src/codegen_shared.h @@ -5,9 +5,12 @@ enum AddressSpace { Generic = 0, - Tracked = 10, Derived = 11, CalleeRooted = 12, + Tracked = 10, + Derived = 11, + CalleeRooted = 12, + Loaded = 13, FirstSpecial = Tracked, - LastSpecial = CalleeRooted, + LastSpecial = Loaded, }; #define JLCALL_CC (CallingConv::ID)36 diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index b6ab352fff668b..7be87aeadad325 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -1160,7 +1160,7 @@ void jl_dump_native(const char *bc_fname, const char *unopt_bc_fname, const char shadow_output->setTargetTriple(TM->getTargetTriple().str()); #if JL_LLVM_VERSION >= 40000 DataLayout DL = TM->createDataLayout(); - DL.reset(DL.getStringRepresentation() + "-ni:10:11:12"); + DL.reset(DL.getStringRepresentation() + "-ni:10:11:12:13"); shadow_output->setDataLayout(DL); #else shadow_output->setDataLayout(TM->createDataLayout()); diff --git a/src/llvm-gc-invariant-verifier.cpp b/src/llvm-gc-invariant-verifier.cpp index 73a8542188f570..226385077655d4 100644 --- a/src/llvm-gc-invariant-verifier.cpp +++ b/src/llvm-gc-invariant-verifier.cpp @@ -65,6 +65,8 @@ void GCInvariantVerifier::visitAddrSpaceCastInst(AddrSpaceCastInst &I) { unsigned ToAS = cast(I.getDestTy())->getAddressSpace(); if (FromAS == 0) return; + Check(ToAS != AddressSpace::Loaded && FromAS != AddressSpace::Loaded, + "Illegal address space cast involving loaded ptr", &I); Check(FromAS != AddressSpace::Tracked || ToAS == AddressSpace::CalleeRooted || ToAS == AddressSpace::Derived, diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index f83ba0d2bfa7a5..a48fd1a96eb8fc 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -461,6 +461,23 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac fld_idx = IdxOp->getLimitedValue(INT_MAX); CurrentV = EEI->getVectorOperand(); } + else if (auto LI = dyn_cast(CurrentV)) { + if (auto PtrT = dyn_cast(LI->getType())) { + if (PtrT->getAddressSpace() == AddressSpace::Loaded) { + CurrentV = LI->getPointerOperand(); + if (!isSpecialPtr(CurrentV->getType())) { + // Special case to bypass the check below. + // This could really be anything, but it's not loaded + // from a tracked pointer, so it doesn't matter what + // it is. + return std::make_pair(CurrentV, fld_idx); + } + continue; + } + } + // In general a load terminates a walk + break; + } else { break; } @@ -538,6 +555,10 @@ int LateLowerGCFrame::NumberBase(State &S, Value *V, Value *CurrentV) getValueAddrSpace(CurrentV) != AddressSpace::Tracked)) { // We know this is rooted in the parent Number = -1; + } else if (!isSpecialPtr(CurrentV->getType()) && !isUnion) { + // Externally rooted somehow hopefully (otherwise there's a bug in the + // input IR) + Number = -1; } else if (isa(CurrentV) && !isUnion && getValueAddrSpace(CurrentV) != AddressSpace::Tracked) { int Number = LiftSelect(S, cast(CurrentV)); S.AllPtrNumbering[V] = Number; @@ -1089,7 +1110,10 @@ State LateLowerGCFrame::LocalScan(Function &F) { // we know that the object is a constant as well and doesn't need rooting. RefinedPtr.push_back(-2); } - MaybeNoteDef(S, BBS, LI, BBS.Safepoints, std::move(RefinedPtr)); + if (!LI->getType()->isPointerTy() || + cast(LI->getType())->getAddressSpace() != AddressSpace::Loaded) { + MaybeNoteDef(S, BBS, LI, BBS.Safepoints, std::move(RefinedPtr)); + } NoteOperandUses(S, BBS, I, BBS.UpExposedUsesUnrooted); } else if (SelectInst *SI = dyn_cast(&I)) { // We need to insert an extra select for the GC root diff --git a/test/llvmpasses/gcroots.ll b/test/llvmpasses/gcroots.ll index db9c25329ba9dc..25d75a8a7a0d16 100644 --- a/test/llvmpasses/gcroots.ll +++ b/test/llvmpasses/gcroots.ll @@ -385,6 +385,22 @@ top: ret %jl_value_t addrspace(10)* %rval } +define i8 @simple_arrayptr() { +; CHECK-LABEL: @simple_arrayptr +; CHECK: %gcframe = alloca %jl_value_t addrspace(10)*, i32 4 +top: + %ptls = call %jl_value_t*** @julia.ptls_states() + %obj1 = call %jl_value_t addrspace(10) *@alloc() + %obj2 = call %jl_value_t addrspace(10) *@alloc() + %decayed = addrspacecast %jl_value_t addrspace(10) *%obj1 to %jl_value_t addrspace(11) * + %arrayptrptr = bitcast %jl_value_t addrspace(11) *%decayed to i8 addrspace(13)* addrspace(11)* + %arrayptr = load i8 addrspace(13)*, i8 addrspace(13)* addrspace(11)* %arrayptrptr + call void @jl_safepoint() + call void @one_arg_boxed(%jl_value_t addrspace(10) *%obj2) + %val = load i8, i8 addrspace(13)* %arrayptr + ret i8 %val +} + !0 = !{!"jtbaa"} !1 = !{!"jtbaa_const", !0, i64 0} !2 = !{!1, !1, i64 0, i64 1}