From 30e82b4cfd78bbb318856a800c5b5aec9a4b7d74 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 26 Aug 2021 12:08:48 -0400 Subject: [PATCH] atomics: optimize modify operation (partially) (#41859) Optimize the load/store portion of the operations, but not yet the invoke part. (cherry picked from commit 690eae23ca0f2d2a6dbca2771d94db3daf851198) --- src/cgutils.cpp | 295 ++++++++++++++++++++++++++------------- src/codegen.cpp | 30 ++-- src/intrinsics.cpp | 25 ++-- src/runtime_intrinsics.c | 22 ++- test/intrinsics.jl | 4 +- 5 files changed, 251 insertions(+), 125 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 288e98e9a712b..4b1f842effe22 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -1542,12 +1542,23 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j } static jl_cgval_t typed_store(jl_codectx_t &ctx, - Value *ptr, Value *idx_0based, const jl_cgval_t &rhs, const jl_cgval_t &cmp, + Value *ptr, Value *idx_0based, jl_cgval_t rhs, jl_cgval_t cmp, jl_value_t *jltype, MDNode *tbaa, MDNode *aliasscope, Value *parent, // for the write barrier, NULL if no barrier needed bool isboxed, AtomicOrdering Order, AtomicOrdering FailOrder, unsigned alignment, - bool needlock, bool issetfield, bool isreplacefield, bool maybe_null_if_boxed) -{ + bool needlock, bool issetfield, bool isreplacefield, bool isswapfield, bool ismodifyfield, + bool maybe_null_if_boxed, const std::string &fname) +{ + auto newval = [&](const jl_cgval_t &lhs) { + jl_cgval_t argv[3] = { cmp, lhs, rhs }; + Value *callval = emit_jlcall(ctx, jlapplygeneric_func, nullptr, argv, 3, JLCALL_F_CC); + argv[0] = mark_julia_type(ctx, callval, true, jl_any_type); + if (!jl_subtype(argv[0].typ, jltype)) { + emit_typecheck(ctx, argv[0], jltype, fname + "typed_store"); + argv[0] = update_julia_type(ctx, argv[0], jltype); + } + return argv[0]; + }; assert(!needlock || parent != nullptr); Type *elty = isboxed ? T_prjlvalue : julia_type_to_llvm(ctx, jltype); if (type_is_ghost(elty)) { @@ -1563,9 +1574,15 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, jl_datatype_t *rettyp = jl_apply_cmpswap_type(jltype); return emit_new_struct(ctx, (jl_value_t*)rettyp, 2, argv); } - else { + else if (isswapfield) { return ghostValue(jltype); } + else { // modifyfield + jl_cgval_t oldval = ghostValue(jltype); + jl_cgval_t argv[2] = { oldval, newval(oldval) }; + jl_datatype_t *rettyp = jl_apply_modify_type(jltype); + return emit_new_struct(ctx, (jl_value_t*)rettyp, 2, argv); + } } Value *intcast = nullptr; if (!isboxed && Order != AtomicOrdering::NotAtomic && !elty->isIntOrPtrTy()) { @@ -1582,13 +1599,15 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, if (nb != nb2) elty = Type::getIntNTy(jl_LLVMContext, nb2); } - Value *r; - if (!isboxed) - r = emit_unbox(ctx, realelty, rhs, jltype); - else - r = boxed(ctx, rhs); - if (realelty != elty) - r = ctx.builder.CreateZExt(r, elty); + Value *r = nullptr; + if (issetfield || isswapfield || isreplacefield) { + if (!isboxed) + r = emit_unbox(ctx, realelty, rhs, jltype); + else + r = boxed(ctx, rhs); + if (realelty != elty) + r = ctx.builder.CreateZExt(r, elty); + } Type *ptrty = PointerType::get(elty, ptr->getType()->getPointerAddressSpace()); if (ptr->getType() != ptrty) ptr = ctx.builder.CreateBitCast(ptr, ptrty); @@ -1598,33 +1617,22 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, alignment = sizeof(void*); else if (!alignment) alignment = julia_alignment(jltype); - Instruction *instr = nullptr; + Value *instr = nullptr; Value *Compare = nullptr; Value *Success = nullptr; - BasicBlock *DoneBB = issetfield || (!isreplacefield && !isboxed) ? nullptr : BasicBlock::Create(jl_LLVMContext, "done_xchg", ctx.f); + BasicBlock *DoneBB = nullptr; if (needlock) emit_lockstate_value(ctx, parent, true); jl_cgval_t oldval = rhs; - if (issetfield || Order == AtomicOrdering::NotAtomic) { - if (!issetfield) { - instr = ctx.builder.CreateAlignedLoad(elty, ptr, Align(alignment)); + if (issetfield || (Order == AtomicOrdering::NotAtomic && isswapfield)) { + if (isswapfield) { + auto *load = ctx.builder.CreateAlignedLoad(elty, ptr, Align(alignment)); if (aliasscope) - instr->setMetadata("noalias", aliasscope); + load->setMetadata("noalias", aliasscope); if (tbaa) - tbaa_decorate(tbaa, instr); + tbaa_decorate(tbaa, load); assert(realelty == elty); - if (isreplacefield) { - oldval = mark_julia_type(ctx, instr, isboxed, jltype); - Value *first_ptr = nullptr; - if (maybe_null_if_boxed) - first_ptr = isboxed ? instr : extract_first_ptr(ctx, instr); - Success = emit_nullcheck_guard(ctx, first_ptr, [&] { - return emit_f_is(ctx, oldval, cmp); - }); - BasicBlock *BB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); - ctx.builder.CreateCondBr(Success, BB, DoneBB); - ctx.builder.SetInsertPoint(BB); - } + instr = load; } StoreInst *store = ctx.builder.CreateAlignedStore(r, ptr, Align(alignment)); store->setOrdering(Order); @@ -1632,20 +1640,37 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, store->setMetadata("noalias", aliasscope); if (tbaa) tbaa_decorate(tbaa, store); - if (DoneBB) - ctx.builder.CreateBr(DoneBB); } - else if (isboxed || isreplacefield) { - // we have to handle isboxed here as a workaround for really bad LLVM design issue: plain Xchg only works with integers + else if (isswapfield && !isboxed) { + // we can't handle isboxed here as a workaround for really bad LLVM + // design issue: plain Xchg only works with integers +#if JL_LLVM_VERSION >= 130000 + auto *store = ctx.builder.CreateAtomicRMW(AtomicRMWInst::Xchg, ptr, r, Align(alignment), Order); +#else + auto *store = ctx.builder.CreateAtomicRMW(AtomicRMWInst::Xchg, ptr, r, Order); + store->setAlignment(Align(alignment)); +#endif + if (aliasscope) + store->setMetadata("noalias", aliasscope); + if (tbaa) + tbaa_decorate(tbaa, store); + instr = store; + } + else { + // replacefield, modifyfield, or swapfield (isboxed && atomic) + DoneBB = BasicBlock::Create(jl_LLVMContext, "done_xchg", ctx.f); bool needloop; PHINode *Succ = nullptr, *Current = nullptr; if (isreplacefield) { - if (!isboxed) { + if (Order == AtomicOrdering::NotAtomic) { + needloop = false; + } + else if (!isboxed) { needloop = ((jl_datatype_t*)jltype)->layout->haspadding; Value *SameType = emit_isa(ctx, cmp, jltype, nullptr).first; if (SameType != ConstantInt::getTrue(jl_LLVMContext)) { BasicBlock *SkipBB = BasicBlock::Create(jl_LLVMContext, "skip_xchg", ctx.f); - BasicBlock *BB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); + BasicBlock *BB = BasicBlock::Create(jl_LLVMContext, "ok_xchg", ctx.f); ctx.builder.CreateCondBr(SameType, BB, SkipBB); ctx.builder.SetInsertPoint(SkipBB); LoadInst *load = ctx.builder.CreateAlignedLoad(elty, ptr, Align(alignment)); @@ -1658,7 +1683,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, ctx.builder.CreateBr(DoneBB); ctx.builder.SetInsertPoint(DoneBB); Succ = ctx.builder.CreatePHI(T_int1, 2); - Succ->addIncoming(ConstantInt::get(T_int1, 0), SkipBB); + Succ->addIncoming(ConstantInt::get(T_int1, false), SkipBB); Current = ctx.builder.CreatePHI(instr->getType(), 2); Current->addIncoming(instr, SkipBB); ctx.builder.SetInsertPoint(BB); @@ -1676,50 +1701,112 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, needloop = true; } } - else { + else { // swap or modify LoadInst *Current = ctx.builder.CreateAlignedLoad(elty, ptr, Align(alignment)); - Current->setOrdering(AtomicOrdering::Monotonic); + Current->setOrdering(Order == AtomicOrdering::NotAtomic ? Order : AtomicOrdering::Monotonic); if (aliasscope) Current->setMetadata("noalias", aliasscope); if (tbaa) tbaa_decorate(tbaa, Current); Compare = Current; - needloop = true; + needloop = !isswapfield || Order != AtomicOrdering::NotAtomic; } BasicBlock *BB; + PHINode *CmpPhi; if (needloop) { BasicBlock *From = ctx.builder.GetInsertBlock(); BB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); ctx.builder.CreateBr(BB); ctx.builder.SetInsertPoint(BB); - PHINode *Cmp = ctx.builder.CreatePHI(r->getType(), 2); - Cmp->addIncoming(Compare, From); - Compare = Cmp; - } - if (Order == AtomicOrdering::Unordered) - Order = AtomicOrdering::Monotonic; - if (!isreplacefield) - FailOrder = AtomicOrdering::Monotonic; - else if (FailOrder == AtomicOrdering::Unordered) - FailOrder = AtomicOrdering::Monotonic; + CmpPhi = ctx.builder.CreatePHI(elty, 2); + CmpPhi->addIncoming(Compare, From); + Compare = CmpPhi; + } + if (ismodifyfield) { + if (needlock) + emit_lockstate_value(ctx, parent, false); + Value *realCompare = Compare; + if (realelty != elty) + realCompare = ctx.builder.CreateTrunc(realCompare, realelty); + if (intcast) { + ctx.builder.CreateStore(realCompare, ctx.builder.CreateBitCast(intcast, realCompare->getType()->getPointerTo())); + if (maybe_null_if_boxed) + realCompare = ctx.builder.CreateLoad(intcast); + } + if (maybe_null_if_boxed) { + Value *first_ptr = isboxed ? Compare : extract_first_ptr(ctx, Compare); + if (first_ptr) + null_pointer_check(ctx, first_ptr, nullptr); + } + if (intcast) + oldval = mark_julia_slot(intcast, jltype, NULL, tbaa_stack); + else + oldval = mark_julia_type(ctx, realCompare, isboxed, jltype); + rhs = newval(oldval); + if (!isboxed) + r = emit_unbox(ctx, realelty, rhs, jltype); + else + r = boxed(ctx, rhs); + if (realelty != elty) + r = ctx.builder.CreateZExt(r, elty); + if (needlock) + emit_lockstate_value(ctx, parent, true); + cmp = oldval; + } + Value *Done; + if (Order == AtomicOrdering::NotAtomic) { + // modifyfield or replacefield + assert(elty == realelty && !intcast); + auto *load = ctx.builder.CreateAlignedLoad(elty, ptr, Align(alignment)); + if (aliasscope) + load->setMetadata("noalias", aliasscope); + if (tbaa) + tbaa_decorate(tbaa, load); + Value *first_ptr = nullptr; + if (maybe_null_if_boxed && !ismodifyfield) + first_ptr = isboxed ? load : extract_first_ptr(ctx, load); + oldval = mark_julia_type(ctx, load, isboxed, jltype); + Success = emit_nullcheck_guard(ctx, first_ptr, [&] { + return emit_f_is(ctx, oldval, cmp); + }); + if (needloop && ismodifyfield) + CmpPhi->addIncoming(load, ctx.builder.GetInsertBlock()); + assert(Succ == nullptr); + BasicBlock *XchgBB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); + ctx.builder.CreateCondBr(Success, XchgBB, needloop && ismodifyfield ? BB : DoneBB); + ctx.builder.SetInsertPoint(XchgBB); + auto *store = ctx.builder.CreateAlignedStore(r, ptr, Align(alignment)); + if (aliasscope) + store->setMetadata("noalias", aliasscope); + if (tbaa) + tbaa_decorate(tbaa, store); + ctx.builder.CreateBr(DoneBB); + instr = load; + } + else { + if (Order == AtomicOrdering::Unordered) + Order = AtomicOrdering::Monotonic; + if (!isreplacefield) + FailOrder = AtomicOrdering::Monotonic; + else if (FailOrder == AtomicOrdering::Unordered) + FailOrder = AtomicOrdering::Monotonic; #if JL_LLVM_VERSION >= 130000 - auto *store = ctx.builder.CreateAtomicCmpXchg(ptr, Compare, r, Align(alignment), Order, FailOrder); + auto *store = ctx.builder.CreateAtomicCmpXchg(ptr, Compare, r, Align(alignment), Order, FailOrder); #else - auto *store = ctx.builder.CreateAtomicCmpXchg(ptr, Compare, r, Order, FailOrder); - store->setAlignment(Align(alignment)); + auto *store = ctx.builder.CreateAtomicCmpXchg(ptr, Compare, r, Order, FailOrder); + store->setAlignment(Align(alignment)); #endif - if (aliasscope) - store->setMetadata("noalias", aliasscope); - if (tbaa) - tbaa_decorate(tbaa, store); - instr = ctx.builder.Insert(ExtractValueInst::Create(store, 0)); - Success = ctx.builder.Insert(ExtractValueInst::Create(store, 1)); - Value *Done = Success; - if (needloop) { - if (isreplacefield) { + if (aliasscope) + store->setMetadata("noalias", aliasscope); + if (tbaa) + tbaa_decorate(tbaa, store); + instr = ctx.builder.Insert(ExtractValueInst::Create(store, 0)); + Success = ctx.builder.Insert(ExtractValueInst::Create(store, 1)); + Done = Success; + if (isreplacefield && needloop) { Value *realinstr = instr; if (realelty != elty) - realinstr = ctx.builder.CreateTrunc(instr, realelty); + realinstr = ctx.builder.CreateTrunc(realinstr, realelty); if (intcast) { ctx.builder.CreateStore(realinstr, ctx.builder.CreateBitCast(intcast, realinstr->getType()->getPointerTo())); oldval = mark_julia_slot(intcast, jltype, NULL, tbaa_stack); @@ -1739,7 +1826,12 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, }); Done = ctx.builder.CreateNot(Done); } - cast(Compare)->addIncoming(instr, ctx.builder.GetInsertBlock()); + if (needloop) + ctx.builder.CreateCondBr(Done, DoneBB, BB); + else + ctx.builder.CreateBr(DoneBB); + if (needloop) + CmpPhi->addIncoming(instr, ctx.builder.GetInsertBlock()); } if (Succ != nullptr) { Current->addIncoming(instr, ctx.builder.GetInsertBlock()); @@ -1747,31 +1839,12 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, Succ->addIncoming(Success, ctx.builder.GetInsertBlock()); Success = Succ; } - if (needloop) - ctx.builder.CreateCondBr(Done, DoneBB, BB); - else - ctx.builder.CreateBr(DoneBB); - } - else { -#if JL_LLVM_VERSION >= 130000 - instr = ctx.builder.CreateAtomicRMW(AtomicRMWInst::Xchg, ptr, r, Align(alignment), Order); -#else - auto *store = ctx.builder.CreateAtomicRMW(AtomicRMWInst::Xchg, ptr, r, Order); - store->setAlignment(Align(alignment)); - instr = store; -#endif - if (aliasscope) - instr->setMetadata("noalias", aliasscope); - if (tbaa) - tbaa_decorate(tbaa, instr); - assert(DoneBB == nullptr); } if (DoneBB) ctx.builder.SetInsertPoint(DoneBB); if (needlock) emit_lockstate_value(ctx, parent, false); if (parent != NULL) { - BasicBlock *DoneBB; if (isreplacefield) { // TOOD: avoid this branch if we aren't making a write barrier BasicBlock *BB = BasicBlock::Create(jl_LLVMContext, "xchg_wb", ctx.f); @@ -1788,7 +1861,12 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx, ctx.builder.SetInsertPoint(DoneBB); } } - if (!issetfield) { + if (ismodifyfield) { + jl_cgval_t argv[2] = { oldval, rhs }; + jl_datatype_t *rettyp = jl_apply_modify_type(jltype); + oldval = emit_new_struct(ctx, (jl_value_t*)rettyp, 2, argv); + } + else if (!issetfield) { // swapfield or replacefield if (realelty != elty) instr = ctx.builder.Insert(CastInst::Create(Instruction::Trunc, instr, realelty)); if (intcast) { @@ -3188,12 +3266,13 @@ static void emit_write_multibarrier(jl_codectx_t &ctx, Value *parent, Value *agg static jl_cgval_t emit_setfield(jl_codectx_t &ctx, jl_datatype_t *sty, const jl_cgval_t &strct, size_t idx0, - const jl_cgval_t &rhs, const jl_cgval_t &cmp, + jl_cgval_t rhs, jl_cgval_t cmp, bool checked, bool wb, AtomicOrdering Order, AtomicOrdering FailOrder, - bool needlock, bool issetfield, bool isreplacefield) + bool needlock, bool issetfield, bool isreplacefield, bool isswapfield, bool ismodifyfield, + const std::string &fname) { if (!sty->name->mutabl && checked) { - std::string msg = "setfield!: immutable struct of type " + std::string msg = fname + "immutable struct of type " + std::string(jl_symbol_name(sty->name->name)) + " cannot be changed"; emit_error(ctx, msg); @@ -3217,29 +3296,48 @@ static jl_cgval_t emit_setfield(jl_codectx_t &ctx, jl_cgval_t rhs_union = convert_julia_type(ctx, rhs, jfty); if (rhs_union.typ == jl_bottom_type) return jl_cgval_t(); - Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jfty); - tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(T_int8, 1)); Value *ptindex = ctx.builder.CreateInBoundsGEP(T_int8, emit_bitcast(ctx, maybe_decay_tracked(ctx, addr), T_pint8), ConstantInt::get(T_size, fsz)); if (needlock) emit_lockstate_value(ctx, strct, true); + BasicBlock *BB = ctx.builder.GetInsertBlock(); jl_cgval_t oldval = rhs; if (!issetfield) oldval = emit_unionload(ctx, addr, ptindex, jfty, fsz, al, strct.tbaa, true); Value *Success; BasicBlock *DoneBB; - if (isreplacefield) { - BasicBlock *BB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); + if (isreplacefield || ismodifyfield) { + if (ismodifyfield) { + if (needlock) + emit_lockstate_value(ctx, strct, false); + jl_cgval_t argv[3] = { cmp, oldval, rhs }; + Value *callval = emit_jlcall(ctx, jlapplygeneric_func, nullptr, argv, 3, JLCALL_F_CC); + rhs = mark_julia_type(ctx, callval, true, jl_any_type); + if (!jl_subtype(rhs.typ, jfty)) { + emit_typecheck(ctx, rhs, jfty, fname); + rhs = update_julia_type(ctx, rhs, jfty); + } + rhs_union = convert_julia_type(ctx, rhs, jfty); + if (rhs_union.typ == jl_bottom_type) + return jl_cgval_t(); + if (needlock) + emit_lockstate_value(ctx, strct, true); + cmp = oldval; + oldval = emit_unionload(ctx, addr, ptindex, jfty, fsz, al, strct.tbaa, true); + } + BasicBlock *XchgBB = BasicBlock::Create(jl_LLVMContext, "xchg", ctx.f); DoneBB = BasicBlock::Create(jl_LLVMContext, "done_xchg", ctx.f); Success = emit_f_is(ctx, oldval, cmp); - ctx.builder.CreateCondBr(Success, BB, DoneBB); - ctx.builder.SetInsertPoint(BB); + ctx.builder.CreateCondBr(Success, XchgBB, ismodifyfield ? BB : DoneBB); + ctx.builder.SetInsertPoint(XchgBB); } + Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jfty); + tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(T_int8, 1)); tbaa_decorate(tbaa_unionselbyte, ctx.builder.CreateAlignedStore(tindex, ptindex, Align(1))); // copy data if (!rhs.isghost) { emit_unionmove(ctx, addr, strct.tbaa, rhs, nullptr); } - if (isreplacefield) { + if (isreplacefield || ismodifyfield) { ctx.builder.CreateBr(DoneBB); ctx.builder.SetInsertPoint(DoneBB); } @@ -3251,6 +3349,11 @@ static jl_cgval_t emit_setfield(jl_codectx_t &ctx, jl_datatype_t *rettyp = jl_apply_cmpswap_type(jfty); oldval = emit_new_struct(ctx, (jl_value_t*)rettyp, 2, argv); } + else if (ismodifyfield) { + jl_cgval_t argv[2] = {oldval, rhs}; + jl_datatype_t *rettyp = jl_apply_modify_type(jfty); + oldval = emit_new_struct(ctx, (jl_value_t*)rettyp, 2, argv); + } return oldval; } else { @@ -3261,7 +3364,7 @@ static jl_cgval_t emit_setfield(jl_codectx_t &ctx, return typed_store(ctx, addr, NULL, rhs, cmp, jfty, strct.tbaa, nullptr, wb ? maybe_bitcast(ctx, data_pointer(ctx, strct), T_pjlvalue) : nullptr, isboxed, Order, FailOrder, align, - needlock, issetfield, isreplacefield, maybe_null); + needlock, issetfield, isreplacefield, isswapfield, ismodifyfield, maybe_null, fname); } } @@ -3440,7 +3543,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg else need_wb = false; emit_typecheck(ctx, rhs, jl_svecref(sty->types, i), "new"); - emit_setfield(ctx, sty, strctinfo, i, rhs, jl_cgval_t(), false, need_wb, AtomicOrdering::NotAtomic, AtomicOrdering::NotAtomic, false, true, false); + emit_setfield(ctx, sty, strctinfo, i, rhs, jl_cgval_t(), false, need_wb, AtomicOrdering::NotAtomic, AtomicOrdering::NotAtomic, false, true, false, false, false, ""); } return strctinfo; } diff --git a/src/codegen.cpp b/src/codegen.cpp index a8cec7abc536a..5c4218a82222e 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -2986,7 +2986,10 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, false, true, false, - false); + false, + false, + false, + ""); } } *ret = ary; @@ -3128,18 +3131,21 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, else if ((f == jl_builtin_setfield && (nargs == 3 || nargs == 4)) || (f == jl_builtin_swapfield && (nargs == 3 || nargs == 4)) || - (f == jl_builtin_replacefield && (nargs == 4 || nargs == 5 || nargs == 6))) { + (f == jl_builtin_replacefield && (nargs == 4 || nargs == 5 || nargs == 6)) || + (true && f == jl_builtin_modifyfield && (nargs == 4 || nargs == 5))) { bool issetfield = f == jl_builtin_setfield; bool isreplacefield = f == jl_builtin_replacefield; + bool isswapfield = f == jl_builtin_swapfield; + bool ismodifyfield = f == jl_builtin_modifyfield; const jl_cgval_t undefval; const jl_cgval_t &obj = argv[1]; const jl_cgval_t &fld = argv[2]; - jl_cgval_t val = argv[isreplacefield ? 4 : 3]; - const jl_cgval_t &cmp = isreplacefield ? argv[3] : undefval; + jl_cgval_t val = argv[isreplacefield || ismodifyfield ? 4 : 3]; + const jl_cgval_t &cmp = isreplacefield || ismodifyfield ? argv[3] : undefval; enum jl_memory_order order = jl_memory_order_notatomic; - const std::string fname = issetfield ? "setfield!" : isreplacefield ? "replacefield!" : "swapfield!"; - if (nargs >= (isreplacefield ? 5 : 4)) { - const jl_cgval_t &ord = argv[isreplacefield ? 5 : 4]; + const std::string fname = issetfield ? "setfield!" : isreplacefield ? "replacefield!" : isswapfield ? "swapfield!" : "modifyfield!"; + if (nargs >= (isreplacefield || ismodifyfield ? 5 : 4)) { + const jl_cgval_t &ord = argv[isreplacefield || ismodifyfield ? 5 : 4]; emit_typecheck(ctx, ord, (jl_value_t*)jl_symbol_type, fname); if (!ord.constant) return false; @@ -3173,7 +3179,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, if (idx != -1) { jl_value_t *ft = jl_svecref(uty->types, idx); if (!jl_has_free_typevars(ft)) { - if (!jl_subtype(val.typ, ft)) { + if (!ismodifyfield && !jl_subtype(val.typ, ft)) { emit_typecheck(ctx, val, ft, fname); val = update_julia_type(ctx, val, ft); } @@ -3189,8 +3195,11 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, isreplacefield ? (isatomic ? "replacefield!: atomic field cannot be written non-atomically" : "replacefield!: non-atomic field cannot be written atomically") : + isswapfield ? (isatomic ? "swapfield!: atomic field cannot be written non-atomically" - : "swapfield!: non-atomic field cannot be written atomically")); + : "swapfield!: non-atomic field cannot be written atomically") : + (isatomic ? "modifyfield!: atomic field cannot be written non-atomically" + : "modifyfield!: non-atomic field cannot be written atomically")); *ret = jl_cgval_t(); return true; } @@ -3208,7 +3217,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, (needlock || fail_order <= jl_memory_order_notatomic) ? (isboxed ? AtomicOrdering::Unordered : AtomicOrdering::NotAtomic) // TODO: we should do this for anything with CountTrackedPointers(elty).count > 0 : get_llvm_atomic_order(fail_order), - needlock, issetfield, isreplacefield); + needlock, issetfield, isreplacefield, isswapfield, ismodifyfield, + fname); return true; } } diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index e1d821a34e42d..7883542c74a13 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -684,7 +684,7 @@ static jl_cgval_t emit_pointerset(jl_codectx_t &ctx, jl_cgval_t *argv) if (!type_is_ghost(ptrty)) { thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); typed_store(ctx, thePtr, im1, x, jl_cgval_t(), ety, tbaa_data, nullptr, nullptr, isboxed, - AtomicOrdering::NotAtomic, AtomicOrdering::NotAtomic, align_nb, false, true, false, false); + AtomicOrdering::NotAtomic, AtomicOrdering::NotAtomic, align_nb, false, true, false, false, false, false, ""); } } return e; @@ -778,15 +778,18 @@ static jl_cgval_t emit_atomic_pointerref(jl_codectx_t &ctx, jl_cgval_t *argv) // e[i] = x (set) // e[i] <= x (swap) // e[i] y => x (replace) -static jl_cgval_t emit_atomic_pointerset(jl_codectx_t &ctx, intrinsic f, const jl_cgval_t *argv, int nargs) +// x(e[i], y) (modify) +static jl_cgval_t emit_atomic_pointerop(jl_codectx_t &ctx, intrinsic f, const jl_cgval_t *argv, int nargs) { bool issetfield = f == atomic_pointerset; bool isreplacefield = f == atomic_pointerreplace; + bool isswapfield = f == atomic_pointerswap; + bool ismodifyfield = f == atomic_pointermodify; const jl_cgval_t undefval; const jl_cgval_t &e = argv[0]; - const jl_cgval_t &x = isreplacefield ? argv[2] : argv[1]; - const jl_cgval_t &y = isreplacefield ? argv[1] : undefval; - const jl_cgval_t &ord = isreplacefield ? argv[3] : argv[2]; + const jl_cgval_t &x = isreplacefield || ismodifyfield ? argv[2] : argv[1]; + const jl_cgval_t &y = isreplacefield || ismodifyfield ? argv[1] : undefval; + const jl_cgval_t &ord = isreplacefield || ismodifyfield ? argv[3] : argv[2]; const jl_cgval_t &failord = isreplacefield ? argv[4] : undefval; jl_value_t *aty = e.typ; @@ -814,7 +817,7 @@ static jl_cgval_t emit_atomic_pointerset(jl_codectx_t &ctx, intrinsic f, const j Value *thePtr = emit_unbox(ctx, T_pprjlvalue, e, e.typ); bool isboxed = true; jl_cgval_t ret = typed_store(ctx, thePtr, nullptr, x, y, ety, tbaa_data, nullptr, nullptr, isboxed, - llvm_order, llvm_failorder, sizeof(jl_value_t*), false, issetfield, isreplacefield, false); + llvm_order, llvm_failorder, sizeof(jl_value_t*), false, issetfield, isreplacefield, isswapfield, ismodifyfield, false, "atomic_pointermodify"); if (issetfield) ret = e; return ret; @@ -826,7 +829,8 @@ static jl_cgval_t emit_atomic_pointerset(jl_codectx_t &ctx, intrinsic f, const j emit_error(ctx, msg); return jl_cgval_t(); } - emit_typecheck(ctx, x, ety, std::string(jl_intrinsic_name((int)f))); + if (!ismodifyfield) + emit_typecheck(ctx, x, ety, std::string(jl_intrinsic_name((int)f))); size_t nb = jl_datatype_size(ety); if ((nb & (nb - 1)) != 0 || nb > MAX_POINTERATOMIC_SIZE) { @@ -847,7 +851,7 @@ static jl_cgval_t emit_atomic_pointerset(jl_codectx_t &ctx, intrinsic f, const j assert(!isboxed); Value *thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); jl_cgval_t ret = typed_store(ctx, thePtr, nullptr, x, y, ety, tbaa_data, nullptr, nullptr, isboxed, - llvm_order, llvm_failorder, nb, false, issetfield, isreplacefield, false); + llvm_order, llvm_failorder, nb, false, issetfield, isreplacefield, isswapfield, ismodifyfield, false, "atomic_pointermodify"); if (issetfield) ret = e; return ret; @@ -1087,10 +1091,9 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar return emit_atomic_pointerref(ctx, argv); case atomic_pointerset: case atomic_pointerswap: - case atomic_pointerreplace: - return emit_atomic_pointerset(ctx, f, argv, nargs); case atomic_pointermodify: - return emit_runtime_call(ctx, f, argv, nargs); + case atomic_pointerreplace: + return emit_atomic_pointerop(ctx, f, argv, nargs); case bitcast: return generic_bitcast(ctx, argv); case trunc_int: diff --git a/src/runtime_intrinsics.c b/src/runtime_intrinsics.c index be78be74172cb..741bb5448b847 100644 --- a/src/runtime_intrinsics.c +++ b/src/runtime_intrinsics.c @@ -142,15 +142,25 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerswap(jl_value_t *p, jl_value_t *x, jl_ return y; } -JL_DLLEXPORT jl_value_t *jl_atomic_pointermodify(jl_value_t *p, jl_value_t *f, jl_value_t *x, jl_value_t *order_sym) +JL_DLLEXPORT jl_value_t *jl_atomic_pointermodify(jl_value_t *p, jl_value_t *f, jl_value_t *x, jl_value_t *order) { - // n.b. we use seq_cst always here, but need to verify the order sym - // against the weaker load-only that happens first - if (order_sym == (jl_value_t*)acquire_release_sym) - order_sym = (jl_value_t*)acquire_sym; - jl_value_t *expected = jl_atomic_pointerref(p, order_sym); + JL_TYPECHK(atomic_pointerref, pointer, p); + JL_TYPECHK(atomic_pointerref, symbol, order) + (void)jl_get_atomic_order_checked((jl_sym_t*)order, 1, 1); jl_value_t *ety = jl_tparam0(jl_typeof(p)); char *pp = (char*)jl_unbox_long(p); + jl_value_t *expected; + if (ety == (jl_value_t*)jl_any_type) { + expected = jl_atomic_load((jl_value_t**)pp); + } + else { + if (!is_valid_intrinsic_elptr(ety)) + jl_error("atomic_pointermodify: invalid pointer"); + size_t nb = jl_datatype_size(ety); + if ((nb & (nb - 1)) != 0 || nb > MAX_POINTERATOMIC_SIZE) + jl_error("atomic_pointermodify: invalid pointer for atomic operation"); + expected = jl_atomic_new_bits(ety, pp); + } jl_value_t **args; JL_GC_PUSHARGS(args, 2); args[0] = expected; diff --git a/test/intrinsics.jl b/test/intrinsics.jl index 7fb6bd651ebc0..589590cf78d14 100644 --- a/test/intrinsics.jl +++ b/test/intrinsics.jl @@ -191,8 +191,8 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co @test_throws ErrorException("atomic_pointerref: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) @test_throws ErrorException("atomic_pointerset: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerset(p, T(1), :sequentially_consistent) @test_throws ErrorException("atomic_pointerswap: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerswap(p, T(100), :sequentially_consistent) - @test_throws ErrorException("atomic_pointerref: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointermodify(p, add, T(1), :sequentially_consistent) - @test_throws ErrorException("atomic_pointerref: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointermodify(p, swap, S(1), :sequentially_consistent) + @test_throws ErrorException("atomic_pointermodify: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointermodify(p, add, T(1), :sequentially_consistent) + @test_throws ErrorException("atomic_pointermodify: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointermodify(p, swap, S(1), :sequentially_consistent) @test_throws ErrorException("atomic_pointerreplace: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerreplace(p, T(100), T(2), :sequentially_consistent, :sequentially_consistent) @test_throws ErrorException("atomic_pointerreplace: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerreplace(p, S(100), T(2), :sequentially_consistent, :sequentially_consistent) @test Core.Intrinsics.pointerref(p, 1, 1) === T(10) === r[]