Skip to content

Commit

Permalink
codegen: fix segfaults with VecElement with union element (#34805)
Browse files Browse the repository at this point in the history
We were being too aggressive at unwrapping these: since they are already
unwrapped once, they need to be treated almost as a normal struct.

fixes #29864

(cherry picked from commit 8720530)
  • Loading branch information
vtjnash authored and KristofferC committed Feb 19, 2020
1 parent d9d6364 commit 967bc47
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 23 deletions.
6 changes: 4 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -694,8 +694,8 @@ static Type *julia_struct_to_llvm(jl_value_t *jt, jl_unionall_t *ua, bool *isbox
assert(jst->layout == NULL); // otherwise should have been caught above
decl = T_void;
}
else if (jl_is_vecelement_type(jt)) {
// VecElement type is unwrapped in LLVM
else if (jl_is_vecelement_type(jt) && !jl_is_uniontype(jl_svecref(ftypes, 0))) {
// VecElement type is unwrapped in LLVM (when possible)
decl = latypes[0];
}
else if (isarray && !type_is_ghost(lasttype)) {
Expand Down Expand Up @@ -2755,6 +2755,8 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
}
llvm_idx = ptindex;
fval = tindex;
if (jl_is_vecelement_type(ty))
fval = ctx.builder.CreateInsertValue(strct, fval, makeArrayRef(llvm_idx));
}
else {
Value *ptindex = emit_struct_gep(ctx, lt, strct, offs + fsz);
Expand Down
11 changes: 6 additions & 5 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ static inline jl_cgval_t mark_julia_type(jl_codectx_t &ctx, Value *v, bool isbox
if (type_is_ghost(T)) {
return ghostValue(typ);
}
if (v && !isboxed && v->getType()->isAggregateType() && CountTrackedPointers(v->getType()).count == 0) {
if (v && !isboxed && v->getType()->isAggregateType() && !jl_is_vecelement_type(typ) && CountTrackedPointers(v->getType()).count == 0) {
// eagerly put this back onto the stack
// llvm mem2reg pass will remove this if unneeded
return value_to_pointer(ctx, v, typ, NULL);
Expand Down Expand Up @@ -6575,16 +6575,17 @@ static std::unique_ptr<Module> emit_function(
}
}
else {
Type *store_ty = julia_type_to_llvm(retvalinfo.typ);
Type *store_ty = retvalinfo.V->getType();
Type *dest_ty = store_ty->getPointerTo();
Value *Val = emit_unbox(ctx, store_ty, retvalinfo, retvalinfo.typ);
Value *Val = retvalinfo.V;
if (returninfo.return_roots) {
assert(store_ty == Val->getType());
assert(julia_type_to_llvm(retvalinfo.typ) == store_ty);
emit_sret_roots(ctx, false, Val, store_ty, f->arg_begin() + 1, returninfo.return_roots);
}
if (dest_ty != sret->getType())
sret = emit_bitcast(ctx, sret, dest_ty);
ctx.builder.CreateStore(Val, sret);
ctx.builder.CreateAlignedStore(Val, sret, julia_alignment(retvalinfo.typ));
assert(retvalinfo.TIndex == NULL && "unreachable"); // unimplemented representation
}
}

Expand Down
29 changes: 13 additions & 16 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,25 +156,22 @@ static Constant *julia_const_to_llvm(const void *ptr, jl_datatype_t *bt)
if (bt == jl_bool_type)
return ConstantInt::get(T_int8, (*(const uint8_t*)ptr) ? 1 : 0);

if (jl_is_vecelement_type((jl_value_t*)bt))
bt = (jl_datatype_t*)jl_tparam0(bt);

Type *lt = julia_struct_to_llvm((jl_value_t*)bt, NULL, NULL);

if (type_is_ghost(lt))
return UndefValue::get(lt);

if (jl_is_primitivetype(bt)) {
if (lt->isFloatTy()) {
uint32_t data32 = *(const uint32_t*)ptr;
return ConstantFP::get(jl_LLVMContext,
APFloat(lt->getFltSemantics(), APInt(32, data32)));
}
if (lt->isDoubleTy()) {
uint64_t data64 = *(const uint64_t*)ptr;
return ConstantFP::get(jl_LLVMContext,
APFloat(lt->getFltSemantics(), APInt(64, data64)));
}
if (lt->isFloatTy()) {
uint32_t data32 = *(const uint32_t*)ptr;
return ConstantFP::get(jl_LLVMContext,
APFloat(lt->getFltSemantics(), APInt(32, data32)));
}
if (lt->isDoubleTy()) {
uint64_t data64 = *(const uint64_t*)ptr;
return ConstantFP::get(jl_LLVMContext,
APFloat(lt->getFltSemantics(), APInt(64, data64)));
}
if (lt->isFloatingPointTy() || lt->isIntegerTy()) {
int nb = jl_datatype_size(bt);
APInt val(8 * nb, 0);
void *bits = const_cast<uint64_t*>(val.getRawData());
Expand Down Expand Up @@ -335,9 +332,9 @@ static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_va

Constant *c = x.constant ? julia_const_to_llvm(x.constant) : NULL;
if (!x.ispointer() || c) { // already unboxed, but sometimes need conversion
Value *unboxed = emit_unboxed_coercion(ctx, to, c ? c : x.V);
Value *unboxed = c ? c : x.V;
if (!dest)
return unboxed;
return emit_unboxed_coercion(ctx, to, unboxed);
Type *dest_ty = unboxed->getType()->getPointerTo();
if (dest->getType() != dest_ty)
dest = emit_bitcast(ctx, dest, dest_ty);
Expand Down
7 changes: 7 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,10 @@ function f33590(b, x)
end
@test f33590(true, (3,)) == (3,)
@test f33590(false, (3,)) == (4,)

# issue 29864
const c29864 = VecElement{Union{Int,Nothing}}(2)
@noinline f29864() = c29864
@noinline g29864() = VecElement{Union{Int,Nothing}}(3)
@test f29864().value === 2
@test g29864().value === 3

0 comments on commit 967bc47

Please sign in to comment.