Skip to content

Commit d75843d

Browse files
gbaraldivtjnash
andauthored
Correct codegen bugs introduced by allocation-hoisting-PR (#45476)
Co-authored-by: Jameson Nash <[email protected]>
1 parent 46a6f22 commit d75843d

File tree

3 files changed

+57
-17
lines changed

3 files changed

+57
-17
lines changed

src/cgutils.cpp

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ static LoadInst *emit_nthptr_recast(jl_codectx_t &ctx, Value *v, ssize_t n, MDNo
10051005
emit_bitcast(ctx, vptr, PointerType::get(type, 0)))));
10061006
}
10071007

1008-
static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &v);
1008+
static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &v, bool is_promotable=false);
10091009
static Value *emit_typeof(jl_codectx_t &ctx, Value *v, bool maybenull);
10101010

10111011
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
32073207
return box_merge;
32083208
}
32093209

3210+
static Function *mangleIntrinsic(IntrinsicInst *call) //mangling based on replaceIntrinsicUseWith
3211+
{
3212+
Intrinsic::ID ID = call->getIntrinsicID();
3213+
auto nargs = call->arg_size();
3214+
SmallVector<Type*, 8> argTys(nargs);
3215+
auto oldfType = call->getFunctionType();
3216+
for (unsigned i = 0; i < oldfType->getNumParams(); i++) {
3217+
auto argi = call->getArgOperand(i);
3218+
argTys[i] = argi->getType();
3219+
}
3220+
3221+
auto newfType = FunctionType::get(
3222+
oldfType->getReturnType(),
3223+
makeArrayRef(argTys).slice(0, oldfType->getNumParams()),
3224+
oldfType->isVarArg());
3225+
3226+
// Accumulate an array of overloaded types for the given intrinsic
3227+
// and compute the new name mangling schema
3228+
SmallVector<Type*, 4> overloadTys;
3229+
{
3230+
SmallVector<Intrinsic::IITDescriptor, 8> Table;
3231+
getIntrinsicInfoTableEntries(ID, Table);
3232+
ArrayRef<Intrinsic::IITDescriptor> TableRef = Table;
3233+
auto res = Intrinsic::matchIntrinsicSignature(newfType, TableRef, overloadTys);
3234+
assert(res == Intrinsic::MatchIntrinsicTypes_Match);
3235+
(void)res;
3236+
bool matchvararg = !Intrinsic::matchIntrinsicVarArg(newfType->isVarArg(), TableRef);
3237+
assert(matchvararg);
3238+
(void)matchvararg;
3239+
}
3240+
auto newF = Intrinsic::getDeclaration(call->getModule(), ID, overloadTys);
3241+
assert(newF->getFunctionType() == newfType);
3242+
newF->setCallingConv(call->getCallingConv());
3243+
return newF;
3244+
}
3245+
3246+
3247+
//Used for allocation hoisting in *boxed
32103248
static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsigned ToAS)
32113249
{
32123250
for (auto *User : Val->users()) {
@@ -3216,13 +3254,8 @@ static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsig
32163254
recursively_adjust_ptr_type(Inst, FromAS, ToAS);
32173255
}
32183256
else if (isa<IntrinsicInst>(User)) {
3219-
IntrinsicInst *II = cast<IntrinsicInst>(User);
3220-
SmallVector<Type*, 3> ArgTys;
3221-
Intrinsic::getIntrinsicSignature(II->getCalledFunction(), ArgTys);
3222-
assert(ArgTys.size() <= II->arg_size());
3223-
for (size_t i = 0; i < ArgTys.size(); ++i)
3224-
ArgTys[i] = II->getArgOperand(i)->getType();
3225-
II->setCalledFunction(Intrinsic::getDeclaration(II->getModule(), II->getIntrinsicID(), ArgTys));
3257+
IntrinsicInst *call = cast<IntrinsicInst>(User);
3258+
call->setCalledFunction(mangleIntrinsic(call));
32263259
}
32273260
#ifndef JL_LLVM_OPAQUE_POINTERS
32283261
else if (isa<BitCastInst>(User)) {
@@ -3238,7 +3271,7 @@ static void recursively_adjust_ptr_type(llvm::Value *Val, unsigned FromAS, unsig
32383271
// dynamically-typed value is required (e.g. argument to unknown function).
32393272
// if it's already a pointer it's left alone.
32403273
// Returns ctx.types().T_prjlvalue
3241-
static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
3274+
static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo, bool is_promotable)
32423275
{
32433276
jl_value_t *jt = vinfo.typ;
32443277
if (jt == jl_bottom_type || jt == NULL)
@@ -3268,7 +3301,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
32683301
box = _boxed_special(ctx, vinfo, t);
32693302
if (!box) {
32703303
bool do_promote = vinfo.promotion_point;
3271-
if (do_promote) {
3304+
if (do_promote && is_promotable) {
32723305
auto IP = ctx.builder.saveIP();
32733306
ctx.builder.SetInsertPoint(vinfo.promotion_point);
32743307
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)
32823315
recursively_adjust_ptr_type(originalAlloca, 0, AddressSpace::Derived);
32833316
originalAlloca->replaceAllUsesWith(decayed);
32843317
// end illegal IR
3285-
cast<Instruction>(vinfo.V)->eraseFromParent();
3318+
originalAlloca->eraseFromParent();
32863319
ctx.builder.restoreIP(IP);
32873320
} else {
32883321
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
36513684
if (fval_info.typ == jl_bottom_type)
36523685
return jl_cgval_t();
36533686
// TODO: Use (post-)domination instead.
3654-
bool field_promotable = !init_as_value && fval_info.promotion_ssa != -1 &&
3687+
bool field_promotable = !jl_is_uniontype(jtype) && !init_as_value && fval_info.promotion_ssa != -1 &&
36553688
fval_info.promotion_point && fval_info.promotion_point->getParent() == ctx.builder.GetInsertBlock();
36563689
if (field_promotable) {
36573690
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
36843717
promotion_point = inst;
36853718
promotion_ssa = fval_info.promotion_ssa;
36863719
}
3687-
} else if (!promotion_point) {
3720+
}
3721+
else if (!promotion_point) {
36883722
promotion_point = inst;
36893723
}
36903724
}
36913725
Value *fval = NULL;
36923726
if (jl_field_isptr(sty, i)) {
3693-
fval = boxed(ctx, fval_info);
3727+
fval = boxed(ctx, fval_info, field_promotable);
36943728
if (!init_as_value)
36953729
cast<StoreInst>(tbaa_decorate(ctx.tbaa().tbaa_stack,
36963730
ctx.builder.CreateAlignedStore(fval, dest, Align(jl_field_align(sty, i)))))
36973731
->setOrdering(AtomicOrdering::Unordered);
36983732
}
36993733
else if (jl_is_uniontype(jtype)) {
3700-
assert(!field_promotable);
37013734
// compute tindex from rhs
37023735
jl_cgval_t rhs_union = convert_julia_type(ctx, fval_info, jtype);
37033736
if (rhs_union.typ == jl_bottom_type)

src/codegen.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5001,7 +5001,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
50015001
expr_t = (jl_value_t*)jl_any_type;
50025002
else {
50035003
expr_t = jl_is_long(ctx.source->ssavaluetypes) ? (jl_value_t*)jl_any_type : jl_array_ptr_ref(ctx.source->ssavaluetypes, ssaidx_0based);
5004-
is_promotable = ctx.ssavalue_usecount[ssaidx_0based] == 1;
5004+
is_promotable = ctx.ssavalue_usecount.at(ssaidx_0based) == 1;
50055005
}
50065006
jl_cgval_t res = emit_call(ctx, ex, expr_t, is_promotable);
50075007
// 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_
51195119
else if (head == jl_new_sym) {
51205120
bool is_promotable = false;
51215121
if (ssaidx_0based >= 0) {
5122-
is_promotable = ctx.ssavalue_usecount[ssaidx_0based] == 1;
5122+
is_promotable = ctx.ssavalue_usecount.at(ssaidx_0based) == 1;
51235123
}
51245124
assert(nargs > 0);
51255125
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs);

test/compiler/codegen.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,3 +747,10 @@ f_donotdelete_input(x) = Base.donotdelete(x+1)
747747
f_donotdelete_const() = Base.donotdelete(1+1)
748748
@test occursin("call void (...) @jl_f_donotdelete(i64", get_llvm(f_donotdelete_input, Tuple{Int64}, true, false, false))
749749
@test occursin("call void (...) @jl_f_donotdelete()", get_llvm(f_donotdelete_const, Tuple{}, true, false, false))
750+
751+
# Test 45476 fixes
752+
struct MaybeTuple45476
753+
val::Union{Nothing, Tuple{Float32}}
754+
end
755+
756+
@test MaybeTuple45476((0,)).val[1] == 0f0

0 commit comments

Comments
 (0)