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.

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

switching to MCJIT #3922

Closed
vtjnash opened this issue Aug 2, 2013 · 16 comments · Fixed by #5208
Closed

switching to MCJIT #3922

vtjnash opened this issue Aug 2, 2013 · 16 comments · Fixed by #5208
Labels
upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@vtjnash
Copy link
Member

vtjnash commented Aug 2, 2013

Eventually, we may need/want to switch to MCJIT for various reasons. This patch is all that appears to be needed (on Linux):

diff --git a/src/codegen.cpp b/src/codegen.cpp
index 5204cfd..42864c3 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -3409,6 +3409,7 @@ extern "C" void jl_init_codegen(void)
 #endif
     jl_ExecutionEngine = EngineBuilder(jl_Module)
         .setEngineKind(EngineKind::JIT)
+        .setUseMCJIT(true)
         .setTargetOptions(options)
         .create();
 #endif // LLVM VERSION

Unfortunately, it doesn't seem to run on Apple yet?

$ ./julia 
LLVM ERROR: Target does not support MC emission!
$ uname
Darwin
@ViralBShah
Copy link
Member

Is a debugger one of those important reasons?

@timholy
Copy link
Member

timholy commented Aug 2, 2013

That's rather surprising given Apple's investment in LLVM. You sure about that?

@StefanKarpinski
Copy link
Member

MCJIT is important for debugger support and for generating binaries.

@vtjnash
Copy link
Member Author

vtjnash commented Aug 3, 2013

I didn't say any of the reasons are important.

@StefanKarpinski Those statements aren't entirely accurate. With the help of ihnorton, I was able to dump the JIT code to a file and compile it offline, resulting in a Julia binary, with the improved debugger support that entails. The MCJIT may be able to provide the same information directly to gdb for JIT code, but that's still unclear to me.

The main reason to switch may be that the LLVM group don't appear to be working on the current JIT, but are intending to finish the MCJIT as a simple replacement. I'm a bit confused by the lack of support on Darwin as well. I opened this issue partly as documentation of the current state, and partly in the hope that someone would be able to identify the problem.

@simonster
Copy link
Member

Following this post on the LLVM blog I added InitializeNativeTargetAsmPrinter() to the top of jl_init_codegen(), and now I have a binary that segfaults on startup on OS X, which may or may not be an improvement. That post suggests that MCJIT will segfault if you try to add a new function to a module after it's been compiled, although that doesn't explain why it works for you on Linux.

@vtjnash
Copy link
Member Author

vtjnash commented Aug 4, 2013

Ah, excellent! That post explains a lot about what we will need to fix.

@ArchRobison
Copy link
Contributor

Andrew Kaylor (andrew.kaylor at intel.com) was at the LLVM Developers Meeting last week in San Francisco and later sent me the following information, which seems worth recording here:

"The exception handling support for the old JIT engine is being removed in the LLVM 3.4 release and I expect the old JIT engine itself to be deprecated in LLVM 3.5 and dropped entirely in 3.6.

I spent some time with Keno Fischer (@loladiro) trying to get Julia to work with MCJIT instead, which does have exception handling support now. We ran into a couple of problems related to the way Julia was using the old JIT. With the old JIT, you could put everything into a single llvm::Module instance and continue adding new functions and variables to the module even after you have generated code from previously defined functions. With MCJIT, you can’t modify an llvm::Module after you have generated code from it.

We modified the Julia source to create a new module as needed. This was kind of hacked in and not in the proper place, but we were just trying to get something up and running. Once multiple modules were being created, we ran into a problem that the Julia code was generating calls to functions that were defined in other modules without creating prototypes for them, which caused LLVM’s module verifier to reject the module. A similar problem existed with variables.

We put in some more, even uglier hacks, to get around the external function and variable problem. That got far enough to generate a Julia system image, but it was breaking somewhere in the smoke tests. That’s where we left things at the end of the Dev Meeting. I exchanged a couple of e-mails with Keno since then about a problem that was uncovered by the LLVM address sanitizer tool (I’ve since fixed that issue in LLVM trunk), but I don’t know if he has figured out the problem that was preventing Julia from working with MCJIT."

@Keno
Copy link
Member

Keno commented Nov 12, 2013

To sum up my current state:
I managed to get the system image building using MCJIT, during the tests we ran into the unfortunate case of running a finalizer during compilation (which might itself be compiled), which didn't quite work as MCJIT will jit the entire module, which is a problem if the finalizer happens to be in the same module as the function you were just compiling. To get around that, I started putting every function in its own module but started hitting random segfaults potentially caused by the above mentioned memory problem (llvm-mirror/llvm@559d409).

@timholy
Copy link
Member

timholy commented Nov 13, 2013

Is one of the motivations behind MCJIT inlining? Will having each function in a separate module eliminate LLVM's ability to inline?

@Keno
Copy link
Member

Keno commented Nov 13, 2013

We do our own language level inlining without LLVM in the picture at the moment anyway, so it doesn't affect us. I don't know if there's a cross-module inlining pass, though I imagine there would be and if not it wouldn't be too difficult to write one. Nevertheless, I'd love to figure out how to avoid the finalizer issue, thus putting more functions in a module (which can only be beneficial). I'm thinking, since it's actually pretty rare that this happens, just do the naive thing and create a separate module for the finalizer before compiling, fixing up any references (think replaceAllUsesWith, RAUW as Chris Lattner calls it).

@staticfloat
Copy link
Member

@loladiro I believe putting every function in its own module is supposed to work, at least, that's what I got from this blog post.

@Keno
Copy link
Member

Keno commented Nov 13, 2013

No, it is (I talked extensively with Andy who wrote that blog post last week), there's just a bit of trouble with assumptions we make in the current code base.

@timholy
Copy link
Member

timholy commented Nov 13, 2013

I know currently we do our own inlining, but to me it seems attractive in principle to offload some of that onto a compiler. I imagine they've put quite a lot of effort into questions like "when is it better to inline, and when not?" IIUC, right now our inlining is pretty simple (e.g. until Jameson's work gets merged, any code with a conditional will not be inlined, which means not much gets inlined). So currently it's not an issue, but eventually we might want some of that wisdom that I imagine compiler-makers must have accumulated.

@Keno
Copy link
Member

Keno commented Nov 13, 2013

I agree, I've actually talked to Jeff about enabling the inlining pass before. For now, it doesn't much matter either way though.

@Keno
Copy link
Member

Keno commented Nov 15, 2013

@JeffBezanson Here's the backtrace from earlier. Contrary to my previous statement this is in type inference not gc. Still not sure why it tries to compile the unfinished module.

EDIT: https://gist.github.com/loladiro/7479856

@Keno
Copy link
Member

Keno commented Nov 17, 2013

I have a branch that works, but I already messed up earlier trying to disentangle it from everything else, so I'll wait for my various other pull requests to make it onto master and then create a branch off that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants