Skip to content

Commit

Permalink
Correct codegen bugs introduced by allocation-hoisting-PR (JuliaLang#…
Browse files Browse the repository at this point in the history
…45476)

Co-authored-by: Jameson Nash <[email protected]>
  • Loading branch information
2 people authored and Francesco Fucci committed Aug 11, 2022
1 parent 54315a9 commit c5a8df8
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 17 deletions.
63 changes: 48 additions & 15 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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<Type*, 8> 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<Type*, 4> overloadTys;
{
SmallVector<Intrinsic::IITDescriptor, 8> Table;
getIntrinsicInfoTableEntries(ID, Table);
ArrayRef<Intrinsic::IITDescriptor> 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()) {
Expand All @@ -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<IntrinsicInst>(User)) {
IntrinsicInst *II = cast<IntrinsicInst>(User);
SmallVector<Type*, 3> 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<IntrinsicInst>(User);
call->setCalledFunction(mangleIntrinsic(call));
}
#ifndef JL_LLVM_OPAQUE_POINTERS
else if (isa<BitCastInst>(User)) {
Expand All @@ -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)
Expand Down Expand Up @@ -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));
Expand All @@ -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<Instruction>(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));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<StoreInst>(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)
Expand Down
4 changes: 2 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c5a8df8

Please sign in to comment.