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

MNT: allow llvm 9.0.1 to be used #571

Closed
wants to merge 2 commits into from

Conversation

tacaswell
Copy link

This only removes the version checks.

@esc
Copy link
Member

esc commented Apr 2, 2020

@tacaswell thanks for submitting this. We are aware that llvmlite will need LLVM 9 support soon. We (the core developers) expect to tackle this during the next (0.50.0) release cycle. This will commence after the current release (0.49.0 and currently in release candidate (RC) phase) is done. The current ETA is 9th April (give or take). We thank you in advance for your patience in this matter.

Also note, that simply removing the version checks is unlikely to yield a usable llvmlite. In fact, work has begun already to make this possible here: #548 If you believe that this PR is a duplicate of that, please do follow up and close it, thanks!

@tacaswell
Copy link
Author

Fair enough, but that PR looks like it is mostly patches to get llvm 9 to install in CI (Arch linux already has llvm 9.0.1 packaged).

The only test that fails locally is

>>> llvmlite.tests.main()
..s...........................................................s..................F...........................................................................................................................................................................
======================================================================
FAIL: test_parse_bitcode_error (llvmlite.tests.test_binding.TestModuleRef)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.9/site-packages/llvmlite/tests/test_binding.py", line 548, in test_parse_bitcode_error
    self.assertIn("Invalid bitcode signature", str(cm.exception))
AssertionError: 'Invalid bitcode signature' not found in 'LLVM bitcode parsing error\nfile too small to contain bitcode header'

----------------------------------------------------------------------
Ran 253 tests in 1.937s

FAILED (failures=1, skipped=2)

which is

def test_parse_bitcode_error(self):
with self.assertRaises(RuntimeError) as cm:
llvm.parse_bitcode(b"")
self.assertIn("LLVM bitcode parsing error", str(cm.exception))
self.assertIn("Invalid bitcode signature", str(cm.exception))
which looks like llvm has changed the text of the warning a bit. I assume the thing you want done here is to version gate checking the strings?

@sklam
Copy link
Member

sklam commented Apr 2, 2020

@tacaswell, the llvm9 PR is stuck at getting it to not segfault on power architectures. In some way the version checks serve as a stamp of approval that we have verified that the particular LLVM version will work for llvmlite uses, mainly that it works fine for a JIT compiler projects (like Numba).

Please be aware that there is a set of patches that we ship along in each of our llvmlite builds. These patches are necessary. The LLVM builds that comes with the distribution may not work well with the llvmlite intended usecases.

I would prefer to bump the version checks once llvm9 passes our CI.

which looks like llvm has changed the text of the warning a bit. I assume the thing you want done here is to version gate checking the strings?

Yes, version gating the checks will do.

@tacaswell
Copy link
Author

Sorry for being thrashy / disruptive.

@sklam Should I close this PR and open a PR against your branch in #548 ?

I'm currently running the numba test suite, it is showing a bunch of errors, hopefully those are not issues with the system build...

@stuartarchibald
Copy link
Contributor

I'm currently running the numba test suite, it is showing a bunch of errors, hopefully those are not issues with the system build...

@tacaswell please could you dump them into a <details></details> block or similar? We might be able to make a guess based on prior things we've bumped into when testing LLVMs! Thanks!!

@tacaswell
Copy link
Author

@stuartarchibald I'll work on that. Currently hanging on test_multiprocessing_fork_concurrent_mix_use_masks_workqueue (numba.tests.test_parallel_backend.TestSpecificBackend) ...

@stuartarchibald
Copy link
Contributor

@tacaswell which OS? Are you building Numba master? These tests really hammer machines and libraries to breaking point. There's a timeout of 9 minutes, if it's still stuck then, it's possibly locked, if you can attach to it with gdb and get a back trace for a new issue on the Numba tracker, that'd be great, thanks!

@tacaswell
Copy link
Author

Arch, yes numba master. It has been hung for about an hour. Can you point me to instruction on how to attach to it with gdb and get the back trace?

jupiter@15:40  ➤  ps -u | grep runtests
tcaswell   30586 54.7 17.9 56590440 5902964 pts/0 Sl+ 13:13 119:32 python runtests.py -v
tcaswell   40774  0.0  0.3 915536 102800 pts/0   Sl+  15:17   0:00 /home/tcaswell/.virtualenvs/bleeding/bin/python -m numba.runtests numba.tests.test_parallel_backend.TestParallelBackend.test_multiprocessing_fork_concurrent_mix_use_masks
tcaswell   40784  0.0  0.2 910692 95616 pts/0    Sl+  15:17   0:00 /home/tcaswell/.virtualenvs/bleeding/bin/python -m numba.runtests numba.tests.test_parallel_backend.TestParallelBackend.test_multiprocessing_fork_concurrent_mix_use_masks
tcaswell   40793  0.0  0.3 913668 100644 pts/0   Sl+  15:17   0:00 /home/tcaswell/.virtualenvs/bleeding/bin/python -m numba.runtests numba.tests.test_parallel_backend.TestParallelBackend.test_multiprocessing_fork_concurrent_mix_use_masks

I'm also doing this with cpython master and numpy master so maybe I should also try in a more stable environment...

@stuartarchibald
Copy link
Contributor

@tacaswell apologies, completely missed your reply. You can just do:

 gdb '-ex' 'set confirm off' -ex 'bt' -ex 'q' attach <pid>

where <pid> is the process id. This will attach to the process, back trace (bt) and then quit.

@tacaswell
Copy link
Author

Sorry, this completely fell off my radar. It looks like there is work going on in #548 so I am going to close this.

@tacaswell tacaswell closed this May 19, 2020
@tacaswell tacaswell deleted the mnt_llvm_901 branch May 19, 2020 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants