Add API for the ORCv2 execution engine

[leifhelm]

Description

This PR aims to add the ability to safely interact with the ORCv2 execution engine. The new orc2 module houses all the new structs and functions. The orc2 feature is automatically enabled when the llvm version is above 11.0. I wrote the lljit example to demonstrate the new execution engine.

Related Issue

#179

How This Has Been Tested

LLVM version: 13.0.0 I tested the new module with the new lljit example. To verify that the memory management is working out I used valgrind. I wrote some doc tests.

Checklist

• [x] I have read the Contributing Guide

[leifhelm]

I changed the underlying implementation of LLVMStringOrRaw to be more generic. Furthermore I added lifetime specifiers to methods that return LLVMStringOrRaw and any wrapper types (DataLayout, TargetTriple). I hope this is fine with you (see b6c5d43). Tests for LLVM version 11 and 12 should be working.

[tpisto]

Hi! Great work - is this compatible with LLVM 14?

[leifhelm]

Inkwell does currently not support LLVM 14 see #317. This PR is kind of stale, as I don't know how to best test some of the features. Especially test that break the current implementation are needed. Furthermore I don't feel motivated currently to invest huge amounts time to move this forward. There are still functions from LLVM 13 that are not wrapped. And I have not looked into multi-threading at all.

[pablosichert]

This PR is indeed some nice work, @leifhelm! I've been working on wrapping the LLVM ORC API before I stumbled on this, and thought I'd rather invest my time to contribute here. Specifically, I have a wrapper for LLVMOrcCreateCustomCAPIDefinitionGenerator to be able to write a custom symbol resolver that I'd like to add.

Especially test that break the current implementation are needed.

Could you elaborate what you're looking for? Does this PR introduce backwards-incompatible changes that are not caught by the compiler with the new lifetime parameters?

Considering that inkwell is currently distributed by linking to the master branch of this repo, I don't think inkwell provides any guarantees about compatibility, or that users would expect any versioning guarantees?

There are still functions from LLVM 13 that are not wrapped. And I have not looked into multi-threading at all.

Providing an exhaustive implementation for all feature of the C API doesn't seem necessary to make this PR very useful nonetheless. Features that build on top of this can be contributed in more PRs in the future. Also since none of the structs are Send + Sync, not implementing multi-threading now won't break user code in the future.

[leifhelm]

The current implementation of orc2 is broken. I have an unpushed testcases that fails, regarding the materialization unit. But I was unable to write an example for that feature.

The changes should not break current functionality of inkwell.

@pablosichert are you able to share how you use this function?

[pablosichert]

@leifhelm sure, you can find my implementation that I added on top of your branch here: https://github.com/pablosichert/inkwell/commits/orc2.

The commit specifically to implement JitDylib::add_generator/CustomDefinitionGenerator: https://github.com/pablosichert/inkwell/commit/e335e56c4f293a6e0ef22d062bce8483cf3ec6d0.

Feel free to pick from that and apply to your branch as you see fit.

I noticed that you use materializer: Box<(dyn Materializer + 'jit)> in MaterializationUnit::create. I think we should be able to use a generic parameter instead, similar to how I did in add_generator.

Generally, I think we should be able to use a little less transmute calls, and use the inner fields that are available from within the crate, or add specific pub(crate) methods.

~Another small nitpick: According to the Rust naming conventions:~

~> In UpperCamelCase, acronyms count as one word: use Uuid rather than UUID~

~Meaning that I'd suggest to rename LLJIT -> Lljit, JITDylib -> JitDylib, RTDyldObjectLinkingLayer -> RtDyldObjectLinkingLayer, IRTransformLayer -> IrTransformLayer and so on.~

Edit: inkwell seems to largely stick to the original C names, so this PR seems consistent with that.

I'd be happy to help move progress forward on this PR. If you need a full code review or help with debugging and fixing test cases, let me know.

[leifhelm]

@pablosichert I had some changes regarding CAPIDefinitionGenerator lying around on my harddrive. I incorporated your changes. Great idea of using a trait to generate multiple extern functions. This is probably the way to go forward.

I added just one simple test. Maybe you could add one that is more real world focused.

[leifhelm]

@pablosichert do you have a better solution than Wrapper that I introduced in leifhelm@8643148? Without it I could call drop right after add_generator

[pablosichert]

@leifhelm hah, yes good catch. I had a segfault locally that boiled down to the generator being dropped when the JIT was still alive. I left a suggestion on your commit

[codecnotsupported]

Any update? Branch fails to build for me. Using LLVM 14.

[gavrilikhin-d]

The work done here must be partially added to inkwell right now.

There are always problems with merging big pull requests (4k lines are a lot). This feature should be split into many small changes, frequently added to master. Otherwise, it will never be implemented.

[leifhelm]

Never it is

[pablosichert]

@gavrilikhin-d your point about incubating smaller incremental changes makes sense in isolation.

However, I think it would be great if you could consider the time constraints and motivation of individuals contributing to open source in their free time before making any demands to the structure or speed of their work.

In case there was some notion that has been lost in translation, I'd like to let you know that your comment has a bit of a rude tone.

[gavrilikhin-d]

I'm sorry for being rude, that was not my intention. English is second language for me.

What I wanted to say is: author of inkwell should not ignore work already done by this person even though it's not complete.

[leifhelm]

Well I think he should not merge it, because there are still some major flaws rooted deep within this API binding. One could say we will fix that later. However I think that the problems that will arise are not easily solvable and do not align with the approach of this PR. So a major rewrite of the orc module would be necessary.

I made this PR so big because I do not understand how LLVM ORC work. So I tried to add features one after the other. This resulted in me rewriting several core structs several times (adding/removing lifetimes, references, ...). This approach is valuable if you want to learn how this ORC thing works, but it is more of a hack.

In the end I concluded that I don't know enough about ORC that I cannot design a proper binding, because I do not understand what lifetimes are here because my implementation needs them and what lifetimes are here because they are intrinsic to the domain.