Skip to content

Conversation

@devajithvs
Copy link
Contributor

@devajithvs devajithvs commented Dec 15, 2023

This Pull request:

Changes or fixes:

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #14209

@devajithvs devajithvs requested a review from hahnjo December 15, 2023 12:32
@phsft-bot
Copy link

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@vgvassilev
Copy link
Member

That’s a good direction. Let’s wait for the dust to settle after the upgrade for a few weeks before landing this. There were several significant changes between llvm16 and llvm17 in order for jitlink to become default. Can you check the llvm git history?

@hahnjo
Copy link
Member

hahnjo commented Dec 15, 2023

That’s a good direction. Let’s wait for the dust to settle after the upgrade for a few weeks before landing this.

Yes, we discussed to not merge before early next year.

There were several significant changes between llvm16 and llvm17 in order for jitlink to become default. Can you check the llvm git history?

I put a bit more documentation in #14209; the switch to default happened very early in the LLVM 17 cycle (February and April, after branching in January). I was hoping there are only very few fixes around that time, if at all...

@phsft-bot
Copy link

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@github-actions
Copy link

github-actions bot commented Dec 15, 2023

Test Results

    22 files      22 suites   3d 19h 1m 33s ⏱️
 3 704 tests  3 703 ✅ 0 💤 1 ❌
79 537 runs  79 536 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit d311996.

♻️ This comment has been updated with latest results.

@vgvassilev
Copy link
Member

Hi @lhames, following the deprecation procedure for RuntimeDyld we are hitting a few test failures. Some of them have things like cling JIT session error: In graph cling-module-315-jitted-objectbuffer, section .eh_frame: relocation target "DW.ref.__gxx_personality_v0" at address 0x7f8d0b32f040 is out of range of Delta32 fixup at 0x7f8c6c00d7c3 (<anonymous block> @ 0x7f8c6c00d7b0 + 0x13). You can find more here: https://github.com/root-project/root/runs/28400824352

I am not sure what's our way forward here.

@hahnjo
Copy link
Member

hahnjo commented Aug 25, 2024

I now understand what is going wrong: @Axel-Naumann pushed me into the right direction by noticing that the errors concern vtables and typeinfo symbols, which are subject to reemission on our side. Then our WeakTypeinfoVTablePass will mark them as weak to make sure the JIT infrastructure deduplicates the symbols as needed. Additionally, ReuseExistingWeakSymbols will entirely remove the definitions if the symbol can be found in the process, for example for compiled classes. Now the problem is that the symbols were originally marked as "local to the shared library" and relocations can take advantage of this. This is where PreventLocalOptPass comes in, but it ran before the two passes mentioned before and therefore doesn't consider these symbols.

So tldr, we can fix the problems by reordering the passes. Let's slot this change in after the upgrade to LLVM 18, which is also where upstream changed to the default to JITLink.

@hahnjo hahnjo changed the title [cling] Enable JITLink for AArch64 and x86_64 on Linux [cling] Enable JITLink for x86_64 on Linux Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cling] Enable JITLink for AArch64 and x86_64 on Linux

4 participants