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

simplify interaction between new JITs and codegen #15556

Merged
merged 1 commit into from
Mar 23, 2016

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Mar 18, 2016

this treats the new JITs (MCJIT, ORCJIT) much like the old JIT, but using Module as the atomic unit instead of Function

reviewer note: much of the code motion is moving the logic to interact with the JIT into jitlayers.cpp. this is not yet separable from codegen.cpp, but I expect they should be able to divide into separate compilation units in the future.

TODO:

  • reimplement memory optimization of strings in the JIT
  • reimplement code-coverage and memory-usage counters

fix #15533

delete specf;
specf = temp;
}
if (specf) {
Copy link
Member

Choose a reason for hiding this comment

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

if (f)?

@Keno
Copy link
Member

Keno commented Mar 18, 2016

I'm generally fine with this, even though it goes in the opposite direction from where I was taking it. I do worry about the performance implications of creating this many modules, but since we're at least not doing per-module codegen, it might not be that bad. As I mentioned, I would like to see some of this code move into ORCJIT proper eventually, but I'm fine with leaving it like this if it makes compatibility with the old JITs easier.

@vtjnash vtjnash force-pushed the jn/moduledecoalescing branch 2 times, most recently from 2a65a90 to dcc3035 Compare March 21, 2016 21:14
@vtjnash
Copy link
Member Author

vtjnash commented Mar 22, 2016

bump. this is ready to merge.

this treats the new JITs (MCJIT, ORCJIT) much like the old JIT,
but using Module as the atomic unit instead of Function

this gets codegen closer to being separable (cachable) at the
module level, with the remerging happening in the JIT after object
file emission

fix #15533
// this assumes that the targets of the two modules are the same
// including the DataLayout and ModuleFlags (for example)
// and that there is no module-level assembly
static void jl_merge_module(Module *dest, std::unique_ptr<Module> src)
Copy link
Member Author

Choose a reason for hiding this comment

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

breadcumb: in the future, we may want to just call Linker::linkModules (introduced in llvm-3.8 for LTO), but for now, our actual usage is much easier than that because we don't need full generality (or barely any at all), so we can just do the simple thing here

@vtjnash vtjnash merged commit c29bff1 into master Mar 23, 2016
@vtjnash vtjnash deleted the jn/moduledecoalescing branch March 23, 2016 05:32
@josefsachsconning
Copy link
Contributor

I think this broke the JULIA_THREADS=1 build, since the number of arguments to jl_get_specialization1 was changed. Here is the bottom of my make output.

    CC src/threading.o
/home/sachs/src/julia-multithreading/src/threading.c: In function ‘jl_threading_run’:
/home/sachs/src/julia-multithreading/src/threading.c:394:5: error: too many arguments to function ‘jl_get_specialization1’
     jl_lambda_info_t *li = jl_get_specialization1(argtypes, NULL);
     ^
In file included from /home/sachs/src/julia-multithreading/src/threading.c:23:0:
/home/sachs/src/julia-multithreading/src/julia_internal.h:266:19: note: declared here
 jl_lambda_info_t *jl_get_specialization1(jl_tupletype_t *types);
                   ^
make[1]: *** [threading.o] Error 1
make: *** [julia-src-release] Error 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM ERROR when loading precompiled package
4 participants