Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix compile=all and codegen bugs #22697

Merged
merged 1 commit into from
Jul 18, 2017
Merged

fix compile=all and codegen bugs #22697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,6 @@ export
# loading source files
__precompile__,
evalfile,
include,
Copy link
Contributor

Choose a reason for hiding this comment

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

what?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

This wasn't supposed to be exported – it causes binding overwrite warnings when you try to extend the system image with compile all.

Copy link
Contributor

Choose a reason for hiding this comment

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

could have mentioned that in the commit message or PR description, I suspect that's totally unclear to anyone other than you

include_string,
include_dependency,
reload,
Expand Down
2 changes: 1 addition & 1 deletion src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ static Value *runtime_apply_type(jl_codectx_t &ctx, jl_value_t *ty, jl_unionall_
args[1] = literal_pointer_val(ctx, (jl_value_t*)ctx.linfo->def.method->sig);
args[2] = ctx.builder.CreateInBoundsGEP(
T_prjlvalue,
emit_bitcast(ctx, decay_derived(ctx.spvals_ptr), T_pprjlvalue),
ctx.spvals_ptr,
Copy link
Member

Choose a reason for hiding this comment

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

Are these guaranteed to be globally rooted?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

They're always callee rooted

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Is it OK to just assert that the argument type is T_pprjlvalue?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Ping @Keno

Copy link
Member

Choose a reason for hiding this comment

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

I think you still need to decay_derived at the very least.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

On what?

Copy link
Member

Choose a reason for hiding this comment

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

ctx.spvals_ptr

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

It can just be considered an opaque (callee-rooted) pointer Type::getPointerTo(Void, 0). Unless the rooting pass cares that I'm loading a T_prjlvalue from it?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, as long as ctx.spvals_ptr is in addrspace(0) it's probably fine. The pass will just not root any values you load from it.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

That should be fine (preferred, actually). I assume it'll still wb them if they escape?

ConstantInt::get(T_size, sizeof(jl_svec_t) / sizeof(jl_value_t*)));
return ctx.builder.CreateCall(prepare_call(jlapplytype_func), makeArrayRef(args));
}
Expand Down
63 changes: 25 additions & 38 deletions src/cgutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,15 @@ static Value *stringConstPtr(IRBuilder<> &irbuilder, const std::string &txt)

