From 8720530fae5e7f34003057fe487c871ca81560a0 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 19 Feb 2020 13:50:55 -0500 Subject: [PATCH] codegen: fix segfaults with VecElement with union element (#34805) 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 --- src/cgutils.cpp | 6 ++++-- src/codegen.cpp | 11 ++++++----- src/intrinsics.cpp | 29 +++++++++++++---------------- test/compiler/codegen.jl | 7 +++++++ 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 6da70b27819a9..c7a03edb91a9f 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -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)) { @@ -2747,6 +2747,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); diff --git a/src/codegen.cpp b/src/codegen.cpp index 4cc5d97d1d8d6..8aa267dfcfbeb 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -722,7 +722,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); @@ -6569,16 +6569,17 @@ static std::unique_ptr 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 } } diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 9aefeedc3942e..9e97d6b010026 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -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(val.getRawData()); @@ -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); diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl index ab1b262842f46..41e153e93544f 100644 --- a/test/compiler/codegen.jl +++ b/test/compiler/codegen.jl @@ -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