From ccd1428242b538118674aec9d9d5312b287087b5 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 9 Jun 2015 20:10:43 -0400 Subject: [PATCH] store tuple and vector types to the stack eagerly fix #11187 (pass struct and tuple objects by stack pointer) fix #11450 (ccall emission was frobbing the stack) likely may fix #11026 and may fix #11003 (ref #10525) invalid stack-read on 32-bit this additionally changes the julia specSig calling convention to pass non-primitive types by pointer instead of by-value this additionally fixes a bug in gen_cfunction that could be exposed by turning off specSig this additionally moves the alloca calls in ccall (and other places) to the entry BasicBlock in the function, ensuring that llvm detects them as static allocations and moves them into the function prologue this additionally fixes some undefined behavior from changing a variable's size through a alloca-cast instead of zext/sext/trunc this additionally prepares for turning back on allocating tuples as vectors, since the gc now guarantees 16-byte alignment future work this makes possible: - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure) - allow moving pointers sometimes rather than always copying immutable data - teach the GC how it can re-use an existing pointer as a box --- src/ccall.cpp | 16 +++++++--------- src/cgutils.cpp | 6 +----- src/codegen.cpp | 4 ++-- src/intrinsics.cpp | 6 +++--- test/ccall.jl | 2 +- 5 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/ccall.cpp b/src/ccall.cpp index be4ecd9b632fe2..130a57bb23cd34 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -255,7 +255,7 @@ Value *llvm_type_rewrite(Value *v, Type *from_type, Type *target_type, bool toju if (v->getType() != ptarget_type) { v = builder.CreatePointerCast(v, ptarget_type); } - return builder.CreateAlignedLoad(v, 1); // unknown alignment from C + return builder.CreateLoad(v); } } else { @@ -267,11 +267,10 @@ Value *llvm_type_rewrite(Value *v, Type *from_type, Type *target_type, bool toju } if (v->getType() != from_type) { // this is already be a pointer in the codegen - unsigned align = v->getType() == jl_pvalue_llvmt ? 16 : 0; if (v->getType() != ptarget_type) { v = builder.CreatePointerCast(v, ptarget_type); } - return builder.CreateAlignedLoad(v, align); + return builder.CreateLoad(v); } } assert(v->getType() == from_type); @@ -341,7 +340,7 @@ static Value *julia_to_native(Type *ty, jl_value_t *jt, Value *jv, if (vt != jl_pvalue_llvmt) { // argument value is unboxed if (vt != jv->getType()) - jv = builder.CreateLoad(jv); // something stack allocated + jv = builder.CreateLoad(jv); if (addressOf || (byRef && inReg)) { if (ty->isPointerTy() && ty->getContainedType(0) == vt) { // pass the address of an alloca'd thing, not a box @@ -411,7 +410,7 @@ static Value *julia_to_native(Type *ty, jl_value_t *jt, Value *jv, *needStackRestore = true; AllocaInst *ai = builder.CreateAlloca(T_int8, nbytes); ai->setAlignment(16); - builder.CreateMemCpy(ai, builder.CreateBitCast(jv, T_pint8), nbytes, 16); + builder.CreateMemCpy(ai, builder.CreateBitCast(jv, T_pint8), nbytes, sizeof(void*)); // minimum gc-alignment in julia is pointer size return builder.CreateBitCast(ai, ty); } // emit maybe copy @@ -437,7 +436,7 @@ static Value *julia_to_native(Type *ty, jl_value_t *jt, Value *jv, false)); AllocaInst *ai = builder.CreateAlloca(T_int8, nbytes); ai->setAlignment(16); - builder.CreateMemCpy(ai, builder.CreatePointerCast(jv, T_pint8), nbytes, 16); + builder.CreateMemCpy(ai, builder.CreatePointerCast(jv, T_pint8), nbytes, sizeof(void*)); // minimum gc-alignment in julia is pointer size Value *p2 = builder.CreatePointerCast(ai, ty); builder.CreateBr(afterBB); builder.SetInsertPoint(afterBB); @@ -1410,8 +1409,7 @@ static Value *emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx) Value *newst = emit_new_struct(rt,1,NULL,ctx); assert(newst != NULL && "Type was not concrete"); assert(newst->getType()->isPointerTy()); - // julia gc is aligned 16, otherwise use default alignment for alloca pointers - builder.CreateAlignedStore(result, builder.CreateBitCast(newst, prt->getPointerTo()), newst->getType()==jl_pvalue_llvmt ? 16 : 0); + builder.CreateStore(result, builder.CreateBitCast(newst, prt->getPointerTo())); result = newst; } else { @@ -1432,7 +1430,7 @@ static Value *emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx) } else { if (result->getType() != jl_pvalue_llvmt && !lrt->isAggregateType()) - result = builder.CreateLoad(result); // something alloca'd above + result = builder.CreateLoad(result); } return mark_or_box_ccall_result(result, args[2], rt, static_rt, ctx); diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 2e25f83eefc42d..8146f5bd7aea57 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -1151,8 +1151,6 @@ static Value *typed_load(Value *ptr, Value *idx_0based, jl_value_t *jltype, // elt = data; //} //else { - if (data->getType()->getContainedType(0)->isVectorTy() && !alignment) - alignment = ((jl_datatype_t*)jltype)->alignment; // prevent llvm from assuming 32 byte alignment of vectors Instruction *load = builder.CreateAlignedLoad(data, alignment, false); if (tbaa) { elt = tbaa_decorate(tbaa, load); @@ -1196,8 +1194,6 @@ static void typed_store(Value *ptr, Value *idx_0based, Value *rhs, data = builder.CreateBitCast(ptr, PointerType::get(elty, 0)); else data = ptr; - if (data->getType()->getContainedType(0)->isVectorTy() && !alignment) - alignment = ((jl_datatype_t*)jltype)->alignment; // prevent llvm from assuming 32 byte alignment of vectors Instruction *store = builder.CreateAlignedStore(rhs, builder.CreateGEP(data, idx_0based), alignment); if (tbaa) tbaa_decorate(tbaa, store); @@ -1622,7 +1618,7 @@ static Value *tpropagate(Value *a, Value *b) static Value *init_bits_value(Value *newv, Value *jt, Type *t, Value *v) { builder.CreateStore(jt, builder.CreateBitCast(emit_typeptr_addr(newv), jl_ppvalue_llvmt)); - builder.CreateAlignedStore(v, builder.CreateBitCast(data_pointer(newv), PointerType::get(t,0)), 16); // Julia's gc-alignment is 16-bytes + builder.CreateStore(v, builder.CreateBitCast(data_pointer(newv), PointerType::get(t,0))); return newv; } diff --git a/src/codegen.cpp b/src/codegen.cpp index b0c26f17d52dcb..edcc6f1f9f964f 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3816,7 +3816,7 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t Value *mem = emit_allocobj(jl_datatype_size(jargty)); builder.CreateStore(literal_pointer_val((jl_value_t*)jargty), emit_typeptr_addr(mem)); - builder.CreateAlignedStore(val, builder.CreateBitCast(mem, val->getType()->getPointerTo()), 16); // julia's gc gives 16-byte aligned addresses + builder.CreateStore(val, builder.CreateBitCast(mem, val->getType()->getPointerTo())); val = mem; } if (specsig) @@ -3858,7 +3858,7 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t } else if (!type_is_ghost(crt)) { if (sret) - prt = fargt_sig[0]; + prt = fargt_sig[0]->getContainedType(0); // sret is a PointerType Value *v = julia_to_native(crt, jlrettype, r, jlrettype, false, false, false, false, false, 0, &ctx, NULL); bool issigned = jl_signed_type && jl_subtype(jlrettype, (jl_value_t*)jl_signed_type, 0); r = llvm_type_rewrite(v, crt, prt, false, false, issigned, &ctx); diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 621a36a0380397..228574d5b6597a 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -266,7 +266,7 @@ static Value *emit_unbox(Type *to, Value *x, jl_value_t *jt) } if (ty != jl_pvalue_llvmt) { if (to->isAggregateType()) { - x = builder.CreateLoad(x); // something stack allocated + x = builder.CreateLoad(x); ty = x->getType(); } else { @@ -300,7 +300,7 @@ static Value *emit_unbox(Type *to, Value *x, jl_value_t *jt) assert(to != T_void); return UndefValue::get(to); } - return builder.CreateAlignedLoad(builder.CreateBitCast(p, to->getPointerTo()), 16); // julia's gc gives 16-byte aligned addresses + return builder.CreateLoad(builder.CreateBitCast(p, to->getPointerTo()), false); } // unbox trying to determine type automatically @@ -488,7 +488,7 @@ static Value *generic_box(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx) // dynamically-determined type; evaluate. if (llvmt->isAggregateType()) { - vx = builder.CreateLoad(vx); // something stack allocated + vx = builder.CreateLoad(vx); } return allocate_box_dynamic(emit_expr(targ, ctx), ConstantInt::get(T_size,nb), vx); } diff --git a/test/ccall.jl b/test/ccall.jl index 6784c97a45d008..ab28a36bff83d9 100644 --- a/test/ccall.jl +++ b/test/ccall.jl @@ -1,7 +1,7 @@ # This file is a part of Julia. License is MIT: http://julialang.org/license import Base.copy, Base.== -const verbose = true +const verbose = false ccall((:set_verbose, "./libccalltest"), Void, (Int32,), verbose) # Test for proper argument register truncation