From d75843d1f2c21157f389cbad478fc3aeb9247822 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Thu, 21 Jul 2022 16:52:21 -0300 Subject: [PATCH] Correct codegen bugs introduced by allocation-hoisting-PR (#45476) Co-authored-by: Jameson Nash --- src/cgutils.cpp | 63 ++++++++++++++++++++++++++++++---------- src/codegen.cpp | 4 +-- test/compiler/codegen.jl | 7 +++++ 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 501b581d7b71d..2f1803ef91051 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -1005,7 +1005,7 @@ static LoadInst *emit_nthptr_recast(jl_codectx_t &ctx, Value *v, ssize_t n, MDNo emit_bitcast(ctx, vptr, PointerType::get(type, 0))))); } -static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &v); +static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &v, bool is_promotable=false); static Value *emit_typeof(jl_codectx_t &ctx, Value *v, bool maybenull); static jl_cgval_t emit_typeof(jl_codectx_t &ctx, const jl_cgval_t &p, bool maybenull) @@ -3207,6 +3207,44 @@ static Value *box_union(jl_codectx_t &ctx, const jl_cgval_t &vinfo, const SmallB return box_merge; } +static Function *mangleIntrinsic(IntrinsicInst *call) //mangling based on replaceIntrinsicUseWith +{ + Intrinsic::ID ID = call->getIntrinsicID(); + auto nargs = call->arg_size(); + SmallVector argTys(nargs); + auto oldfType = call->getFunctionType(); + for (unsigned i = 0; i < oldfType->getNumParams(); i++) { + auto argi = call->getArgOperand(i); + argTys[i] = argi->getType(); + } + + auto newfType = FunctionType::get( + oldfType->getReturnType(), + makeArrayRef(argTys).slice(0, oldfType->getNumParams()), + oldfType->isVarArg()); + + // Accumulate an array of overloaded types for the given intrinsic + // and compute the new name mangling schema + SmallVector overloadTys; + { + SmallVector Table; + getIntrinsicInfoTableEntries(ID, Table); + ArrayRef TableRef = Table; + auto res = Intrinsic::matchIntrinsicSignature(newfType, TableRef, overloadTys); + assert(res == Intrinsic::MatchIntrinsicTypes_Match); + (void)res; + bool matchvararg = !Intrinsic::matchIntrinsicVarArg(newfType->isVarArg(), TableRef); + assert(matchvararg); + (void)matchvararg; + } + auto newF = Intrinsic::getDeclaration(call->getModule(), ID, overloadTys); + assert(newF->getFunctionType() == newfType); + newF->setCallingConv(call->getCallingConv()); + return newF; +} + + +//Used for allocation hoisting in *boxed static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsigned ToAS) { for (auto *User : Val->users()) { @@ -3216,13 +3254,8 @@ static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsig recursively_adjust_ptr_type(Inst, FromAS, ToAS); } else if (isa(User)) { - IntrinsicInst *II = cast(User); - SmallVector ArgTys; - Intrinsic::getIntrinsicSignature(II->getCalledFunction(), ArgTys); - assert(ArgTys.size() <= II->arg_size()); - for (size_t i = 0; i < ArgTys.size(); ++i) - ArgTys[i] = II->getArgOperand(i)->getType(); - II->setCalledFunction(Intrinsic::getDeclaration(II->getModule(), II->getIntrinsicID(), ArgTys)); + IntrinsicInst *call = cast(User); + call->setCalledFunction(mangleIntrinsic(call)); } #ifndef JL_LLVM_OPAQUE_POINTERS else if (isa(User)) { @@ -3238,7 +3271,7 @@ static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsig // dynamically-typed value is required (e.g. argument to unknown function). // if it's already a pointer it's left alone. // Returns ctx.types().T_prjlvalue -static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo) +static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo, bool is_promotable) { jl_value_t *jt = vinfo.typ; if (jt == jl_bottom_type || jt == NULL) @@ -3268,7 +3301,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo) box = _boxed_special(ctx, vinfo, t); if (!box) { bool do_promote = vinfo.promotion_point; - if (do_promote) { + if (do_promote && is_promotable) { auto IP = ctx.builder.saveIP(); ctx.builder.SetInsertPoint(vinfo.promotion_point); box = emit_allocobj(ctx, jl_datatype_size(jt), literal_pointer_val(ctx, (jl_value_t*)jt)); @@ -3282,7 +3315,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo) recursively_adjust_ptr_type(originalAlloca, 0, AddressSpace::Derived); originalAlloca->replaceAllUsesWith(decayed); // end illegal IR - cast(vinfo.V)->eraseFromParent(); + originalAlloca->eraseFromParent(); ctx.builder.restoreIP(IP); } else { box = emit_allocobj(ctx, jl_datatype_size(jt), literal_pointer_val(ctx, (jl_value_t*)jt)); @@ -3651,7 +3684,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg if (fval_info.typ == jl_bottom_type) return jl_cgval_t(); // TODO: Use (post-)domination instead. - bool field_promotable = !init_as_value && fval_info.promotion_ssa != -1 && + bool field_promotable = !jl_is_uniontype(jtype) && !init_as_value && fval_info.promotion_ssa != -1 && fval_info.promotion_point && fval_info.promotion_point->getParent() == ctx.builder.GetInsertBlock(); if (field_promotable) { savedIP = ctx.builder.saveIP(); @@ -3684,20 +3717,20 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg promotion_point = inst; promotion_ssa = fval_info.promotion_ssa; } - } else if (!promotion_point) { + } + else if (!promotion_point) { promotion_point = inst; } } Value *fval = NULL; if (jl_field_isptr(sty, i)) { - fval = boxed(ctx, fval_info); + fval = boxed(ctx, fval_info, field_promotable); if (!init_as_value) cast(tbaa_decorate(ctx.tbaa().tbaa_stack, ctx.builder.CreateAlignedStore(fval, dest, Align(jl_field_align(sty, i))))) ->setOrdering(AtomicOrdering::Unordered); } else if (jl_is_uniontype(jtype)) { - assert(!field_promotable); // compute tindex from rhs jl_cgval_t rhs_union = convert_julia_type(ctx, fval_info, jtype); if (rhs_union.typ == jl_bottom_type) diff --git a/src/codegen.cpp b/src/codegen.cpp index 03cb6806a988d..b0417d254b40c 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -5001,7 +5001,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_ expr_t = (jl_value_t*)jl_any_type; else { expr_t = jl_is_long(ctx.source->ssavaluetypes) ? (jl_value_t*)jl_any_type : jl_array_ptr_ref(ctx.source->ssavaluetypes, ssaidx_0based); - is_promotable = ctx.ssavalue_usecount[ssaidx_0based] == 1; + is_promotable = ctx.ssavalue_usecount.at(ssaidx_0based) == 1; } jl_cgval_t res = emit_call(ctx, ex, expr_t, is_promotable); // some intrinsics (e.g. typeassert) can return a wider type @@ -5119,7 +5119,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_ else if (head == jl_new_sym) { bool is_promotable = false; if (ssaidx_0based >= 0) { - is_promotable = ctx.ssavalue_usecount[ssaidx_0based] == 1; + is_promotable = ctx.ssavalue_usecount.at(ssaidx_0based) == 1; } assert(nargs > 0); jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs); diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl index 74938e5d635ca..5db536af6222f 100644 --- a/test/compiler/codegen.jl +++ b/test/compiler/codegen.jl @@ -747,3 +747,10 @@ f_donotdelete_input(x) = Base.donotdelete(x+1) f_donotdelete_const() = Base.donotdelete(1+1) @test occursin("call void (...) @jl_f_donotdelete(i64", get_llvm(f_donotdelete_input, Tuple{Int64}, true, false, false)) @test occursin("call void (...) @jl_f_donotdelete()", get_llvm(f_donotdelete_const, Tuple{}, true, false, false)) + +# Test 45476 fixes +struct MaybeTuple45476 + val::Union{Nothing, Tuple{Float32}} +end + +@test MaybeTuple45476((0,)).val[1] == 0f0