Update llvmlite to be compatible with llvm-17 #1042

modiking · 2024-04-24T17:55:14Z

~/llvm-project# git log
commit 6009708b4367171ccdbf4b5905cb6a803753fe18 (HEAD -> release/17.x, tag: llvmorg-17.0.6, upstream/release/17.x)
~/llvm-project/build-rel-static# cmake \
   -G Ninja \
   -DCMAKE_CXX_COMPILER=clang++ \
   -DCMAKE_C_COMPILER=clang \
   -DCMAKE_ASM_COMPILER=clang \
   -DCMAKE_ASM_COMPILER_ID=Clang \
   -DLLVM_TARGETS_TO_BUILD="X86" \
   -DLLVM_ENABLE_LLD=ON\
   -DLLVM_ENABLE_PROJECTS="clang;lld" \
   -DCMAKE_BUILD_TYPE=RelWithDebInfo \
   -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \
   -DCMAKE_INSTALL_PREFIX=$1 \
   -DBUILD_SHARED_LIBS=false \
   -DLLVM_ENABLE_ASSERTIONS=ON \
   -DLLVM_OPTIMIZED_TABLEGEN=true \
    ../llvm
~/llvm-project/build-rel-static# ninja
...
~/migration/llvmlite# git log
commit 4907a4eee128ae5d95eddbd0ce4aeaa83bab4787 (HEAD -> modiking/llvm17, origin/modiking/llvm17)
~/migration/llvmlite# LLVM_CONFIG=/home/modimo/llvm-project/build-rel-static/bin/llvm-config python3.9 setup.py build
~/migration/llvmlite# LD_LIBRARY_PATH=/home/modimo/llvm-project/build-rel-static/lib python3.9 runtests.py 
..s................................................................s.............................................................x.sss...........s.....................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 375 tests in 9.153s

OK (skipped=6, expected failures=1)

TODO:

I haven't hooked up all the boilerplate for every pass. With New Pass Manager (NPM) certain passes become specific to Module/Function/Loop and need adaptors to pass to the FunctionPassManager/ModulePassManager. We can do this with Macros in the code or I can generate separate methods for FPM/MPM. Alternatively, MPM is a superset of FPM so if there's interest we can consolidate everything into just MPM for simplicity.
I haven't tested with SVML patches in LLVM nor with SVML in general. I don't expect too much work beyond rebasing the LLVM patch however. Would appreciate any guidance on how to do this
Currently I don't have a conda package for LLVM. Would appreciate any help on getting that set up

High-level details:

The llvm-c APIs have not been updated from NPM. AFAICT the "c" APIs are ad-hoc maintained by whoever uses them. We can definitely bring up that boilerplate but currently the setup is using the C++ APIs
NPM has additional state for analysis managers it needs to carry. It also must be initialized by the pass builder before using. I've kept the high level python APIs as close as before but we can certainly re-evaluate how we want to expose these to keep things simpler
Opaque pointers means types can no longer be queried on pointers. Instead I've updated the general type query to use global_value_type which looks at the value rather than the pointer

modiking · 2024-04-24T18:01:48Z

ffi/transforms.cpp

