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

Add ORC-MCJIT replacement binding #245

Closed
wants to merge 1 commit into from
Closed

Conversation

sklam
Copy link
Member

@sklam sklam commented Feb 22, 2017

Adds binding to OrcMCJITReplacement.

Progress:

  • write binding
  • write tests
  • write docs

Note:

The Orc-MCJIT is not a direct replacement for MCJIT for all cases. When testing in numba, the lazy compilation in Orc-MCJIT seems to be non-threadsafe and it is failing any parallel execution of the same function. (See related LLVM issue https://bugs.llvm.org//show_bug.cgi?id=5184). In Orc-MCJIT, it seems the lazy compilation cannot be turned off.

@codecov-io
Copy link

Codecov Report

Merging #245 into master will increase coverage by 0.03%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
+ Coverage   91.87%   91.91%   +0.03%     
==========================================
  Files          33       33              
  Lines        4728     4774      +46     
  Branches      330      332       +2     
==========================================
+ Hits         4344     4388      +44     
- Misses        308      309       +1     
- Partials       76       77       +1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8047388...9bcf87c. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 93.527% when pulling 9bcf87c on sklam:feature/orcjit into 8047388 on numba:master.

@seibert
Copy link
Contributor

seibert commented Feb 22, 2017

Does this mean we'll need to write our own MCJIT replacement with the ORC API that is actually thread-safe?

@sklam
Copy link
Member Author

sklam commented Feb 22, 2017

Yes. I think there are two potential approaches:

  1. Make a eager-version. The problem comes from ORC-MCJIT being lazy. It puts a stub function that defers the compilation to 1st call. The stub is not threadsafe though. Making it eager should remove the problem.

  2. Make a threadsafe compile-on-demand layer ourselves.

@seibert
Copy link
Contributor

seibert commented Feb 22, 2017

Do we benefit from the lazy compilation, since we are already deferring our call to LLVM to the first function call anyway? If no benefit, then option 1 seems to make the most sense.

@seibert
Copy link
Contributor

seibert commented Feb 22, 2017

And I assume that option 1 means Numba (or llvmlite?) will still need to enforce locking at the Python level to prevent concurrent compilation.

@sklam
Copy link
Member Author

sklam commented Feb 22, 2017

The current numba setup won't benefit from lazy compilation. I prefer option 1 and hope that it actually fixes the issue. Option 1 will be exactly like what MCJIT is doing---eager compilation all the time. Numba already has the concurrent compilation lock.

def test_object_cache_getbuffer(self): # overrides
# The ORC JIT needs a different test because it will fail at the
# second phase if loading function "sum" in a module without one.
# Instead, we will supply another "sum" implementation and make sure it
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow. Is that a documented difference in LLVM? A bug?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the behavior in the MCJIT-version documented?

It seems OrcJIT is stricter here. Due to its laziness, Orc first tries to find a IR module that has the symbol. Once found, it checks if it is already compiled and do its thing. If not found, it stops and return NULL.

@alendit
Copy link
Contributor

alendit commented Jun 14, 2018

What's the status of this? AFAIK ORC supports eager compilation via the basic IRCompileLayer. Maybe the OrcMCJITReplacement isn't the best way forward? I could add ORC as a separate ExecutionEngine with a superset of MCJIT's python API. This way the numba wouldn't be directly affected and the new EE could be thoroughly tested before any switch is made.

@lhames
Copy link

lhames commented Feb 1, 2021

@abebeos OrcMCJITReplacement was intended as an OrcV1 drop-in replacement for MCJIT. has been deprecated for a while and was removed with the rest of OrcV1 in LLVM 12. I think this PR can be closed.

@esc
Copy link
Member

esc commented Feb 4, 2021

sure

@esc esc closed this Feb 4, 2021
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.

8 participants