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

Wrap LLD into python #898

Open
wants to merge 124 commits into
base: main
Choose a base branch
from
Open

Wrap LLD into python #898

wants to merge 124 commits into from

Conversation

ArachnidAbby
Copy link

@ArachnidAbby ArachnidAbby commented Jan 11, 2023

Based on: Add dependency on LLD, wrap it into python

todo:

  • Bring the old pr up-to-date with the 800 commits made to llvmlite since the pr opened
  • Fix any broken code from the original pr
  • add .binding.lld module with functions for each platform lld supports
  • complete remaining items on the original PR
  • add documentation for any newly added functions

items remaining in the original PR

  • Make all Travis tests pass, test LLD on 64bit linux
  • Write a test for 32 bits linux? Currently only 64 bits are tested. (not required)
  • Get it to compile on macOS
  • Get it to compile on Windows
  • Get the PyPy test working (not required)
  • Implement LLD for macOS and test it
  • Implement LLD for Windows and test it
    (all non-required items are going to be checked off)

I will update you as I complete items.

certik and others added 28 commits January 26, 2019 14:40
This fixes the following link errors:

.../miniconda3/envs/build/conda-bld/lld_1542237425154/_build_env/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: ../../lib/liblldCOFF.a(MapFile.cpp.o):(.data.rel.ro._ZTIN4llvm13format_objectIJmmmEEE[_ZTIN4llvm13format_objectIJmmmEEE]+0x10): undefined reference to `typeinfo for llvm::format_object_base'
For now we just call lld::elf::link.
This is a temporary workaround to get it working. We have to figure out how to
obtain these automatically.
@gmarkall
Copy link
Member

Many thanks for opening this PR!

@ArachnidAbby
Copy link
Author

@esc Hey, looks like i did it. There is still some stuff to cleanup (some temp/test code), but at least it works now.

I can also cleanup the git history for this pr like previously discussed if you all want me to. I just cant/wont squash together any commits that i didnt make, i dont want to cut the other contributor out of the git history.

@ArachnidAbby
Copy link
Author

alright, I'm going to take another look real quick and see if there is anything I should fix. I'm thinking about changing how the lld libraries are stored in the build script. currently they look kind of like: "-llldCommon -llldELF -llldWasm". I think this should just be a list without the -l prefixing the libraries. With the -l included, I end up needing to remove it later and then also split the string. I instead could just stored this as list to begin with and format the elements as needed.

@ArachnidAbby
Copy link
Author

Just want to verify that there is nothing else that I need to do with this for the time being?

@esc esc added this to the v0.43.0-rc1 milestone Dec 18, 2023
@esc
Copy link
Member

esc commented Dec 18, 2023

Just want to verify that there is nothing else that I need to do with this for the time being?

I think it's OK for the time being. I've now added this to the 0.43 milestone -- so it stands a good chance of being reviewed and making it into the release. Pinging @stuartarchibald on this was as I recall there is interest on this one.

@spidertyler2005 thank you for your persistence on this, it's much appreciated!!

Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

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

Looking at the documentation, seems like the underlines are one character too long.

docs/source/user-guide/binding/lld.rst Outdated Show resolved Hide resolved
docs/source/user-guide/binding/lld.rst Outdated Show resolved Hide resolved
docs/source/user-guide/binding/lld.rst Outdated Show resolved Hide resolved
docs/source/user-guide/binding/lld.rst Outdated Show resolved Hide resolved
@ArachnidAbby
Copy link
Author

ArachnidAbby commented Dec 18, 2023

@esc damn, can you rerun the tests? One of them had an http error while doing a conda install.

@esc
Copy link
Member

esc commented Dec 19, 2023

@esc damn, can you rerun the tests? One of them had an http error while doing a conda install.

done

@ArachnidAbby
Copy link
Author

sick, thank you!

@PrimozGodec
Copy link

Thank you for preparing those changes. It is a great addition. What is the state of this PR? Do you think it can be merged soon?

@esc
Copy link
Member

esc commented Jan 10, 2024

@PrimozGodec the state is: pending on review and scheduled for inclusion in the 0.43 release of llvmlite. If you want, feel free to test this and add a review in case you discover any snags or comment with: "works for me" if no snags are to be found.

@ArachnidAbby
Copy link
Author

If you decide to try it out, remember to run conda install -c conda-forge libstdcxx-ng=12 if you are on linux. I couldn't get it working with llvm14 without doing this.

@ArachnidAbby
Copy link
Author

btw, changed my GH name so my old @ isn't formatted anymore.

@esc esc modified the milestones: v0.43.0-rc1, v0.44.0 May 7, 2024
@esc
Copy link
Member

esc commented May 7, 2024

Unfortunately we couldn't find a reviewer for this, so it has been punted to the 0.44 release.

@ArachnidAbby
Copy link
Author

damn, that's a bummer :(

Just tell me if there is anything I can do to make it easier when you do find a reviewer. Good luck with the next release!

@dlee992
Copy link
Contributor

dlee992 commented May 22, 2024

[Not a reviewer] Thanks for this PR! In Numba side, I often have an issue: when using typed.dict or typed.list, currently numba compiles a C-version of them as the underlying implementation. Then when users call a method of these types in jit-ed code, the actual call chain is: user jitted func -> numba type wrapper -> c implementation by ctypes.

So the issue is when calling into the c impl, runtime has to create a call frame for c func, but sometimes the c func is so small, then the creation of call frame becomes the performance bottleneck. A more concrete example can be found: numba/numba#9520 (comment).

I wonder if this PR can help solve the issue fundamentally? ex: compiling and linking the C code as a LLVM IR module, then enable further optimizations, e.g., inlining, vectorization. I am not sure if what I want is another feature, not this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow llvmlite to also link object code
6 participants