This is an archive of the discontinued LLVM Phabricator instance.

[Orc] At CBindings for LazyRexports
ClosedPublic

Authored by vchuravy on Jun 21 2021, 2:26 PM.

Details

Summary

At C bindings and an example for LLJIT with lazy reexports

Diff Detail

Event Timeline

vchuravy created this revision.Jun 21 2021, 2:26 PM
vchuravy requested review of this revision.Jun 21 2021, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2021, 2:26 PM

The new C API functions should include comments noting that they're relatively unstable, and likely to change in coming releases. As you've seen: these APIs are due for a clean-up.

llvm/include/llvm-c/Orc.h
869

This is actually returning a stubs-manager-builder -- a factory that produces stub-manager objects. This is a hangover from OrcV1 and should be replaced with a simpler stub manager object, we just haven't gotten around to it yet.

Sadly, for the sake of consistency, we probably need to rename the type and API to LLVMOrcIndirectStubsManagerBuilderRef and LLVMOrcCreateLocalIndirectStubsManagerBuilder.

877

This should return an LLVMErrorRef and return the LCTM value (on success) via a result argument:

LLVMErrorRef LLVMOrcCreateLocalLazyCallThroughManager(
    LLVMOrcLazyCallThroughManagerRef *Result,
    const char *TargetTriple, LLVMOrcExecutionSessionRef ES,
    LLVMOrcJITTargetAddress ErrorHandlerAddr) {
    auto LCTM = createLocalLazyCallThroughManager(
      Triple(TargetTriple), *unwrap(ES), ErrorHandlerAddr);
    if (!LCTM)
      return wrap(LCTM.takeError());
    *Result = wrap(LCTM->release());
    return LLVMErrorSuccess;
}

The patch will also need to be clang-formatted.

llvm/include/llvm-c/Orc.h
869

Oh, I missed that the implementation was actually running the builder and returning an instance, not returning the builder. Scratch this comment -- these APIs are named correctly.

vchuravy updated this revision to Diff 355894.Jul 1 2021, 8:20 AM
vchuravy marked 3 inline comments as done.

Address review comments

address review

vchuravy updated this revision to Diff 355899.Jul 1 2021, 8:35 AM

Add comment and clang-format

lhames accepted this revision.Jul 1 2021, 9:20 AM

LGTM. Thanks Valentin!

This revision is now accepted and ready to land.Jul 1 2021, 9:20 AM
This revision was landed with ongoing or failed builds.Jul 1 2021, 12:52 PM
This revision was automatically updated to reflect the committed changes.