Add dependency on LLD, wrap it into Python #419
Conversation
|
On my machine the |
|
Not 100% certain here, but a quick scan of the recipes indicates that the difference is likely due to conda-forge building LLVM with RTTI enabled, and our LLVM recipe building with RTTI disabled. I don't know the history of why we disabled RTTI, except perhaps we didn't need it and we thought it would make the final library smaller. |
|
@seibert you are right, I just independently discovered that. LLD now builds. Now I am figuring out how to wrap it, |
|
Now it fails during tests in I think that's probably because I don't link the |
|
Still fails with the same error. |
|
That missing symbols is in the |
|
Weird. When I compile it as follows: and check the But if I build it using So that's probably something with using different compilers somehow? |
|
I didn't realize that I had to clean my working directory. So I did |
|
The last commit fixes it. The undefined symbol was in the The |
|
I think it works: And the LLD program gets loaded and help displayed. The size of the So this is a good start. Now we just need to expose the general arguments (list of strings) of |
certik
changed the title
|
@seibert it works. Here is an example how to use from llvmlite.binding import lld_main
lld_main(["ld.lld", "-o", "a.out", "print_01.o",
"/usr/lib/x86_64-linux-musl/crt1.o", "/usr/lib/x86_64-linux-musl/libc.a"])and the executable works: This example uses Can you please help me get the PR in a state that can be merged? I don't know what design you want to have in Regarding the size of the So I think that didn't add any significant increase in size and given that |
|
This is great! I've tagged @stuartarchibald to take a look at this next week, as the rest of the Numba developers are in the US and will be on holiday. |
|
I installed the |
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
=========================================
Coverage ? 92.27%
=========================================
Files ? 33
Lines ? 5194
Branches ? 363
=========================================
Hits ? 4793
Misses ? 323
Partials ? 78 |
|
I've added tests on x86-64, the test uses lld to build an executable (without any dependencies, not even libc), then executes it and tests that it returns an exit code of 42. It works on all the Linux builds on Travis (in all Python versions). I think that is a very robust test that things actually work. After |
|
@seibert, @stuartarchibald can you please help me get this merged? Let's make a plan what needs to be done and how. This PR provides an extremely useful functionality, so that |
|
Sorry this has been delayed. You caught us during US Thanksgiving and preparations for the Numba release. Once that settles down in the next couple days, we can work on reviewing this. (We have a shorter dev cycle next, so this should be able to make it out in our mid-December release.) |
|
Hi @seibert no problem. I just want to stress that it's not about sitting for 1h and merge. Likely you might want to do things a bit differently than I have done in this PR, and so it's about figuring a plan how to get this in, and then we have to execute the plan. In other words, it requires iterations between me and your team. So the sooner we begin these iterations the better. |
|
Thanks for the idea/patches. This is something that's been a struggle for e.g. the AMD GPU target compilation chain before, AFAIAA there is no programmatic API for
I think:
Once 1. and 2. are resolved, with whatever conda recipe is decided upon, either a |
|
Thanks @stuartarchibald for looking at this PR.
At the end of the day, |
|
@certik thanks for the responses.
I'm convinced of the merits of undertaking this, especially with view that at some point
Thoughts? |
|
@stuartarchibald thanks. I think we agree on things and as long as you are convinced that this is worth doing, I actually don't really care one way or another, as long as this is done and part of llvmlite. So I'll work on 1., I found documentation here: https://conda.io/docs/user-guide/tasks/build-packages/define-metadata.html#source-from-multiple-sources and send a separate PR. |
|
@certik no problem. I think that if you would find such a tool useful and would use it then it is worth doing. I would anticipate that other users would also find such a tool useful too. I can also see potential use for it in circumstances similar to those for the AMD GPU bindings. Thanks for volunteering to try 1. if you get stuck then please shout. This may help: https://github.com/numba/roctools/blob/master/conda-recipes/llvmdev_amdgcn/meta.yaml |
|
@stuartarchibald 1. is implemented in #428. |
|
I rebased on top of the latest master. Indeed, things now work out of the box for Python 3. The Python 2.7 tests are failing, due to the missing @stuartarchibald do you see anything wrong with this? |
|
I figured it out --- I had to call |
|
The linux tests now pass, the Windows test fails because it can't find the LLD libraries (https://ci.appveyor.com/project/seibert/llvmlite/builds/21913057#L1288). The Mac tests fail for the same reason. The PyPy test fails due to the missing |
|
I created a TODO list in the PR description. @stuartarchibald let me know if you want to implement anything else, or if you don't want some of the items implemented before merging. |
|
Thanks for the fixups... RE the TODO list, first, thanks for writing this:
We'll need to take a look at the |
|
Many thanks for the PR and your efforts so far @certik! I'm spending some time going through the llvmlite PR backlog - this PR seems to be one that's been stuck for a while. I see that in #311 (comment) @certik mentioned a lack of time to work on this PR. I think for this PR to move forward, the following is required:
With the above said, would anyone with an interest in this functionality like to volunteer to take over this PR to bring it through to completion? Anyone who is interested, please:
|
|
@gmarkall I decided to take a stab at it. https://github.com/Hassium-Software/llvmlite-lld I have the commits from the PR working in modern llvmlite. I also fixed I will continue to work on this and see what I can do. I was just hoping for feedback on the changes I made to that cpp file. I have very little knowledge of cpp or the internal workings of llvm. edit:
|
|
@spidertyler2005 Many thanks for taking a look at this!
This sounds great! Could you please open a new PR from your fork / branch so we can discuss your work there and reference it more directly?
You could have multiple branches in your fork though - for example, I have several current branches in my llvmlite fork (orcjit, orcjit-llvm15, etc.): https://github.com/gmarkall/llvmlite/branches |
Adds LLD as a dependency, and wraps it into Python. This allows llvmlite to compile to executables, without any external dependencies such as
ldorgcc.TODO:
Fixes #311.