static DIType *julia_type_to_di(jl_value_t *jt, DIBuilder *dbuilder, bool isboxed = false)
{
if (isboxed)
if (isboxed || !jl_is_datatype(jt))
return jl_pvalue_dillvmt;
jl_datatype_t *jdt = (jl_datatype_t*)jt;
// always return the boxed representation for types with hidden content
if (jl_is_abstracttype(jt) || !jl_is_datatype(jt) || jl_is_array_type(jt) ||
if (jl_is_abstracttype(jt) || jl_is_array_type(jt) ||
jt == (jl_value_t*)jl_sym_type || jt == (jl_value_t*)jl_module_type ||
jt == (jl_value_t*)jl_simplevector_type || jt == (jl_value_t*)jl_datatype_type ||
jt == (jl_value_t*)jl_method_instance_type)
return jl_pvalue_dillvmt;
if (jl_is_unionall(jt) || jl_is_typevar(jt))
return jl_pvalue_dillvmt;
assert(jl_is_datatype(jt));
jl_datatype_t *jdt = (jl_datatype_t*)jt;
if (jdt->ditype != NULL) {
DIType* t = (DIType*)jdt->ditype;
return t;
Expand All @@ -168,45 +165,35 @@ static DIType *julia_type_to_di(jl_value_t *jt, DIBuilder *dbuilder, bool isboxe
return t;
#endif
}
else if (!jl_is_leaf_type(jt)) {
jdt->ditype = jl_pvalue_dillvmt;
return jl_pvalue_dillvmt;
}
else if (jl_is_structtype(jt)) {
jl_datatype_t *jst = (jl_datatype_t*)jt;
size_t ntypes = jl_datatype_nfields(jst);
if (jl_is_structtype(jt) && jdt->layout) {
size_t ntypes = jl_datatype_nfields(jdt);
const char *tname = jl_symbol_name(jdt->name->name);
std::stringstream unique_name;
unique_name << tname << "_" << globalUnique++;
llvm::DICompositeType *ct = dbuilder->createStructType(
NULL, // Scope
tname, // Name
NULL, // File
0, // LineNumber
jl_datatype_nbits(jdt), // SizeInBits
8 * jl_datatype_align(jdt), // AlignInBits
DIFlagZero, // Flags
NULL, // DerivedFrom
DINodeArray(), // Elements
dwarf::DW_LANG_Julia, // RuntimeLanguage
nullptr, // VTableHolder
unique_name.str() // UniqueIdentifier
);
NULL, // Scope
tname, // Name
NULL, // File
0, // LineNumber
jl_datatype_nbits(jdt), // SizeInBits
8 * jl_datatype_align(jdt), // AlignInBits
DIFlagZero, // Flags
NULL, // DerivedFrom
DINodeArray(), // Elements
dwarf::DW_LANG_Julia, // RuntimeLanguage
nullptr, // VTableHolder
unique_name.str() // UniqueIdentifier
);
jdt->ditype = ct;
std::vector<llvm::Metadata*> Elements;
for(unsigned i = 0; i < ntypes; i++)
Elements.push_back(julia_type_to_di(jl_svecref(jst->types,i),dbuilder,false));
for (unsigned i = 0; i < ntypes; i++)
Elements.push_back(julia_type_to_di(jl_svecref(jdt->types, i), dbuilder, false));
dbuilder->replaceArrays(ct, dbuilder->getOrCreateArray(ArrayRef<Metadata*>(Elements)));
return ct;
}
else {
assert(jl_is_datatype(jt));
jdt->ditype = dbuilder->createTypedef(jl_pvalue_dillvmt,
jdt->ditype = dbuilder->createTypedef(jl_pvalue_dillvmt,
jl_symbol_name(jdt->name->name), NULL, 0, NULL);
return (llvm::DIType*)jdt->ditype;
}
// TODO: Fixme
return jl_pvalue_dillvmt;
return (llvm::DIType*)jdt->ditype;
}

static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
Expand Down Expand Up @@ -452,8 +439,8 @@ static Type *julia_struct_to_llvm(jl_value_t *jt, jl_unionall_t *ua, bool *isbox
if (jl_is_primitivetype(jt))
return bitstype_to_llvm(jt);
bool isTuple = jl_is_tuple_type(jt);
if ((isTuple || jl_is_structtype(jt)) && !jl_is_array_type(jt)) {
jl_datatype_t *jst = (jl_datatype_t*)jt;
jl_datatype_t *jst = (jl_datatype_t*)jt;
if (jl_is_structtype(jt) && !(jst->layout && jl_is_layout_opaque(jst->layout))) {
if (jst->struct_decl == NULL) {
size_t i, ntypes = jl_svec_len(jst->types);
if (ntypes == 0 || (jst->layout && jl_datatype_nbits(jst) == 0))
Expand Down Expand Up @@ -1069,7 +1056,7 @@ static void emit_typecheck(jl_codectx_t &ctx, const jl_cgval_t &x, jl_value_t *t

static void emit_leafcheck(jl_codectx_t &ctx, Value *typ, const std::string &msg)
{
assert(typ->getType() == T_pjlvalue);
assert(typ->getType() == T_prjlvalue);
emit_typecheck(ctx, mark_julia_type(ctx, typ, true, jl_any_type, false), (jl_value_t*)jl_datatype_type, msg);
Value *isleaf;
isleaf = ctx.builder.CreateConstInBoundsGEP1_32(T_int8, emit_bitcast(ctx, decay_derived(typ), T_pint8), offsetof(jl_datatype_t, isleaftype));
Expand Down
25 changes: 16 additions & 9 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3335,8 +3335,9 @@ static jl_cgval_t emit_sparam(jl_codectx_t &ctx, size_t i)
}
}
assert(ctx.spvals_ptr != NULL);
Value *bp = ctx.builder.CreateConstInBoundsGEP1_32(T_prjlvalue,
emit_bitcast(ctx, decay_derived(ctx.spvals_ptr), T_pprjlvalue),
Value *bp = ctx.builder.CreateConstInBoundsGEP1_32(
T_prjlvalue,
ctx.spvals_ptr,
i + sizeof(jl_svec_t) / sizeof(jl_value_t*));
return mark_julia_type(ctx, tbaa_decorate(tbaa_const, ctx.builder.CreateLoad(bp)), true, jl_any_type, false);
}
Expand Down Expand Up @@ -4923,9 +4924,14 @@ static jl_returninfo_t get_specsig_function(Module *M, const std::string &name,
}

static DISubroutineType *
get_specsig_di(jl_value_t *sig, DIFile *topfile, DIBuilder &dbuilder)
get_specsig_di(jl_value_t *rt, jl_value_t *sig, DIFile *topfile, DIBuilder &dbuilder)
{
std::vector<Metadata*> ditypes(0);
Type *ty = julia_type_to_llvm(rt);
if (type_is_ghost(ty))
ditypes.push_back(nullptr);
else
ditypes.push_back(julia_type_to_di(rt, &dbuilder, false));
for (size_t i = 0; i < jl_nparams(sig); i++) {
jl_value_t *jt = jl_tparam(sig, i);
Type *ty = julia_type_to_llvm(jt);
Expand Down Expand Up @@ -5188,7 +5194,7 @@ static std::unique_ptr<Module> emit_function(
subrty = jl_di_func_sig;
}
else {
subrty = get_specsig_di(lam->specTypes, topfile, dbuilder);
subrty = get_specsig_di(lam->rettype, lam->specTypes, topfile, dbuilder);
}
SP = dbuilder.createFunction(CU,
dbgFuncName, // Name
Expand Down Expand Up @@ -5264,7 +5270,8 @@ static std::unique_ptr<Module> emit_function(
if (!specsig) {
Function::arg_iterator AI = f->arg_begin();
if (needsparams) {
ctx.spvals_ptr = &*AI++;
ctx.spvals_ptr = &*AI;
++AI;
}
fArg = &*AI++;
argArray = &*AI++;
Expand Down Expand Up @@ -6191,8 +6198,8 @@ static void init_julia_llvm_env(Module *m)
jl_init_jit(T_pjlvalue);

std::vector<Type*> ftargs(0);
ftargs.push_back(T_prjlvalue); // linfo->sparam_vals
ftargs.push_back(T_prjlvalue); // function
ftargs.push_back(T_pprjlvalue); // linfo->sparam_vals
ftargs.push_back(T_prjlvalue); // function
ftargs.push_back(T_pprjlvalue); // args[]
ftargs.push_back(T_int32); // nargs
jl_func_sig_sparams = FunctionType::get(T_prjlvalue, ftargs, false);
Expand Down Expand Up @@ -6624,9 +6631,9 @@ static void init_julia_llvm_env(Module *m)
std::vector<Type *> applytype_args(0);
applytype_args.push_back(T_prjlvalue);
applytype_args.push_back(T_prjlvalue);
applytype_args.push_back(PointerType::get(T_prjlvalue, AddressSpace::Derived));
applytype_args.push_back(T_pprjlvalue);
jlapplytype_func =
Function::Create(FunctionType::get(T_pjlvalue, applytype_args, false),
Function::Create(FunctionType::get(T_prjlvalue, applytype_args, false),
Function::ExternalLinkage,
"jl_instantiate_type_in_env", m);
add_named_global(jlapplytype_func, &jl_instantiate_type_in_env);
Expand Down
2 changes: 1 addition & 1 deletion src/julia_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,8 +966,8 @@ STATIC_INLINE void *jl_get_frame_addr(void)
}

JL_DLLEXPORT jl_array_t *jl_array_cconvert_cstring(jl_array_t *a);
JL_DLLEXPORT void jl_depwarn_partial_indexing(size_t n);
void jl_depwarn(const char *msg, jl_value_t *sym);
void jl_depwarn_partial_indexing(size_t n);

int isabspath(const char *in);

Expand Down
28 changes: 19 additions & 9 deletions src/precompile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,23 @@ void jl_write_compiler_output(void)
JL_GC_POP();
}

static int tupletype_any_bottom(jl_value_t *sig)
static int any_bottom_field(jl_value_t *typ)
{
sig = jl_unwrap_unionall(sig);
assert(jl_is_tuple_type(sig));
jl_svec_t *types = ((jl_tupletype_t*)sig)->types;
size_t i, l = jl_svec_len(types);
if (typ == jl_bottom_type)
return 1;
typ = jl_unwrap_unionall(typ);
if (jl_is_vararg_type(typ))
typ = jl_unwrap_vararg(typ);
if (!jl_is_datatype(typ))
return 0;
jl_svec_t *fields = ((jl_datatype_t*)typ)->types;
size_t i, l = jl_svec_len(fields);
if (l != ((jl_datatype_t*)typ)->ninitialized)
if (((jl_datatype_t*)typ)->name != jl_tuple_typename)
return 0;
for (i = 0; i < l; i++) {
if (jl_svecref(types, i) == jl_bottom_type)
jl_value_t *ft = jl_svecref(fields, i);
if (any_bottom_field(ft))
return 1;
}
return 0;
Expand Down Expand Up @@ -130,8 +139,7 @@ static void _compile_all_tvar_union(jl_value_t *methsig)
JL_CATCH {
goto getnext; // sigh, we found an invalid type signature. should we warn the user?
}
assert(jl_is_tuple_type(sig));
if (sig == jl_bottom_type || tupletype_any_bottom(sig))
if (any_bottom_field(sig))
goto getnext; // signature wouldn't be callable / is invalid -- skip it
if (jl_is_leaf_type(sig)) {
if (jl_compile_hint((jl_tupletype_t*)sig))
Expand Down Expand Up @@ -181,9 +189,11 @@ static void _compile_all_union(jl_value_t *sig)
++count_unions;
else if (ty == jl_bottom_type)
return; // why does this method exist?
else if (!jl_is_leaf_type(ty) && !jl_has_free_typevars(ty))
return; // no amount of union splitting will make this a leaftype signature
}

if (count_unions == 0) {
if (count_unions == 0 || count_unions >= 6) {
_compile_all_tvar_union(sig);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/rtutils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ void jl_depwarn(const char *msg, jl_value_t *sym)
JL_GC_POP();
}

void jl_depwarn_partial_indexing(size_t n)
JL_DLLEXPORT void jl_depwarn_partial_indexing(size_t n)
{
static jl_value_t *depwarn_func = NULL;
if (!depwarn_func && jl_base_module) {
Expand Down