From 4d3a223db6e0892c1dd95cd29ba3f7e278356044 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 23 Jul 2018 19:44:17 -0400 Subject: [PATCH] Add a special GC AS for array ptrs 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 10af27ff45f62..9ed7da79ba699 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 33ec24fcfe47d..e52a57acbb5f0 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 8f031440935de..265a95e31b4e3 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 a63d1fb62ffeb..50faf038028da 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 c274fbdeb6958..d93b2de8e8a9c 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 c43f0a2e017e7..2a331c7776d07 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 73a8542188f57..226385077655d 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 f83ba0d2bfa7a..a48fd1a96eb8f 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 db9c25329ba9d..25d75a8a7a0d1 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}