Enable LLVM-12 #802
Enable LLVM-12 #802
Conversation
|
@r-barnes thank you for submitting this! Your effort is appreciated. I have labelled this as ready for review. I would like to set some expectations ahead of time however: a) we are currently in a release candidate phase followed by holidays. It is unlikely that anyone will take a closer look before mid/end Q1 2022 b) we have discussed going stright to LLVM 13 in a recent developer meeting. This means, this PR may never actually be looked at, but no-one can say for certain at this stage. Thank you again for your efforts. Perhaps they will be useful sometime soon! |
If you want to repeat this for LLVM 13, that may actually turn our to be really useful. If LLVM 13 is released even. |
I also noticed, that the conda recpie was not updated to LLVM 12. We run our LLVM with a set of custom patches needed for Numba. To that end, we compile LLVM with You can take a look at what is needed, as I have done this for LLVM 11.1.0 here: https://github.com/numba/llvmlite/pull/715/files Hope it helps! |
r-barnes
force-pushed
the
richard/llvm12
branch
from
6437dc4
to
dcf4e5b
Compare
|
@esc: I've updated the PR to include LLVM-13 as well as LLVM-12. I'm looking at the conda stuff now. |
|
@esc: Are you suggesting replacing LLVM-11 with LLVM-12 (or 13) in the conda files or adding a different set of conda tests for LLVM-12 to be run in addition to the existing ones for LLVM-11? |
Correct. Historically, llvmlite has supported only one LLVM version at a given time. My suggestion would be to replace existing LLVM 11 support with LLVM 13 support. A further note (a 'heads up' if you will): one of the goals of the LLVM 13 upgrade is to migrate from MCJIT to ORCJIT https://llvm.org/docs/ORCv2.html -- while this task is not yet concretely specified (I think there is not even a ticket for this) -- it will be part of the upgrade. And from what I can tell, this is unlikely to be a trivial task. Anyway, one thing at a time. |
Thank you!! 🙏🙏 there is some documentation here: https://llvmlite.readthedocs.io/en/latest/admin-guide/install.html#compiling-llvm -- however after giving it a quick skim, it looks very out of date, so please be careful when consulting it and don't hesitate to ask questions. I am also available at: https://gitter.im/numba/numba-dev in case a more real-time communication channels suits you better. |
|
@esc: I'm working in an environment pinned to LLVM-12, so support for that's kind of important to me. So far, there are no differences between what's required to make llvmlite work on LLVM-12 versus LLVM-13. Could we:
|
|
(Current numba test failures are due to an inability to find the |
r-barnes
force-pushed
the
richard/llvm12
branch
from
dcf4e5b
to
1d928eb
Compare
Supporting multiple LLVMs will be a deviation of current policy. We will need to achieve consensus with the other stakeholders of this project, but I think it is a very reasonable ask. Historically, llvmlite was strongly coupled to Numba. However, now that it is more and more becoming a project in it's own right, we may need to think about supporting multiple LLVMs, if that is what folks want. You can of course, always apply simple patches, but Numba requires a patched variant of LLVM and most of the trouble when porting LLVM comes from porting these patches. If you don't have llvmlite running for the sake of Numba, of course those patches are not relevant and you can use whatever llvmlite you can get your hands on. I presume this is your use-case? Having said all of that, I think this is a good thing to start tackling in 2022. The Numba team is currently wrapping up a Numba release candidate so all hands are occupied with this. My suggestion would be, for you to come and join our public developer meeting. The announcement for 2021 is here: https://numba.discourse.group/t/weekly-public-meeting-every-tuesday-for-2021/658 And I am assuming that this will continue in 2022 with the same link and calendar invite. It would be awesome to see you there!! |
From: https://llvmlite.readthedocs.io/en/latest/admin-guide/install.html Does that help? |
|
We will discuss the use-case of llvmlite to support multiple LLVM versions during the developer meeting tomorrow. |
|
@r-barnes we discussed this today: https://github.com/numba/numba/wiki/Minutes_2021_12_07 -- it looks like I will have time to work on ways to support multiple LLVMs in January/Febuary, hopefully. |
|
@esc: Awesome. Let me know if there are things I can do to be useful. |
Great, that sounds very good! Thank you in advance for your kind offer. I'll probably need some user-feedback on this use-case shortly after the holidays so I'll probably be reaching out to you then. |
Based on this plan, I'll include this in the 0.39RC milestone for now, with the hope that we see some progress on this PR in time. |
|
(Marked as "waiting on reviewer" as I believe the next step is when @esc looks into support for multiple LLVMs) |
|
Sounds good, all. |
|
Just a quick heads up here: the work to support the Apple M1 Silicon is taking more time than expected, so I am having some delays. However, this is the next item in my Queue. (My (experimental) queue is now here: https://github.com/orgs/numba/projects/4/views/1) |
|
Great, thanks! |
|
We plan to discuss supporting multiple LLVM versions on Tuesday April 5, 2022. See meeting calendar for the time and meeting video conferencing link. |
|
Hi, I was trying to compile the Debian llvmlite 0.39.1 version with the patch from here against llvm-13, but ran into a problem. That include is wrapped in a But RemarkStreamer.h doesn't appear to be present in the llvm-13 package, though LLVMRemarkStreamer does exist. Since I'm using the released version 0.39.1, I made sure the line I'm having trouble with is also present in HEAD. I forced the above pull request to apply against 0.39.1 and refreshed it, dropping one change, and then added removing the include "llvm/IR/RemarkStreamer.h" block to the patch Leaving me with this patch to make 0.39.1 compatible with llvm-13 From 1d928ebcd59b23b5050234a2bf71f9be7f5f6bd1 Mon Sep 17 00:00:00 2001
From: Richard Barnes <rbarnes@...>
Date: Wed, 1 Dec 2021 10:29:08 -0700
Subject: [PATCH] Enable LLVM-12 and LLVM-13
---
ffi/build.py | 5 ++---
ffi/targets.cpp | 2 ++
llvmlite/tests/test_binding.py | 19 ++++++++++++++++---
3 files changed, 20 insertions(+), 6 deletions(-)
--- a/ffi/build.py
+++ b/ffi/build.py
@@ -163,9 +163,8 @@
print(msg)
print(warning + '\n')
else:
-
- if not out.startswith('11'):
- msg = ("Building llvmlite requires LLVM 11.x.x, got "
+ if not (out.startswith('11') or out.startswith('12') or out.startswith('13')):
+ msg = ("Building llvmlite requires LLVM 11-13.x.x, got "
"{!r}. Be sure to set LLVM_CONFIG to the right executable "
"path.\nRead the documentation at "
"http://llvmlite.pydata.org/ for more information about "
--- a/ffi/targets.cpp
+++ b/ffi/targets.cpp
@@ -204,7 +204,9 @@
rm = Reloc::DynamicNoPIC;
TargetOptions opt;
+#if LLVM_VERSION_MAJOR < 12
opt.PrintMachineCode = PrintMC;
+#endif
opt.MCOptions.ABIName = ABIName;
bool jit = JIT;
--- a/llvmlite/tests/test_binding.py
+++ b/llvmlite/tests/test_binding.py
@@ -18,6 +18,16 @@
from llvmlite.tests import TestCase
+def clean_string_whitespace(x: str) -> str:
+ # Remove trailing whitespace from the end of each line
+ x = re.sub(r"\s+$", "", x, flags=re.MULTILINE)
+ # Remove intermediate blank lines
+ x = re.sub(r"\n\s*\n", r"\n", x, flags=re.MULTILINE)
+ # Remove extraneous whitespace from the beginning and end of the string
+ x = x.strip()
+ return x
+
+
# arvm7l needs extra ABI symbols to link successfully
if platform.machine() == 'armv7l':
llvm.load_library_permanently('libgcc_s.so.1')
@@ -555,7 +565,10 @@
bd = ir.IRBuilder(fn.append_basic_block(name="<>!*''#"))
bd.ret(ir.Constant(ir.IntType(32), 12345))
asm = str(mod)
- self.assertEqual(asm, asm_nonalphanum_blocklabel)
+ self.assertEqual(
+ clean_string_whitespace(asm),
+ clean_string_whitespace(asm_nonalphanum_blocklabel)
+ )
def test_global_context(self):
gcontext1 = llvm.context.get_global_context()
@@ -640,7 +653,7 @@
def test_version(self):
major, minor, patch = llvm.llvm_version_info
# one of these can be valid
- valid = [(11,)]
+ valid = [(11,), (12,), (13,)]
self.assertIn((major,), valid)
self.assertIn(patch, range(10))
--- a/ffi/passmanagers.cpp
+++ b/ffi/passmanagers.cpp
@@ -17,9 +17,6 @@
#include "llvm-c/Transforms/IPO.h"
#include "llvm-c/Transforms/Scalar.h"
#include "llvm/IR/LegacyPassManager.h"
-#if LLVM_VERSION_MAJOR > 11
-#include "llvm/IR/RemarkStreamer.h"
-#endif
#include "llvm/IR/LLVMRemarkStreamer.h"
#include "llvm/Remarks/RemarkStreamer.h"
#include "llvm/Transforms/IPO.h"With the above patch the debian package for 0.39.1 builds and passes all of its tests cases. (on x86_64) For the real test then I built numba using the version of llvmlite 0.39.1 modified to use llvm-13. I do have some failures, though I think most of them are due Debian's test environment being different from what the tests expect.
One chunk of failures is problems with gdb not behaving as numba expects. I've got a a couple memory leak errors And a couple of long error messages like this which seem to have the best chance of having been caused by the update to llvm-13. |
|
Yes, I'm working on these fixes in #830 , but it's going rather slowly. |
|
@apmasell do we want to close this issue, since we don't intend to support LLVM 12 or 13? |
|
I think so. Most of this work has been superseded by the LLVM 14 changes. |
|
@apmasell thank you! @detrout thank you again for asking about this, unfortunately, I am not sure we'll be able to help you out here due to resource constraints. llvmlite 0.39.1 only supports LLVM 11. llvmlite 0.40.0 will most likely support only LLVM 14 (or stay at 11). There is also some text about this on the FAQ of the llvmlite docs here: https://llvmlite.readthedocs.io/en/latest/faqs.html FWIW: Using Numba with llvmlite REQUIRES a patched version of LLVM to pass the whole test suite and function correctly, we discourage using this setup with a dynamically linked LLVM. Not doing this may end up in strange bugs that manifest on user systems and then end up on our tracker, but which are actually caused by the incorrect redistribution of the software: But of course, the terms of the OSS licence mean you have all the freedom. 🎉 |
No description provided.