Permalink
Browse files

Fix MCJIT issues

All of our problem were related to naming. Most notably:

The jlcall wrapper:
Since they are in the same module as the original function, LLVM will prevent name collision as
appropriate, by doing it's own uniquing, e.g. julia_foo1, becomes julia_foo12, however, if we're
not careful, 11 functions later things go BOOM

Not resetting global unique:
When I originally wrote this code, precompilation did not exists, and I failed to take into account
the fact that the symbol from the generated functions will be visible in the new process (llvm dlsyms
all loaded modules including the system image). Thus if you have just the right combination of
function and globalUnique, you'll run into the system image.

Both of these issues cause the wrong function to be executed at run time, with the corresponding
chaos. Note that this was never an issue during system image building, because we emit everything
into the shadow module where LLVM will take care of it.

I also added a unqiue to the julia_gv global variables. This isn't strictly nessecary from a
correctness point of view, but it helps when looking at IR dumps from several modules.

Also, those linalg tests take forever...
  • Loading branch information...
1 parent 596d5c4 commit 5c1b5ad24e745ec785c2bd32d036977fbd79b5e9 @Keno Keno committed Feb 8, 2014
Showing with 23 additions and 5 deletions.
  1. +11 −1 src/cgutils.cpp
  2. +7 −4 src/codegen.cpp
  3. +2 −0 src/disasm.cpp
  4. +3 −0 src/dump.c
View
@@ -227,6 +227,13 @@ static void jl_gen_llvm_gv_array()
GlobalVariable::ExternalLinkage,
ConstantArray::get(atype, ArrayRef<Constant*>(jl_sysimg_gvars)),
"jl_sysimg_gvars");
+ new GlobalVariable(
+ *jl_Module,
+ T_size,
+ true,
+ GlobalVariable::ExternalLinkage,
+ ConstantInt::get(T_size,globalUnique+1),
+ "jl_globalUnique");
}
static int32_t jl_assign_functionID(Function *functionObject)
@@ -246,10 +253,13 @@ static Value *julia_gv(const char *cname, void *addr)
it = jl_value_to_llvm.find(addr);
if (it != jl_value_to_llvm.end())
return builder.CreateLoad(it->second.gv);
+
+ std::stringstream gvname;
+ gvname << cname << globalUnique++;
// no existing GlobalVariable, create one and store it
GlobalValue *gv = new GlobalVariable(*jl_Module, jl_pvalue_llvmt,
false, imaging_mode ? GlobalVariable::InternalLinkage : GlobalVariable::ExternalLinkage,
- ConstantPointerNull::get((PointerType*)jl_pvalue_llvmt), cname);
+ ConstantPointerNull::get((PointerType*)jl_pvalue_llvmt), gvname.str());
// make the pointer valid for this session
#ifdef USE_MCJIT
View
@@ -358,6 +358,8 @@ static Type *NoopType;
// --- utilities ---
+extern "C" int globalUnique = 0;
@jiahao

jiahao Feb 8, 2014

Member

on julia.mit.edu, this causes the build to emit a warning:

codegen.cpp:361:16: warning: ‘globalUnique’ initialized and declared ‘extern’ [enabled by default]
@Keno

Keno Feb 8, 2014

Owner

Should be fixed now.

@jiahao

jiahao Feb 8, 2014

Member

Confirmed.

