Skip to content

Commit

Permalink
fix #5312
Browse files Browse the repository at this point in the history 
Previously, empty types (recursively defined as structs and tuples
that only contain other empty types) were all mapped to T_void, which
has no values. This caused problems for non-0-length "empty" tuples,
which are now represented as empty LLVM structs.

Part of this entails calling mark_julia_type with tuple types, which
then need to be GC rooted, so the typeToTypeId table is now an
ObjectIdDict.
  • Loading branch information
@JeffBezanson
JeffBezanson committed Jan 6, 2014
1 parent c7fbcd0 commit ced6490
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 24 deletions.
32 changes: 18 additions & 14 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,22 +224,20 @@ static Type *julia_type_to_llvm(jl_value_t *jt)
if (purebits) {
// Can't be bool due to
// http://llvm.org/bugs/show_bug.cgi?id=12618
if (isvector && type != T_int1) {
if (isvector && type != T_int1 && type != T_void) {
Type *ret = NULL;
if (type == T_void)
return T_void;
if (type->isSingleValueType() && !type->isVectorTy())
ret = VectorType::get(type,ntypes);
else
ret = ArrayType::get(type,ntypes);
return ret;
}
}
else {
Type *types[ntypes];
size_t j = 0;
for (size_t i = 0; i < ntypes; ++i) {
Type *ty = julia_struct_to_llvm(jl_tupleref(jt,i));
if (ty == T_void)
if (ty == T_void || ty->isEmptyTy())
continue;
types[j++] = ty;
}
Expand Down Expand Up @@ -302,9 +300,11 @@ static Type *julia_struct_to_llvm(jl_value_t *jt)
size_t i;
for(i = 0; i < ntypes; i++) {
jl_value_t *ty = jl_tupleref(jst->types, i);
Type *lty = ty==(jl_value_t*)jl_bool_type ? T_int8 : julia_type_to_llvm(ty);
Type *lty;
if (jst->fields[i].isptr)
lty = jl_pvalue_llvmt;
else
lty = ty==(jl_value_t*)jl_bool_type ? T_int8 : julia_type_to_llvm(ty);
latypes.push_back(lty);
}
structdecl->setBody(latypes);
Expand All @@ -328,6 +328,7 @@ static jl_value_t *llvm_type_to_julia(Type *t, bool throw_error)
if (t == T_float32) return (jl_value_t*)jl_float32_type;
if (t == T_float64) return (jl_value_t*)jl_float64_type;
if (t == T_void) return (jl_value_t*)jl_bottom_type;
if (t->isEmptyTy()) return (jl_value_t*)jl_nothing->type;
if (t == jl_pvalue_llvmt)
return (jl_value_t*)jl_any_type;
if (t->isPointerTy()) {
Expand All @@ -347,21 +348,24 @@ static jl_value_t *llvm_type_to_julia(Type *t, bool throw_error)
// --- scheme for tagging llvm values with julia types using metadata ---

static std::map<int, jl_value_t*> typeIdToType;
static std::map<jl_value_t*, int> typeToTypeId;
static jl_array_t *typeToTypeId;
static int cur_type_id = 1;

static int jl_type_to_typeid(jl_value_t *t)
{
std::map<jl_value_t*, int>::iterator it = typeToTypeId.find(t);
if (it == typeToTypeId.end()) {
jl_value_t *id = jl_eqtable_get(typeToTypeId, t, NULL);
if (id == NULL) {
int mine = cur_type_id++;
if (mine > 65025)
jl_error("internal compiler error: too many bits types");
typeToTypeId[t] = mine;
JL_GC_PUSH1(&id);
id = jl_box_long(mine);
jl_eqtable_put(typeToTypeId, t, id);
typeIdToType[mine] = t;
JL_GC_POP();
return mine;
}
return (*it).second;
return jl_unbox_long(id);
}

static jl_value_t *jl_typeid_to_type(int i)
Expand Down Expand Up @@ -797,7 +801,7 @@ static Value *emit_tupleset(Value *tuple, Value *i, Value *x, jl_value_t *jt, jl
for (size_t i=0,j = 0; i<n; ++i) {
Type *ty = julia_struct_to_llvm(jl_tupleref(jt,i));
if (ci == i) {
if (ty == T_void)
if (ty == T_void || ty->isEmptyTy())
return tuple;
else
ret = builder.CreateInsertValue(tuple,x,ArrayRef<unsigned>(j));
Expand Down Expand Up @@ -847,7 +851,7 @@ static Value *emit_tupleref(Value *tuple, Value *ival, jl_value_t *jt, jl_codect
for (size_t i = 0,j = 0; i<n; ++i) {
Type *ty = julia_struct_to_llvm(jl_tupleref(jt,i));
if (ci == i) {
if (ty == T_void)
if (ty == T_void || ty->isEmptyTy())
return mark_julia_type(UndefValue::get(NoopType),jl_tupleref(jt,i));
else
return mark_julia_type(builder.CreateExtractValue(tuple,ArrayRef<unsigned>(j)),jl_tupleref(jt,i));
Expand Down Expand Up @@ -1212,7 +1216,7 @@ static Value *boxed(Value *v, jl_codectx_t *ctx, jl_value_t *jt)
if (t == T_int1) return julia_bool(v);
if (jt == NULL || jl_is_uniontype(jt) || jl_is_abstracttype(jt))
jt = julia_type_of(v);
if (t == T_void) {
if (t == T_void || t->isEmptyTy()) {
jl_value_t *s = static_void_instance(jt);
if (jl_is_tuple(jt) && jl_tuple_len(jt) > 0)
jl_add_linfo_root(ctx->linfo, s);
Expand Down
25 changes: 15 additions & 10 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@ static Value *emit_known_call(jl_value_t *ff, jl_value_t **args, size_t nargs,
Type *ety = NULL;
if (tpl != NULL)
ety = jl_llvmtuple_eltype(tpl->getType(),tt,i);
if (tpl == NULL || ety == T_void) {
if (tpl == NULL || ety == T_void || ety->isEmptyTy()) {
emit_expr(args[i+1],ctx); //for side effects (if any)
continue;
}
Expand All @@ -1467,6 +1467,8 @@ static Value *emit_known_call(jl_value_t *ff, jl_value_t **args, size_t nargs,
}
JL_GC_POP();
JL_GC_POP();
if (ty->isEmptyTy())
return mark_julia_type(tpl, tt);
return tpl;
}

Expand Down Expand Up @@ -1802,7 +1804,7 @@ static Value *emit_call(jl_value_t **args, size_t arglen, jl_codectx_t *ctx,
for(size_t i=0; i < nargs; i++) {
Type *at = cft->getParamType(idx);
Type *et = julia_type_to_llvm(jl_tupleref(f->linfo->specTypes,i));
if (et == T_void) {
if (et == T_void || et->isEmptyTy()) {
// Still emit the expression in case it has side effects
emit_expr(args[i+1], ctx);
continue;
Expand Down Expand Up @@ -2313,7 +2315,7 @@ static Value *emit_expr(jl_value_t *expr, jl_codectx_t *ctx, bool isboxed,
for(size_t i=0; i < na; i++) {
jl_value_t *jtype = jl_tupleref(sty->types,i);
Type *fty = julia_type_to_llvm(jtype);
if (fty == T_void)
if (fty == T_void || fty->isEmptyTy())
continue;
Value *fval = emit_unbox(fty, emit_unboxed(args[i+1],ctx), jtype);
if (fty == T_int1)
Expand Down Expand Up @@ -2508,7 +2510,7 @@ static Value *alloc_local(jl_sym_t *s, jl_codectx_t *ctx)
assert(store_unboxed_p(s,ctx));
Type *vtype = julia_struct_to_llvm(jt);
assert(vtype != jl_pvalue_llvmt);
if (vtype != T_void) {
if (vtype != T_void && !vtype->isEmptyTy()) {
lv = builder.CreateAlloca(vtype, 0, s->name);
if (vtype != jl_pvalue_llvmt)
mark_julia_type(lv, jt);
Expand Down Expand Up @@ -2688,14 +2690,14 @@ static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Funct
((jl_datatype_t*)ty)->size > 0)) {
Type *lty = julia_struct_to_llvm(ty);
assert(lty != NULL);
if (lty == T_void)
if (lty == T_void || lty->isEmptyTy())
continue;
theNewArg = emit_unbox(lty, theArg, ty);
}
else if (jl_is_tuple(ty)) {
Type *lty = julia_struct_to_llvm(ty);
if (lty != jl_pvalue_llvmt) {
if (lty == T_void)
if (lty == T_void || lty->isEmptyTy())
continue;
theNewArg = emit_unbox(lty, theArg, ty);
}
Expand Down Expand Up @@ -2870,11 +2872,11 @@ static Function *emit_function(jl_lambda_info_t *lam, bool cstyle)
std::vector<Type*> fsig(0);
for(size_t i=0; i < jl_tuple_len(lam->specTypes); i++) {
Type *ty = julia_type_to_llvm(jl_tupleref(lam->specTypes,i));
if (ty != T_void) {
fsig.push_back(ty);
if (ty == T_void || ty->isEmptyTy()) {
ctx.vars[jl_decl_var(jl_cellref(largs,i))].isGhost = true;
}
else {
ctx.vars[jl_decl_var(jl_cellref(largs,i))].isGhost = true;
fsig.push_back(ty);
}
}
Type *rt = (jlrettype == (jl_value_t*)jl_nothing->type ? T_void : julia_type_to_llvm(jlrettype));
Expand Down Expand Up @@ -3372,7 +3374,7 @@ extern "C" void jl_fptr_to_llvm(void *fptr, jl_lambda_info_t *lam, int specsig)
std::vector<Type*> fsig(0);
for(size_t i=0; i < jl_tuple_len(lam->specTypes); i++) {
Type *ty = julia_type_to_llvm(jl_tupleref(lam->specTypes,i));
if (ty != T_void)
if (ty != T_void && !ty->isEmptyTy())
fsig.push_back(ty);
}
Type *rt = (jlrettype == (jl_value_t*)jl_nothing->type ? T_void : julia_type_to_llvm(jlrettype));
Expand Down Expand Up @@ -3902,6 +3904,9 @@ extern "C" void jl_init_codegen(void)
jl_Module);
jl_ExecutionEngine->addGlobalMapping(restore_arg_area_loc_func,
(void*)&restore_arg_area_loc);

typeToTypeId = jl_alloc_cell_1d(16);
jl_gc_preserve((jl_value_t*)typeToTypeId);
}

/*
Expand Down
4 changes: 4 additions & 0 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,10 @@ DLLEXPORT void jl_array_sizehint(jl_array_t *a, size_t sz);
DLLEXPORT void *jl_value_ptr(jl_value_t *a);
DLLEXPORT void jl_cell_1d_push(jl_array_t *a, jl_value_t *item);

// eq hash tables
DLLEXPORT jl_array_t *jl_eqtable_put(jl_array_t *h, void *key, void *val);
DLLEXPORT jl_value_t *jl_eqtable_get(jl_array_t *h, void *key, jl_value_t *deflt);

// system information
DLLEXPORT int jl_errno(void);
DLLEXPORT int32_t jl_stat(const char* path, char* statbuf);
Expand Down
10 changes: 10 additions & 0 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1227,3 +1227,13 @@ let
x = 65
@test [x, x|=0x20] == [65, 97]
end

# issue #5312
let
local x = 0
global incr5312, foo5312
incr5312() = (x+=1; nothing)
foo5312() = (incr5312(),)
@test foo5312() === (nothing,)
@test x == 1
end

1 comment on commit ced6490

@Keno
Copy link
Member

@Keno Keno commented on ced6490 Jan 6, 2014

Choose a reason for hiding this comment

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

Nice! I hadn't realized {} was a valid LLVM type.

Please sign in to comment.