Skip to content

Commit

Permalink
in codegen, use StructRet where appropriate
Browse files Browse the repository at this point in the history 
it seems that the llvm 3.3 return type legalizer may not be properly
handling large types. this is more efficient anyways.

fixes #8932, should fix #12163, probably fixes #7434

(cherry picked from commit 13c83db)
  • Loading branch information
@vtjnash
vtjnash committed Sep 17, 2015
1 parent 1af871b commit 889e31f
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 26 deletions.
6 changes: 6 additions & 0 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,12 @@ static Value *mark_julia_type(Value *v, jl_value_t *jt)
return v;
}

static bool deserves_sret(jl_value_t *dt, Type *T)
{
assert(jl_is_datatype(dt));
return jl_datatype_size(dt) > sizeof(void*) && !T->isFloatingPointTy();
}

// --- generating various field accessors ---

static Value *emit_nthptr_addr(Value *v, ssize_t n)
Expand Down
123 changes: 97 additions & 26 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ typedef struct {
std::string funcName;
jl_sym_t *vaName; // name of vararg argument
bool vaStack; // varargs stack-allocated
bool sret;
int nReqArgs;
int lineno;
std::vector<bool> boundsCheck;
Expand Down Expand Up @@ -2527,14 +2528,20 @@ static Value *emit_call_function_object(jl_function_t *f, Value *theF, Value *th
jl_value_t **args, size_t nargs,
jl_codectx_t *ctx)
{
Value *result;
if (f!=NULL && specialized && f->linfo!=NULL && f->linfo->specFunctionObject!=NULL) {
// emit specialized call site
Function *cf = (Function*)f->linfo->specFunctionObject;
FunctionType *cft = cf->getFunctionType();
size_t nfargs = cft->getNumParams();
Value **argvals = (Value**) alloca(nfargs*sizeof(Value*));
bool sret = cf->hasStructRetAttr();
unsigned idx = 0;
Value *result;
if (sret) {
result = emit_static_alloca(cft->getParamType(0)->getContainedType(0), ctx);
argvals[idx] = result;
idx++;
}
for(size_t i=0; i < nargs; i++) {
Type *at = cft->getParamType(idx);
jl_value_t *jt = jl_nth_slot_type(f->linfo->specTypes,i);
Expand Down Expand Up @@ -2571,13 +2578,15 @@ static Value *emit_call_function_object(jl_function_t *f, Value *theF, Value *th
idx++;
}
assert(idx == nfargs);
result = builder.CreateCall(prepare_call(cf), ArrayRef<Value*>(&argvals[0],nfargs));
result = mark_julia_type(emit_reg2mem(result, ctx), jl_ast_rettype(f->linfo, f->linfo->ast));
}
else {
result = emit_jlcall(theFptr, theF, &args[1], nargs, ctx);
CallInst *call = builder.CreateCall(prepare_call(cf), ArrayRef<Value*>(&argvals[0], nfargs));
call->setAttributes(cf->getAttributes());
if (sret)
result = builder.CreateLoad(result);
else
result = call;
return mark_julia_type(emit_reg2mem(result, ctx), jl_ast_rettype(f->linfo, f->linfo->ast));
}
return result;
return emit_jlcall(theFptr, theF, &args[1], nargs, ctx);
}

static Value *emit_is_function(Value *x, jl_codectx_t *ctx)
Expand Down Expand Up @@ -3643,6 +3652,7 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
jl_codectx_t ctx;
ctx.f = cw;
ctx.linfo = lam;
ctx.sret = false;
allocate_gc_frame(0, b0, &ctx);

// Save the Function object reference
Expand All @@ -3656,15 +3666,17 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
lam->cFunctionList = list2;

// See whether this function is specsig or jlcall
bool specsig;
bool specsig, jlfunc_sret;
Function *theFptr;
if (lam->specFunctionObject != NULL) {
theFptr = (Function*)lam->specFunctionObject;
specsig = true;
jlfunc_sret = theFptr->hasStructRetAttr();
}
else {
theFptr = (Function*)lam->functionObject;
specsig = false;
jlfunc_sret = false;
}
assert(theFptr);

Expand All @@ -3676,7 +3688,18 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
if (sret)
sretPtr = AI++; //const Argument &fArg = *AI++;

for (size_t i=0; i < nargs; i++) {
Value *result;
size_t FParamIndex = 0;
if (jlfunc_sret) {
if (sret)
result = sretPtr;
else
result = builder.CreateAlloca(theFptr->getFunctionType()->getParamType(0)->getContainedType(0));
args.push_back(result);
FParamIndex++;
}

for (size_t i = 0; i < nargs; i++) {
Value *val = AI++;
jl_value_t *jargty = jl_nth_slot_type(lam->specTypes, i);

Expand Down Expand Up @@ -3738,7 +3761,13 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
// Create the call
Value *r;
if (specsig) {
r = emit_reg2mem(builder.CreateCall(prepare_call(theFptr), ArrayRef<Value*>(args)), &ctx);
CallInst *call = builder.CreateCall(prepare_call(theFptr), ArrayRef<Value*>(args));
call->setAttributes(theFptr->getAttributes());
if (jlfunc_sret)
r = builder.CreateLoad(result);
else
r = call;
r = emit_reg2mem(r, &ctx);
}
else {
r = emit_jlcall(theFptr, literal_pointer_val((jl_value_t*)ff), 0, nargs, &ctx);
Expand All @@ -3752,6 +3781,9 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
r = boxed(r, &ctx, jlrettype);
}
}
else if (sret && jlfunc_sret) {
// nothing to do
}
else if (!type_is_ghost(crt)) {
if (sret)
prt = fargt_sig[0]->getContainedType(0); // sret is a PointerType
Expand Down Expand Up @@ -3815,7 +3847,7 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
}

// generate a julia-callable function that calls f (AKA lam)
static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Function *f)
static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Function *f, bool sret)
{
std::stringstream funcName;
const std::string &fname = f->getName().str();
Expand All @@ -3829,23 +3861,31 @@ static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Funct
funcName.str(), f->getParent());
addComdat(w);
Function::arg_iterator AI = w->arg_begin();
AI++; //const Argument &fArg = *AI++;
/* const Argument &fArg = */ *AI++;
Value *argArray = AI++;
//const Argument &argCount = *AI++;
/* const Argument &argCount = *AI++; */
BasicBlock *b0 = BasicBlock::Create(jl_LLVMContext, "top", w);

builder.SetInsertPoint(b0);
DebugLoc noDbg;
builder.SetCurrentDebugLocation(noDbg);

jl_codectx_t ctx;
ctx.f = w;
ctx.linfo = lam;
ctx.sret = false;
allocate_gc_frame(0, b0, &ctx);

size_t nargs = jl_array_dim0(jl_lam_args(ast));
size_t nfargs = f->getFunctionType()->getNumParams();
Value **args = (Value**) alloca(nfargs*sizeof(Value*));
unsigned idx = 0;
Value *result;
if (sret) {
result = builder.CreateAlloca(f->getFunctionType()->getParamType(0)->getContainedType(0));
args[idx] = result;
idx++;
}
for(size_t i=0; i < nargs; i++) {
jl_value_t *ty = jl_nth_slot_type(lam->specTypes, i);
Type *lty = julia_type_to_llvm(ty);
Expand All @@ -3867,9 +3907,19 @@ static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Funct
}
// TODO: consider pulling the function pointer out of fArg so these
// wrappers can be reused for different functions of the same type.
Value *r = builder.CreateCall(prepare_call(f), ArrayRef<Value*>(&args[0], nfargs));
if (r->getType() != jl_pvalue_llvmt) {
r = boxed(r, &ctx, jl_ast_rettype(lam, (jl_value_t*)ast));
CallInst *call = builder.CreateCall(prepare_call(f), ArrayRef<Value*>(&args[0], nfargs));
call->setAttributes(f->getAttributes());
Value *r;
if (sret || call->getType() != jl_pvalue_llvmt) {
jl_value_t *ty = jl_ast_rettype(lam, (jl_value_t*)ast);
if (sret)
r = builder.CreateLoad(result);
else
r = call;
r = boxed(r, &ctx, ty);
}
else {
r = call;
}

builder.CreateRet(r);
Expand Down Expand Up @@ -4022,8 +4072,15 @@ static Function *emit_function(jl_lambda_info_t *lam)
#endif
funcName << "_" << globalUnique++;

ctx.sret = false;
if (specsig) { // assumes !va
std::vector<Type*> fsig(0);
Type *rt = (jlrettype == (jl_value_t*)jl_void_type ? T_void : julia_type_to_llvm(jlrettype));
if (rt != jl_pvalue_llvmt && rt != T_void && deserves_sret(jlrettype, rt)) {
ctx.sret = true;
fsig.push_back(rt->getPointerTo());
rt = T_void;
}
for(size_t i=0; i < jl_nparams(lam->specTypes); i++) {
Type *ty = julia_type_to_llvm(jl_tparam(lam->specTypes,i));
if (type_is_ghost(ty)) {
Expand All @@ -4035,17 +4092,18 @@ static Function *emit_function(jl_lambda_info_t *lam)
ty = PointerType::get(ty,0);
fsig.push_back(ty);
}
Type *rt = (jlrettype == (jl_value_t*)jl_void_type ? T_void : julia_type_to_llvm(jlrettype));
f = Function::Create(FunctionType::get(rt, fsig, false),
imaging_mode ? GlobalVariable::InternalLinkage : GlobalVariable::ExternalLinkage,
funcName.str(), m);
if (ctx.sret)
f->addAttribute(1, Attribute::StructRet);
addComdat(f);
if (lam->specFunctionObject == NULL) {
lam->specFunctionObject = (void*)f;
lam->specFunctionID = jl_assign_functionID(f);
}
if (lam->functionObject == NULL) {
Function *fwrap = gen_jlcall_wrapper(lam, ast, f);
Function *fwrap = gen_jlcall_wrapper(lam, ast, f, ctx.sret);
lam->functionObject = (void*)fwrap;
lam->functionID = jl_assign_functionID(fwrap);
}
Expand Down Expand Up @@ -4211,7 +4269,7 @@ static Function *emit_function(jl_lambda_info_t *lam)
varinfo.dinfo = ctx.dbuilder->createParameterVariable(
SP, // Scope (current function will be fill in later)
argname->name, // Variable name
i+1, // Argument number (1-based)
ctx.sret + i + 1, // Argument number (1-based)
topfile, // File
ctx.lineno == -1 ? 0 : ctx.lineno, // Line
// Variable type
Expand All @@ -4226,15 +4284,15 @@ static Function *emit_function(jl_lambda_info_t *lam)
julia_type_to_di(varinfo.declType,ctx.dbuilder,specsig), // Variable type
false, // May be optimized out
0, // Flags (TODO: Do we need any)
i+1); // Argument number (1-based)
ctx.sret + i + 1); // Argument number (1-based)
#endif
}
if (va) {
#ifdef LLVM38
ctx.vars[ctx.vaName].dinfo = ctx.dbuilder->createParameterVariable(
SP, // Scope (current function will be fill in later)
ctx.vaName->name, // Variable name
nreq + 1, // Argument number (1-based)
ctx.sret + nreq + 1, // Argument number (1-based)
topfile, // File
ctx.lineno == -1 ? 0 : ctx.lineno, // Line (for now, use lineno of the function)
julia_type_to_di(ctx.vars[ctx.vaName].declType,ctx.dbuilder,false));
Expand All @@ -4248,7 +4306,7 @@ static Function *emit_function(jl_lambda_info_t *lam)
julia_type_to_di(ctx.vars[ctx.vaName].declType,ctx.dbuilder,false), // Variable type
false, // May be optimized out
0, // Flags (TODO: Do we need any)
nreq + 1); // Argument number (1-based)
ctx.sret + nreq + 1); // Argument number (1-based)
#endif
}
for(i=0; i < vinfoslen; i++) {
Expand Down Expand Up @@ -4479,6 +4537,8 @@ static Function *emit_function(jl_lambda_info_t *lam)
// step 12. move args into local variables
Function::arg_iterator AI = f->arg_begin();
argIdx = 0;
if (ctx.sret)
AI++; // skip sret slot
for(i=0; i < nreq; i++) {
jl_sym_t *s = jl_decl_var(jl_cellref(largs,i));
jl_varinfo_t &vi = ctx.vars[s];
Expand Down Expand Up @@ -4701,7 +4761,7 @@ static Function *emit_function(jl_lambda_info_t *lam)
if (jl_is_expr(stmt) && ((jl_expr_t*)stmt)->head == return_sym) {
jl_expr_t *ex = (jl_expr_t*)stmt;
Value *retval;
Type *retty = f->getReturnType();
Type *retty = ctx.sret ? f->getFunctionType()->getParamType(0)->getContainedType(0) : f->getReturnType();
if (retty == jl_pvalue_llvmt) {
retval = boxed(emit_expr(jl_exprarg(ex,0), &ctx, true),&ctx,expr_type(stmt,&ctx));
}
Expand All @@ -4714,7 +4774,10 @@ static Function *emit_function(jl_lambda_info_t *lam)
}
if (do_malloc_log && lno != -1)
mallocVisitLine(filename, lno);
if (retty == T_void)

if (ctx.sret)
builder.CreateStore(retval, ctx.f->arg_begin());
if (type_is_ghost(retty) || ctx.sret)
builder.CreateRetVoid();
else
builder.CreateRet(retval);
Expand Down Expand Up @@ -4800,8 +4863,15 @@ extern "C" void jl_fptr_to_llvm(void *fptr, jl_lambda_info_t *lam, int specsig)
std::string funcName = lam->name->name;
funcName = "julia_" + funcName;
if (specsig) { // assumes !va
jl_value_t *jlrettype = jl_ast_rettype(lam, (jl_value_t*)lam->ast);
std::vector<Type*> fsig(0);
jl_value_t *jlrettype = jl_ast_rettype(lam, (jl_value_t*)lam->ast);
Type *rt = (jlrettype == (jl_value_t*)jl_void_type ? T_void : julia_type_to_llvm(jlrettype));
bool sret = false;
if (rt != jl_pvalue_llvmt && rt != T_void && deserves_sret(jlrettype, rt)) {
sret = true;
fsig.push_back(rt->getPointerTo());
rt = T_void;
}
for (size_t i=0; i < jl_nparams(lam->specTypes); i++) {
Type *ty = julia_type_to_llvm(jl_tparam(lam->specTypes,i));
if (type_is_ghost(ty))
Expand All @@ -4810,9 +4880,10 @@ extern "C" void jl_fptr_to_llvm(void *fptr, jl_lambda_info_t *lam, int specsig)
ty = PointerType::get(ty,0);
fsig.push_back(ty);
}
Type *rt = (jlrettype == (jl_value_t*)jl_void_type ? T_void : julia_type_to_llvm(jlrettype));
Function *f = Function::Create(FunctionType::get(rt, fsig, false), Function::ExternalLinkage, funcName,
shadow_module);
if (sret)
f->addAttribute(1, Attribute::StructRet);

if (lam->specFunctionObject == NULL) {
lam->specFunctionObject = (void*)f;
Expand Down
24 changes: 24 additions & 0 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3344,3 +3344,27 @@ end
typealias B11888{T} A11888{A11888{A11888{T}}}

@test sizeof(B11888{B11888{Int64}}) == (1 << 24) * 8

# issue #13175
immutable EmptyImmutable13175 end
immutable EmptyIIOtherField13175
x::EmptyImmutable13175
y::Float64
end
@test EmptyIIOtherField13175(EmptyImmutable13175(), 1.0) == EmptyIIOtherField13175(EmptyImmutable13175(), 1.0)
@test EmptyIIOtherField13175(EmptyImmutable13175(), 1.0) != EmptyIIOtherField13175(EmptyImmutable13175(), 2.0)

# issue #13183
gg13183{X}(x::X...) = 1==0 ? gg13183(x, x) : 0
@test gg13183(5) == 0

# issue 8932 (llvm return type legalizer error)
immutable Vec3_8932
x::Float32
y::Float32
z::Float32
end
f8932(a::Vec3_8932, b::Vec3_8932) = Vec3_8932(a.x % b.x, a.y % b.y, a.z % b.z)
a8932 = Vec3_8932(1,1,1)
b8932 = Vec3_8932(2,2,2)
@test f8932(a8932, b8932) == Vec3_8932(1.0, 1.0, 1.0)

4 comments on commit 889e31f

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I'll pull this into my rollup actually

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was a small conflict with one of the other backports on tk/backports3 but easy for me to resolve this time - ref 6457fd3

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yes. i see it pulled in too many tests. thanks for fixing that.

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They'd already been backported with different commits. The conflict was in code, something around the size_t FParamIndex = 0; IIRC, but trivial to resolve.

Please sign in to comment.