+    LLVMFunctionAnalysisManager FAM, LLVMCGSCCAnalysisManager CGAM,
+    LLVMPassInstrumentationCallbacks PIC) {
+    // TODO handle PB memory better
+    if (PB)


This is here because the PipelineTuningOptions cannot currently be updated after creation of a PassBuilder. I'm planning on floating the idea to LLVM upstream of allowing this change so we don't have to do this stuff

gmarkall · 2024-04-25T22:45:08Z

Thanks for the PR @modiking - I was hoping to provide some timely feedback on your PR I'm out on PTO for the rest of the week - I'll follow up next week.

gmarkall · 2024-04-29T10:39:43Z

Hi @modiking - I'm looking at this a bit today - I have a couple of clarifying questions:

Do you have a corresponding Numba fork that you're maintaining?
Are you using llvmlite for non-Numba use cases?
Have you tried building LLVM with all targets enabled? (I'm curious if the RISC-V tests are OK)

modiking · 2024-04-29T18:18:47Z

Hi @modiking - I'm looking at this a bit today - I have a couple of clarifying questions:

Do you have a corresponding Numba fork that you're maintaining?

I haven't tested this against Numba yet. I wanted to make sure we were on the same page on how we hook up custom passes before doing more. Are there docs on how to test it? Also for our internal use case of Numba I suspect we'll upgrade this in tandem with whatever the latest release is given this is a more or less a "breaking" change on some level.

Are you using llvmlite for non-Numba use cases?

Internally I believe we are. However part of the reason we're investing in the upgrade is that figuring out the answer to that question is quite difficult.

Have you tried building LLVM with all targets enabled? (I'm curious if the RISC-V tests are OK)

I believe the RISC-V tests are running with my build, I'll double check. I've currently working on getting the SVML patches upgraded and testing that scenario.

modiking · 2024-04-30T18:55:34Z

You were right that the RISCV tests were not running. Building LLVM against all targets showed the register allocation changed but the instruction sequences are still the same so updated the tests:

~/migration/llvmlite# LD_LIBRARY_PATH=/home/modimo/llvm-project2/build-rel-static-all/lib python3.9 runtests.py -v &> test.log
~/migration/llvmlite# grep RISCV test.log
test_rv32d_ilp32 (llvmlite.tests.test_binding.TestRISCVABI) ... ok
test_rv32d_ilp32d (llvmlite.tests.test_binding.TestRISCVABI) ... ok
test_rv32d_ilp32f (llvmlite.tests.test_binding.TestRISCVABI) ... ok

makslevental · 2024-05-02T17:10:01Z

I would be interested in helping/leaning on this but...

I forked @modiking's fork (here)

$ git log | head -n1
commit 6009708b4367171ccdbf4b5905cb6a803753fe18
$ export LLVM_CONFIG=$PWD/build-rel-static/bin/llvm-config && cd ../llvmlite
$ git branch -v
  main            622c33f Merge pull request #1025 from dlee992/skip_if_meet_raise_bb
* modiking/llvm17 4907a4e [behind 4] Format
$ git log | head -n1
commit 4907a4eee128ae5d95eddbd0ce4aeaa83bab4787
$ python --version
Python 3.9.19

Then just blind python3.9 setup.py build gets me

targets.cpp:1:10: fatal error: ffi_types.h: No such file or directory
    1 | #include "ffi_types.h"
      |          ^~~~~~~~~~~~~
transforms.cpp:1:10: fatal error: ffi_types.h: No such file or directory
    1 | #include "ffi_types.h"
      |          ^~~~~~~~~~~~~

because libffi keeps those things in ffi.h on ubuntu. Patching that I get all kinds of stuff like

targets.cpp:35:1: error: expected constructor, destructor, or type conversion before ‘LLVMPY_GetProcessTriple’
   35 | LLVMPY_GetProcessTriple(const char **Out) {
      | ^~~~~~~~~~~~~~~~~~~~~~~
targets.cpp:45:1: error: expected constructor, destructor, or type conversion before ‘LLVMPY_GetHostCPUFeatures’
   45 | LLVMPY_GetHostCPUFeatures(const char **Out) {
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
targets.cpp:62:1: error: expected constructor, destructor, or type conversion before ‘LLVMPY_GetDefaultTargetTriple’
   62 | LLVMPY_GetDefaultTargetTriple(const char **Out) {
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
targets.cpp:67:1: error: expected constructor, destructor, or type conversion before ‘LLVMPY_GetHostCPUName’
   67 | LLVMPY_GetHostCPUName(const char **Out) {

which turns out to be due to the fact that API_EXPORT isn't included from core.h. Patching that I now get all kinds of stuff that seem like API mismatches?

targets.cpp:274:1: error: variable or field ‘LLVMPY_AddTargetLibraryInfoPass’ declared void
  274 | LLVMPY_AddTargetLibraryInfoPass(LLVMFunctionAnalysisManager FAM,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
targets.cpp:274:33: error: ‘LLVMFunctionAnalysisManager’ was not declared in this scope; did you mean ‘LLVMRunFunctionPassManager’?
  274 | LLVMPY_AddTargetLibraryInfoPass(LLVMFunctionAnalysisManager FAM,
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                 LLVMRunFunctionPassManager
targets.cpp:275:33: error: expected primary-expression before ‘const’
  275 |                                 const char *TripleStr) {
      |                                 ^~~~~
transforms.cpp:18:9: error: ‘llvm::PipelineTuningOptions’ has not been declared
   18 |         llvm::PipelineTuningOptions PTO = llvm::PipelineTuningOptions())
      |         ^~~~
transforms.cpp:23:11: error: ‘PipelineTuningOptions’ in namespace ‘llvm’ does not name a type
   23 |     llvm::PipelineTuningOptions PTO;

I'll keep "following my nose" (i.e., patching stuff willy-nilly) but I'm just kind of surprised/confused about how you were able to successfully build @modiking

modiking · 2024-05-03T03:15:20Z

Hi @makslevental, thanks for taking a look! I took a look and there’s a simple and dumb answer to this: I forgot to add the new file “ffi_types.h” to git 🤦‍♂️

modiking · 2024-05-03T03:29:37Z

I'm also seeing that:

test_run_with_remarks_filter_in (llvmlite.tests.test_binding.TestFunctionPassManager) ... Fatal Python error: Segmentation fault

Current thread 0x00007f3eb0c34740 (most recent call first):
  File "/home/modimo/migration/llvmlite/llvmlite/binding/ffi.py", line 206 in __call__
  File "/home/modimo/migration/llvmlite/llvmlite/binding/transforms.py", line 125 in _dispose
  File "/home/modimo/migration/llvmlite/llvmlite/binding/ffi.py", line 349 in close
  File "/home/modimo/migration/llvmlite/llvmlite/binding/ffi.py", line 389 in __del__

Fails with SEGFAULT on first run but then subsequently succeeds. I suspect there's some memory management issue happening with the sub-optimal way I'm dealing with the PassBuilder. I'm hoping to modify PassBuilder in LLVM upstream to allow modifying PipelineTuningOptions so this isn't as sub-optimal.

I'm on PTO till next Monday but when I get back I'll track this down so we're not blocked on any future LLVM patch. I also see the build issue with the CI where override is not working correctly and I'll look into that as well.

makslevental · 2024-05-03T03:56:10Z

@modiking SGTM. Also (if you're up for it) when you get back from PTO we should sync up and maybe colab - my goal is to bump all the way to TOM LLVM.

azrael417 · 2024-05-06T09:48:10Z

Hello everybody, I am super interested in this fix as well since I am building a code which requires LLVM17 and cannot use LLVM14.

gmarkall · 2024-05-09T11:15:22Z

@modiking Thanks for your efforts so far here, and apologies for the delay in getting back to looking at this. I appreciate the time and effort you've taken to fully map out a way towards getting llvmlite to LLVM 17. For us to be able to move forward in the context of Numba + llvmlite, we need to do things a bit more piecewise than advancing everything forward to LLVM 17 in one go, because getting to LLVM 17 with Numba would mean making sure it all works with the new pass manager, opaque pointers, and any other API changes that I don't yet know about, and having enough time to catch any issues that result from that migration (particularly through having it working in Numba in CI for a while).

With that in mind, the first thing I think we need to do is to have separate APIs for the new and legacy pass managers, allowing Numba to have code to use one or the other and a config flag to switch between the two. My understanding of the changes in this PR is that it retains the existing interface, but with the new pass manager.

In a separate PR from @yashssh (#1046), there is the other approach where the old and new pass managers are used with separate APIs. That doesn't yet do anything about the refprune pass though, and it looks like you've already worked out updating it for the new pass manager. I was able to make some minimal changes to Numba to test it with the new pass manager in PR #1046, but without the refcount pruning pass.

With those points in mind, for us to progress forward, is it possible for you to:

Take the changes you made to the refcount pruning pass so that it could be used with the new pass manager, and create a new PR with them based on top of Add basic infra required to move Numba to NewPassManager #1046? The idea here is that I'd then be able to test those changes with Numba, and the move forward with finding a way to get your updates there merged into llvmlite.
Have a look over Add basic infra required to move Numba to NewPassManager #1046 and provide any other thoughts / feedback / considerations / comparisons based on your experience and work here so far

please? How does this sound?

modiking · 2024-05-10T17:44:59Z

It looks like there's enough parties here that it would be good to align on all our goals and figure out what's the best way forward. There's Numba office hours next Tuesday at 8:30 AM PST (https://calendar.google.com/calendar/embed?src=5rqnddm4gjrdfjm31fsv182epk%40group.calendar.google.com) that seems like a good time to meet up. We'll also get more Numba perspective on how we want to proceed.

cc @makslevental @azrael417 @gmarkall

modiking · 2024-05-10T17:51:24Z

@modiking Thanks for your efforts so far here, and apologies for the delay in getting back to looking at this. I appreciate the time and effort you've taken to fully map out a way towards getting llvmlite to LLVM 17. For us to be able to move forward in the context of Numba + llvmlite, we need to do things a bit more piecewise than advancing everything forward to LLVM 17 in one go, because getting to LLVM 17 with Numba would mean making sure it all works with the new pass manager, opaque pointers, and any other API changes that I don't yet know about, and having enough time to catch any issues that result from that migration (particularly through having it working in Numba in CI for a while).

True, this does require extensive testing and validation.

With that in mind, the first thing I think we need to do is to have separate APIs for the new and legacy pass managers, allowing Numba to have code to use one or the other and a config flag to switch between the two. My understanding of the changes in this PR is that it retains the existing interface, but with the new pass manager.

Can you elaborate more on the advantages of doing it this way? Anything that uses llvmlite needs to be atomically on one pass manager or the other so having NPM be an option requires full correctness.

In a separate PR from @yashssh (#1046), there is the other approach where the old and new pass managers are used with separate APIs. That doesn't yet do anything about the refprune pass though, and it looks like you've already worked out updating it for the new pass manager. I was able to make some minimal changes to Numba to test it with the new pass manager in PR #1046, but without the refcount pruning pass.

With those points in mind, for us to progress forward, is it possible for you to:

Take the changes you made to the refcount pruning pass so that it could be used with the new pass manager, and create a new PR with them based on top of Add basic infra required to move Numba to NewPassManager #1046? The idea here is that I'd then be able to test those changes with Numba, and the move forward with finding a way to get your updates there merged into llvmlite.

Have a look over Add basic infra required to move Numba to NewPassManager #1046 and provide any other thoughts / feedback / considerations / comparisons based on your experience and work here so far

please? How does this sound?

My plan is to test Numba as well once this change is functionally complete. Definitely do not want to push 2 separate solutions to the same problem though. I suggested earlier to meet up at the next Numba office hours to discuss this-how about we discuss it in more detail then?

…rameter yet

gmarkall · 2024-05-14T09:37:24Z

Can you elaborate more on the advantages of doing it this way? Anything that uses llvmlite needs to be atomically on one pass manager or the other so having NPM be an option requires full correctness.

My concern is that between the old and new pass managers we'll see some change in optimization behaviour for certain cases, which some Numba users are sensitive to - there's not a single place I can point to that lays it out clearly, but some past and recent discussions around this area are:

I'd envision the way forward is that Numba could be atomically switched between the old and new pass managers, via a configuration variable. Initially I think we'd use the old pass manager by default, but probably test with the new pass manager enabled in CI. Then switch over to the new pass manager being default once we feel comfortable that most issues are worked out. If someone's use case is negatively affected by the new pass manager, then they can switch back to the old one with the configuration variable.

If we only have the new pass manager and "flip the switch" in one go, I don't think we'll have a way to address individual cases, other than attempting to fix them with the way we use the new pass manager for a subsequent release. Because Numba releases are not on a frequent cadence, and our resources to look into a particular issue could be limited at any given time, this might not be a straightforward experience either for Numba users or us as the maintainers.

My plan is to test Numba as well once this change is functionally complete.

A general question about getting Numba to the point where it can be tested with this branch - is there a quick solution for Numba's assumption that pointers are typed, thus that you can get the type and size of the pointee, or would there be a lot of code changes needed in Numba to switch over to opaque pointers and run with the LLVM 17 branch?

Definitely do not want to push 2 separate solutions to the same problem though. I suggested earlier to meet up at the next Numba office hours to discuss this-how about we discuss it in more detail then?

Sure, I'd be happy to discuss this in the next office hours as well - note that this week the office hours are the EU version, which are at 12pm GMT (1pm BST, 2pm CET) which I think is 5am in PST, so we could also do this the following week's office hours if that works better?

makslevental · 2024-05-21T15:03:33Z

It looks like there's enough parties here that it would be good to align on all our goals and figure out what's the best way forward. There's Numba office hours next Tuesday at 8:30 AM PST

Sorry I missed this. I'm going to drop in today (in about 30) and ask around. Otherwise we can plan for next week or we can coordinate a sideband convo over email or something. FYI I generally hang out on the LLVM discord https://discord.gg/fKKqJ9sn (under the same handle).

dlee992 · 2024-05-21T16:27:43Z

As a Numba user, I do like the feature opaque pointers, since sometimes I need to write some lowering code in Numba with llvmlite builder/IR APIs. And many times, the bitcasts between ptr and pointee cause me many troubles to debug/test.
I just wonder after enabling opaque pointers, users who want to extend numba lower parts can work more efficiently.

But right now, I believe numba has many code locations using bitcasts to do many "magical" things (in my view). It could be painful for this transition.

gmarkall · 2024-05-21T17:14:49Z

As a Numba user, I do like the feature opaque pointers, since sometimes I need to write some lowering code in Numba with llvmlite builder/IR APIs. And many times, the bitcasts between ptr and pointee cause me many troubles to debug/test. I just wonder after enabling opaque pointers, users who want to extend numba lower parts can work more efficiently.

I should think it would make it easier to work on these parts more efficiently.

But right now, I believe numba has many code locations using bitcasts to do many "magical" things (in my view). It could be painful for this transition.

Maybe... I hope it won't be too long before we find out - Modi's discussion in the open meeting sounded encouraging, but we'll have to see what fails when we make the changes too.

@modiking

Based on the changes introduced in numba#1042 by @modiking

@modiking

Based on the changes introduced in numba#1042 by @modiking

@modiking

Based on the changes introduced in numba#1042 by @modiking

@modiking

Based on the changes introduced in numba#1042 by @modiking

modiking added 4 commits April 23, 2024 15:26

initial commit

30b251f

test fixes and code updates

35dc025

Cleanup

13dca92

Format

4907a4e

modiking marked this pull request as draft April 24, 2024 17:58

modiking marked this pull request as ready for review April 24, 2024 17:59

modiking commented Apr 24, 2024

View reviewed changes

modiking added 3 commits April 24, 2024 11:06

Minor fixes

49f73ad

clang-format

4a63bf8

Following numba#1035 on updating pipeline files

7376017

remove leftover code, update RISCV tests

781773b

Add ffi_types.h

4de4afb

modiking added 2 commits May 7, 2024 16:02

Fix dangling PB pointer

dde7f9c

Move everything to llvm-17, add C++17 requirement for MSVC

3725b26

esc mentioned this pull request May 12, 2024

LLVM update plan? #1048

Open

modiking added 3 commits May 13, 2024 11:43

plumb llvm 17 through (maybe?) all CI steps

620c2a3

flake8 formatting

ea53b03

clang-format

63afeee

modiking added 3 commits May 13, 2024 20:01

hardcode setup_conda_environment.sh as that doesn't take in a LLVM pa…

5e41cb7

…rameter yet

fix docs and windows build

fe05327

update docs link because the release is 17.0.1

880a9c7

dlee992 mentioned this pull request May 21, 2024

[llvmlite] Support opaque pointers. #908

Closed

yashssh added a commit to yashssh/llvmlite that referenced this pull request Jun 7, 2024

Port refprune to NewPassManager

de2a5ad

Based on the changes introduced in numba#1042 by @modiking

yashssh mentioned this pull request Jun 7, 2024

Port refprune pass to NewPassManager #1057

Merged

yashssh added a commit to yashssh/llvmlite that referenced this pull request Jul 9, 2024

Port refprune to NewPassManager

4eeb079

Based on the changes introduced in numba#1042 by @modiking

yashssh added a commit to yashssh/llvmlite that referenced this pull request Jul 18, 2024

Port refprune to NewPassManager

6b21b2a

Based on the changes introduced in numba#1042 by @modiking

esc added the 4 - Waiting on reviewer label Jul 18, 2024

yashssh added a commit to yashssh/llvmlite that referenced this pull request Aug 19, 2024

Port refprune to NewPassManager

4b260e8

Based on the changes introduced in numba#1042 by @modiking

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update llvmlite to be compatible with llvm-17 #1042

Update llvmlite to be compatible with llvm-17 #1042

modiking commented Apr 24, 2024 •

edited

Loading

modiking Apr 24, 2024

gmarkall commented Apr 25, 2024

gmarkall commented Apr 29, 2024

modiking commented Apr 29, 2024

modiking commented Apr 30, 2024 •

edited

Loading

makslevental commented May 2, 2024

modiking commented May 3, 2024

modiking commented May 3, 2024 •

edited

Loading

makslevental commented May 3, 2024 •

edited

Loading

azrael417 commented May 6, 2024

gmarkall commented May 9, 2024

modiking commented May 10, 2024 •

edited

Loading

modiking commented May 10, 2024

gmarkall commented May 14, 2024

makslevental commented May 21, 2024 •

edited

Loading

dlee992 commented May 21, 2024

gmarkall commented May 21, 2024

Update llvmlite to be compatible with llvm-17 #1042

Are you sure you want to change the base?

Update llvmlite to be compatible with llvm-17 #1042

Conversation

modiking commented Apr 24, 2024 • edited Loading

modiking Apr 24, 2024

Choose a reason for hiding this comment

gmarkall commented Apr 25, 2024

gmarkall commented Apr 29, 2024

modiking commented Apr 29, 2024

modiking commented Apr 30, 2024 • edited Loading

makslevental commented May 2, 2024

modiking commented May 3, 2024

modiking commented May 3, 2024 • edited Loading

makslevental commented May 3, 2024 • edited Loading

azrael417 commented May 6, 2024

gmarkall commented May 9, 2024

modiking commented May 10, 2024 • edited Loading

modiking commented May 10, 2024

gmarkall commented May 14, 2024

makslevental commented May 21, 2024 • edited Loading

dlee992 commented May 21, 2024

gmarkall commented May 21, 2024

modiking commented Apr 24, 2024 •

edited

Loading

modiking commented Apr 30, 2024 •

edited

Loading

modiking commented May 3, 2024 •

edited

Loading

makslevental commented May 3, 2024 •

edited

Loading

modiking commented May 10, 2024 •

edited

Loading

makslevental commented May 21, 2024 •

edited

Loading