[wip] Add llvm 21 support #672
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #672 +/- ##
=======================================
Coverage 81.46% 81.46%
=======================================
Files 9 9
Lines 4213 4213
=======================================
Hits 3432 3432
Misses 781 781
🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
clang-tidy review says "All clean, LGTM! 👍" |
mcbarton
force-pushed
the
Add-llvm-21-support
branch
from
086ee33 to
9f09f8a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
mcbarton
force-pushed
the
Add-llvm-21-support
branch
from
fbd1215 to
410db40
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
3 similar comments
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Current progress with adding compatibility for llvm 21. Can build on all platforms. Can pass all tests for native Windows, native Linux and Emscripten platform. MacOS errors for the following test on both arm and x86 1: [ RUN ] InterpreterTest.IncludePaths |
mcbarton
force-pushed
the
Add-llvm-21-support
branch
from
c04a271 to
015f4f0
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
mcbarton
force-pushed
the
Add-llvm-21-support
branch
from
015f4f0 to
60df3d7
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
For the MacOS failures we apear to be triggering this line CppInterOp/lib/CppInterOp/Paths.cpp Line 161 in 82f08c6 |
mcbarton
force-pushed
the
Add-llvm-21-support
branch
from
60df3d7 to
2a280d1
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Something has happened to the llvm 21 release branch in the last 2 weeks to break the ScopeReflectionTest.GetNumBases test in CppInterOp. Also the Windows build of CppInterOp has broken somehow. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Hey @mcbarton , Are you working on this ? I think I saw quite some errors errors while trying to build cppinterop against latest llvm 21 few days back. |
|
clang-tidy review says "All clean, LGTM! 👍" |
@anutosh491 With the exception of the Windows native build you can build CppInterOp against llvm 21 with the changes in this PR (Windows fails building llvm). The tests do not pass however. Given the changes in this PR are minimal, then the source of these errors are likely on the llvm side. I will update this PR soon to get rid of the Windows Emscripten cross compile patch since it is no longer needed, and to update the documentation. Not sure I have any spare time at the moment though to work out the source of the failing tests. |
instantiate templated classes to TSK_ExplicitInstantiationDefinition
instantiate templated classes to TSK_ExplicitInstantiationDefinition
|
clang-tidy review says "All clean, LGTM! 👍" |
Mcbarton/add llvm 21 support
There was a problem hiding this comment.
LGTM. Will approve if the CI is green.
@mcbarton, does this also test cppyy with LLVM 21?
@vgvassilev, could you take a look at the .patch files in this PR?
| @@ -0,0 +1,20 @@ | |||
| diff --git a/llvm/cmake/modules/CrossCompile.cmake b/llvm/cmake/modules/CrossCompile.cmake | |||
There was a problem hiding this comment.
This patch needs removing now since we no longer need it. The other llvm patch is one we have used for Emscripten builds for both llvm 19 and 20 (it isn't needed for functionality, but helps the xeus-cpp deployment have a clear folder when executing cells). I think we also need to add the exception handling patch to this PR too, so that the deployment will work correctly.
This PR doesn't test cppyy for llvm 21. This is normally done is 2 stages. First we update CppInterOp to a new llvm, and then once that is in, we open up another PR in cppyy to switch the cppyy option to on. We are still not testing cppyy for llvm 20 I believe. |
|
The documentation still needs updating as part of this PR, so that people are aware they can build with llvm 21. This is usually done by updating the release branch cloned for llvm for both the native and emscripten builds in the documentation, and updating the names for the patches applied. |
|
Once we have the ci green, we probably need to remove a few ci jobs, since we are hitting the maximum cache size again. My suggestion is to remove the Ubuntu x86 llvm 18 and osx x86 llvm 18 jobs. This will scrap us just under the maximum I think, and we can continue to support these from the cppyy repos. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
I have opened up an issue in llvm about the failing Windows build for llvm 21 here llvm/llvm-project#158716 |
|
@anutosh491 Has this patch been moved upstream yet https://github.com/compiler-research/CppInterOp/blob/main/patches/llvm/emscripten-clang20-3-enable_exception_handling.patch or do I need to work out how to update this patch to work for the llvm 21 release branch? |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Hey @mcbarton does the patch not sit well with llvm 21 ? If that's the case feel free to update it accordingly if that's possible. I shall revamp my PR on this (now that we have emscripten based tests) and try getting this upstream in a couple of days. |
|
hey @mcbarton here's the updated pr on llvm for the patch : llvm/llvm-project#132670 Hope this helps if we need to keep it for some more time on llvm 21 |
Description
Please include a summary of changes, motivation and context for this PR.
Fixes # (issue)
Type of change
Please tick all options which are relevant.
Testing
Please describe the test(s) that you added and ran to verify your changes.
Checklist