bscarlet
commented
8 years ago You're going to have to help me out evaluating this by explaining your design, as I'm not yet particularly familiar with ORC JIT.
Overall, stability and consistency of the llvm-general API is a major goal of its design. The rule I've chosen is to stick to the logical content of the C++ API, with as much of the C++-isms taken out as I could (and some stricter lexical conventions - i.e. pretty much no abbreviations except for very standard ones (and those all-caps, so "JIT" please, not "Jit")). Incremental implementation is fine, but by sticking to the level of abstraction offered by the C++ API provides a guiding path. So I avoid choices that obstruct eventual implementation of any part of the C++ API, and I avoid inventing higher-level abstractions of my own (such are abstractly fine, just not in this package).
Unfortunately, the C API sometimes deviates from that path. In those cases, I think we should stick to exposing the "true" abstract content of the C++ API, even if it means implementing alternates to stuff in the C-API when the C-API is going down its own path. This choice ensures there's always some consistent way to expose whatever new feature we want. If the C-API helps, that's great, but if not, the point of compromise is that we write more glue code, not that we limit what llvm-general can expose.
So I ask you to judge whether this work follows that principle. I took a quick look at how the ORC JIT is exposed in the C++ API and how it relates to the ExecutionEngine, and my first impression was that the main ORC JIT interface doesn't use ExecutionEngine, but goes its own way - supporting your design. Is that right? There's also the OrcMCJITReplacement thing, but that seems to be a separate path to using it. Is the C-API you're using doing well sticking close the C++ API, or is it reinterpreting things in new ways? If I fully understood the C++ API, would your Haskell API seem just a Haskell-style exposure of its concepts, or would I have to understand some choice in the C API for it to make sense?
As to your desire to "kill that abstraction", I'm not sure what you mean. The C++ API has the feature that one can run code from a module using either JIT or an Interpreter, and easily swap between them. It's an important feature.
cocreature
The bindings only cover the part that’s exposed by the C API but it looks like most of the important stuff is in there or at least enough to be useful on its own. One function is missing which requires bindings to the object loading stuff which is orthogonal to this PR. Should be easy to add once we have those.
I deliberately didn’t provide instances for
ExecuteEnginebecause that would expose a lossy API that doesn’t have the benefits that this JIT gives you and I don’t see a lot of value in abstracting over the JIT (frankly I’d rather kill that abstraction completely but that’s a separate issue and out of scope for this PR).