+
#include "cgutils.cpp"
@@ -2066,7 +2068,7 @@ static Value *emit_checked_var(Value *bp, jl_sym_t *name, jl_codectx_t *ctx)
BasicBlock *ifok = BasicBlock::Create(getGlobalContext(), "ok");
builder.CreateCondBr(ok, ifok, err);
builder.SetInsertPoint(err);
- builder.CreateCall(jlundefvarerror_func, literal_pointer_val((jl_value_t*)name));
+ builder.CreateCall(prepare_call(jlundefvarerror_func), literal_pointer_val((jl_value_t*)name));
builder.CreateBr(ifok);
ctx->f->getBasicBlockList().push_back(ifok);
builder.SetInsertPoint(ifok);
@@ -2796,8 +2798,11 @@ static void finalize_gc_frame(jl_codectx_t *ctx)
// 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)
{
+ std::stringstream funcName;
+ funcName << "jlcall_" << f->getName().str();
+
Function *w = Function::Create(jl_func_sig, Function::ExternalLinkage,
- f->getName(), f->getParent());
+ funcName.str(), f->getParent());
Function::arg_iterator AI = w->arg_begin();
AI++; //const Argument &fArg = *AI++;
Value *argArray = AI++;
@@ -2865,8 +2870,6 @@ static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Funct
return w;
}
-static int globalUnique = 0;
-
// cstyle = compile with c-callable signature, not jlcall
static Function *emit_function(jl_lambda_info_t *lam, bool cstyle)
{
View
@@ -140,7 +140,9 @@ void jl_dump_function_asm(void* Fptr, size_t Fsize,
}
Streamer.reset(TheTarget->createAsmStreamer(Ctx, stream, /*asmverbose*/true,
+#ifndef LLVM35
/*useLoc*/ true,
+#endif
/*useCFI*/ true,
/*useDwarfDirectory*/ true,
IP, CE, MAB, ShowInst));
View
@@ -94,6 +94,8 @@ static jl_value_t *jl_deserialize_value(ios_t *s);
static jl_value_t *jl_deserialize_value_internal(ios_t *s);
static jl_value_t ***sysimg_gvars = NULL;
+extern int globalUnique;
+
static void jl_load_sysimg_so(char *fname)
{
// attempt to load the pre-compiled sysimg at fname
@@ -102,6 +104,7 @@ static void jl_load_sysimg_so(char *fname)
uv_lib_t *sysimg_handle = jl_load_dynamic_library_e(fname, JL_RTLD_DEFAULT);
if (sysimg_handle != 0) {
sysimg_gvars = (jl_value_t***)jl_dlsym(sysimg_handle, "jl_sysimg_gvars");
+ globalUnique = *(size_t*)jl_dlsym(sysimg_handle, "jl_globalUnique");
}
else {
sysimg_gvars = 0;

15 comments on commit 5c1b5ad

Owner

staticfloat replied Feb 9, 2014

This is some good detective work, Keno. Nice job.

Member

ihnorton replied Feb 9, 2014

👍 going to give ARM a try again :)

Member

johnmyleswhite replied Feb 9, 2014

Works reliably for me on OS X. Well done!

Member

kmsquire replied Feb 9, 2014

+1

Owner

Keno replied Feb 9, 2014

Thanks, I'm not quite happy with it yet. For some reason as soon as type inference jumps in, it gets unbearably slow. It might just be that MCJIT is fundamentally slow and type inferences exercises it, but there might be something else going on.

Owner

Keno replied Feb 9, 2014

Alright, now I'm happy :). 57ec5a7

Does this mean that we could turn MCJIT on even for LLVM 3.3 – or that we could switch to LLVM 3.4 + MCJIT?

Owner

Keno replied Feb 9, 2014

Definitely not 3.3 (I have several patches that I submitted to MCJIT that made it into 3.4). 3.4 maybe but I haven't really tested. I'm currently testing with 3.5.

Member

johnmyleswhite replied Feb 9, 2014

@loladiro: In the off case you don't know this already, I have trouble downgrading from LLVM 3.4. When I try modifying deps/Versions.make, the build dies right at the end with a ld: library not found for -lLLVMLTO error.

Owner

Keno replied Feb 9, 2014

You need to delete usr. Some llvm files stick around. We should probably fix whatever clean rule that is, but for now that works just fine.

Is there anything fundamental stopping us from using LLVM 3.4 with MCJIT? (I know we can't use the old JIT with LLVM 3.4 since they broke stack traces.) That seems like the quickest route to having something that could potentially be properly debugged, no? Obviously LLVM 3.5 is the way forward, but it's probably not going to be released until summer, right?

Member

johnmyleswhite replied Feb 9, 2014

Good to know about deleting usr. Thanks!

Owner

Keno replied Feb 9, 2014

I don't know I haven't tried. Note though that there may be a bit of work to be done upstream in LLVM before we get proper debugging with MCJIT (3.4/linux/gdb might work, anything else probably wouldn't).

Ok, I'm just trying to angle towards getting a basic debugger as quickly as possible. If it won't be possible with LLVM 3.4, that's ok, but it's good to know what the dependencies are. If you do get something working with a pre-release LLVM 3.5, we could always start using a development snapshot.

+1 Debugging would be a huge upgrade for Julia.

Please sign in to comment.