-
Notifications
You must be signed in to change notification settings - Fork 751
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
Update LLVM examples for LLVM 12 and libffi #1050
Conversation
I find that splitting everything up in multiple directories make things more complicated. @Neiko2002 Did it for LLVM vs Clang, and @yukoba did it for Polly, which is not too confusing because those correspond to different LLVM modules, but I'm not following what the benefit is for the changes proposed here. Isn't there a way to make each sample stand alone? They are not even 100 lines long. Why do they need to be related to each other? If you're trying to create a structure for some high-level API, that code could be better placed as part of another project like LLVM4J (whose repository you could move under this organization if you wished BTW).
The rest sounds good. 👍 FYI, running samples would only work for builds that do not use a cross compiler, in other words probably only linux-x86_64, macosx-x86_64, and windows-x86_64. |
Thank you for the feedback, I figured a module for each sample would be easier to follow, but if you prefer to keep it as is, then I'm good with that. I will also merge the Bitcode samples into a single file.
Yeah that sounds about right. I'll add a ci build for those environments. |
commit 4c2cf5b9e6b896b5fbf6898ff2552ba6de591fe8 Author: Mats Larsen <me@supergrecko.com> Date: Tue May 25 10:44:02 2021 +0200 Undo separating samples into their own modules
Went ahead and implemented running the various samples with Maven profiles which will conditionally set the exec.mainClass Edit: having some issues with macos and the libffi sample: https://github.com/supergrecko/javacpp-presets/runs/2663909894 |
Ah, yes, we need to hack the rpath with Autoconf on Mac. Try with this patch: --- a/libffi/cppbuild.sh
+++ b/libffi/cppbuild.sh
@@ -84,6 +84,7 @@ case $PLATFORM in
make install-strip
;;
macosx-*)
+ sedinplace 's/\\\$rpath/@rpath/g' configure
CC="clang" ./configure --prefix="$INSTALL_PATH" --disable-multi-os-directory
make -j $MAKEJ
make install-strip |
The samples run off of the libffi packages which are on Sonatype OSS, shouldn't that patch be merged upstream first (or is it not supposed to be)? |
Sure, I've applied the patch in commit 82b9be7 and updated snapshots are up. |
libffi seems to be working - getting a segfault in the sample on macos, will investigate. |
The ThreadSafeModule is moved into the LLJIT which takes care of it from there. We should not need to delete it ourselves.
This should be fixed now. Let me know if there's anything else. |
As mentioned in #833 I have updated the LLVM samples.
Changes in this patch:
I suppose we should also change the sample code shown in
llvm/README.md
.I also thought about adding the samples to CI so we're sure the samples still run when we update code.
Let me know what you think.