Upgrade CppInterOp to support llvm 20 #491

mcbarton · 2025-01-29T14:04:40Z

Description

Please include a summary of changes, motivation and context for this PR.

Given the llvm 20.x branch got released today I can begin the upgrade to CppInterOp to support llvm 20.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

New feature
Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

I have read the contribution guide recently

codecov · 2025-01-29T15:39:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.16%. Comparing base (721d33b) to head (780f02b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #491   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files           9        9           
  Lines        3655     3655           
=======================================
  Hits         2784     2784           
  Misses        871      871

Files with missing lines	Coverage Δ
lib/Interpreter/Paths.cpp	`55.19% <ø> (ø)`

Files with missing lines	Coverage Δ
lib/Interpreter/Paths.cpp	`55.19% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions

clang-tidy made some suggestions

github-actions · 2025-01-29T19:26:26Z

lib/Interpreter/Paths.cpp

@@ -174,13 +174,15 @@ void CopyIncludePaths(const clang::HeaderSearchOptions& Opts,
      if (!withSystem) continue;
      if (withFlags) incpaths.push_back("-isystem");
      break;
-
+//option doesn't appear to exist for llvm>19. Will revert if I determine otherwise.
+#if CLANG_VERSION_MAJOR < 20


warning: no header providing "CLANG_VERSION_MAJOR" is directly included [misc-include-cleaner]

lib/Interpreter/Paths.cpp:20:

+ #include <clang/Basic/Version.h>

anutosh491 · 2025-02-04T08:10:26Z

I get these warnings while building latest llvm

CMake Deprecation Warning at /Users/anutosh491/work/llvm-project/clang/CMakeLists.txt:444 (message):
  'CLANG_ENABLE_ARCMT' is deprecated as ARCMigrate has been removed from
  Clang.  Please use 'CLANG_ENABLE_OBJC_REWRITER' instead to enable or
  disable the Objective-C rewriter.

Possibly should be fixed in our llvm build too !

mcbarton · 2025-02-04T08:14:14Z

I get these warnings while building latest llvm

CMake Deprecation Warning at /Users/anutosh491/work/llvm-project/clang/CMakeLists.txt:444 (message):
  'CLANG_ENABLE_ARCMT' is deprecated as ARCMigrate has been removed from
  Clang.  Please use 'CLANG_ENABLE_OBJC_REWRITER' instead to enable or
  disable the Objective-C rewriter.

Possibly should be fixed in our llvm build too !

Thanks. Will look into this. Need to see if this option is avilable on older versions of llvm, given the ci covers multiple versions.

This reverts commit ba99ace.

anutosh491 · 2025-03-11T14:10:19Z

Hi,

Just curious as to what work is left here (as llvm 20.0.1 was released last week) ?
And it is there something specific requirement that we only handle "non-wasm" through this PR ?

mcbarton · 2025-03-11T14:50:16Z

Hi,

Just curious as to what work is left here (as llvm 20.0.1 was released last week) ? And it is there something specific requirement that we only handle "non-wasm" through this PR ?

@anutosh491 This PR is simply waiting for review and approval. I wasn't expecting the release last week, so that is why I didn't request a review yet (I thought it was this week). I only just realised it got released today. The reason its only covering non-wasm is because I can't get an Emscripten build of llvm 20 to work. I was planning to ask if you had had any luck compiling.

@vgvassilev can you review this PR?

anutosh491 · 2025-03-11T14:52:49Z

Yeah someone else told me too that getting an emscripten build for llvm is tough. I haven't tried it yet and shall give it an attempt!

Edit: out of curiosity, I guess it makes sense to keep the latest possible plvm versions used for wasm and non-wasm the same. Does it ?

I'll try to get a working wasm build with llvm 20 asap if that's case (I didn't realise it was this tough)

anutosh491 · 2025-03-12T10:44:45Z

Hi @mcbarton

I was trying to get a wasm build for latest upstream llvm this morning and these were the only two things I found concerning.

When you start building the first error expected is this

 │ │ In file included from $SRC_DIR/llvm/lib/Frontend/OpenACC/ACC.cpp:9:
 │ │ $SRC_DIR/build/include/llvm/Frontend/OpenACC/ACC.h.inc:192:1: error: unknown type name 'LLVM_ABI'
 │ │   192 | LLVM_ABI Directive getOpenACCDirectiveKind(llvm::StringRef Str);
 │ │       | ^
 │ │ $SRC_DIR/build/include/llvm/Frontend/OpenACC/ACC.h.inc:192:19: error: expected ';' after top level declarator
 │ │   192 | LLVM_ABI Directive getOpenACCDirectiveKind(llvm::StringRef Str);
 │ │       |                   ^

Can also be seen here through my build on emscripten-forge.

Now this was happening because we weren't defining LLVM_ABI correctly when building against emscripten. If you see llvm/Support/Compiler.h, the condition only checked for the platform __WASM__ . Now Emscripten targets WebAssembly but doesn't imply the platform by default so the check isn't complete to define LLVM_ABI. So I added a patch to address this for now.

There was an error creating the TargetMachine for creating the wasm modules while running clang-repl in the browser. That was fixed by [clang-repl] Fix target creation in Wasm.cpp llvm/llvm-project#130909 (merged)

So we should be good to have a wasm build for llvm.

The build was successful and I also tried it with my pet project

vgvassilev · 2025-03-12T18:15:31Z

unittests/CppInterOp/CUDATest.cpp

@@ -54,7 +57,12 @@ TEST(CUDATest, Sanity) {
  EXPECT_TRUE(Cpp::CreateInterpreter({}, {"--cuda"}));
 }

+#if CLANG_VERSION_MAJOR == 20
+// FIXME: CUDA Tests Broken for llvm 20 release/
+TEST(DISABLED_CUDATest, CUDAH) {


We should probably figure out what's going on here before moving forward.

@vgvassilev This is the error with the CudaTests trying to run with an llvm 20 build https://github.com/compiler-research/CppInterOp/actions/runs/13820931151/job/38668593188?pr=491#step:10:187 . Any ideas about what is going on, or how to fix it?

@anutosh491 @aaronj0 @Vipul-Cariappa any ideas about what is happening to cause the above error message in the native build, or how to debug it?

anutosh491 · 2025-03-18T05:51:59Z

With respect to this

#491 (comment)

Both my PRs required to for building latest llvm against emscripten were merged. I had added one as a patch on emscripten-forge but that's merged too, so hopefully we should be fine after the next release (this one llvm/llvm-project#131578)

So hopefully moving for llvm 20 for the emscripten use case isn't an issue now ;)

mcbarton · 2025-03-18T15:00:59Z

With respect to this

#491 (comment)

Both my PRs required to for building latest llvm against emscripten were merged. I had added one as a patch on emscripten-forge but that's merged too, so hopefully we should be fine after the next release (this one llvm/llvm-project#131578)

So hopefully moving for llvm 20 for the emscripten use case isn't an issue now ;)

@anutosh491 Your cherry pick command in the llvm PR failed (see llvm/llvm-project#131578 (comment) ). You may need to try again.

anutosh491 · 2025-03-19T09:16:29Z

With respect to this
#491 (comment)
Both my PRs required to for building latest llvm against emscripten were merged. I had added one as a patch on emscripten-forge but that's merged too, so hopefully we should be fine after the next release (this one llvm/llvm-project#131578)
So hopefully moving for llvm 20 for the emscripten use case isn't an issue now ;)

@anutosh491 Your cherry pick command in the llvm PR failed (see llvm/llvm-project#131578 (comment) ). You may need to try again.

Hey @vgvassilev could you help us here by applying the required milestone on the PR so that the cherry pick passes ?

anutosh491 · 2025-04-02T11:41:47Z

Hey @mcbarton

I was interested in knowing if you were able to figure out what blocks this PR ?
Also I am not sure a separate PR would be need for llvm 20 for wasm cases ?
If yes, I think I should have all tools to make that happen. Please let me know if that's something I should do but again I am guessing updating both cases together would make more sense !

Asking this cause llvm 20.1.2 was out today (llvm 20 being out quite some time ago) and would be great if we could make the move soon and also probably a release supporting latest llvm too.

mcbarton · 2025-04-02T12:06:59Z

Hey @mcbarton

I was interested in knowing if you were able to figure out what blocks this PR ? Also I am not sure a separate PR would be need for llvm 20 for wasm cases ? If yes, I think I should have all tools to make that happen. Please let me know if that's something I should do but again I am guessing updating both cases together would make more sense !

Asking this cause llvm 20.1.2 was out today (llvm 20 being out quite some time ago) and would be great if we could make the move soon and also probably a release supporting latest llvm too.

Hi @anutosh491 With the changes you've made to llvm I plan to make this PR add jobs for both native and Emscripten llvm 20 cases. The only blocker for this PR is the error mentioned in this comment #491 (comment) . Any ideas what is going on?

anutosh491 · 2025-04-02T12:12:09Z

Any ideas what is going on?

Hmm, can give it a look as I find time.

mcbarton · 2025-04-17T20:59:54Z

While investigating the failing CUDA test locally I noticed the following. If I run clang-repl version 19 and 20 locally on MacOS I get (obviously not expecting to work given MacOS doesn't support Nvidia gpus, but it is interesting to note that version 20 crashes with a segmentation fault with the CUDA flag)

CppInterOp ->clang-repl-mp-19 --cuda
clang-repl: dlopen(libcudart.so, 0x0009): tried: 'libcudart.so' (no such file), '/System/Volumes/Preboot/Cryptexes/OSlibcudart.so' (no such file), '/opt/local/libexec/llvm-19/lib/../lib/libcudart.so' (no such file), '/opt/local/libexec/llvm-19/bin/../lib/libcudart.so' (no such file), '/usr/lib/libcudart.so' (no such file, not in dyld cache), 'libcudart.so' (no such file)
CppInterOp ->clang-repl-mp-20 --cuda
#0 0x0000000111da4fbc llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x150fbc)
#1 0x0000000111da31dc llvm::sys::RunSignalHandlers() (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x14f1dc)
#2 0x0000000111da5628 SignalHandler(int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x151628)
#3 0x000000019b242de4 (/usr/lib/system/libsystem_platform.dylib+0x180482de4)
#4 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0)
#5 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0)
#6 0x0000000107f6bac8 clang::Interpreter::createWithCUDA(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x2173ac8)
#7 0x000000010206f8a8 main (/opt/local/libexec/llvm-20/bin/clang-repl+0x1000038a8)
#8 0x000000019ae8c274 
Segmentation fault: 11

Running clang-repl version 20 with the cuda flag on Ubuntu also has a segmentation fault

I absolutely could be wrong, but this at least to me suggests that fixing the test may require a change in llvm. I'm not well versed on how llvm is handling anything related to cuda. @vgvassilev can you provide me with some insight, or cc someone who may be able to help?

patches/llvm/emscripten-clang20-2-shift-temporary-files-to-tmp-dir.patch

github-actions bot reviewed Jan 29, 2025

View reviewed changes

mcbarton force-pushed the Update-to-llvm-20 branch 4 times, most recently from c8a5a4d to e435205 Compare February 2, 2025 21:22

Upgrade to support llvm 20

391be18

mcbarton force-pushed the Update-to-llvm-20 branch from e435205 to 391be18 Compare February 2, 2025 21:23

mcbarton added 2 commits February 2, 2025 21:26

Update emscripten.yml

9df8277

Merge branch 'main' into Update-to-llvm-20

9c64f35

mcbarton force-pushed the Update-to-llvm-20 branch from f04cc0f to 699c6d7 Compare February 3, 2025 08:29

mcbarton changed the title ~~[wip] Upgrade CppInterOp to support llvm 20.~~ Upgrade CppInterOp to support llvm 20 (non-wasm) Feb 3, 2025

mcbarton marked this pull request as ready for review February 3, 2025 21:23

mcbarton requested a review from vgvassilev February 3, 2025 21:23

Part of upgrade to llvm 20

889f996

mcbarton force-pushed the Update-to-llvm-20 branch from ba50742 to 889f996 Compare February 10, 2025 14:17

mcbarton and others added 5 commits February 20, 2025 15:32

Merge branch 'main' into Update-to-llvm-20

fce2b83

See if CUDA tests still broken llvm 20

ba99ace

Revert "See if CUDA tests still broken llvm 20"

8c6e593

This reverts commit ba99ace.

Merge branch 'main' into Update-to-llvm-20

8a4c0af

Merge branch 'main' into Update-to-llvm-20

a38a66e

vgvassilev reviewed Mar 12, 2025

View reviewed changes

Revert changes to CudaTests to show error message in ci run

641da6a

anutosh491 mentioned this pull request Apr 2, 2025

Frame tests for loading shared objects for the emscripten build #548

Draft

4 tasks

mcbarton added 3 commits April 3, 2025 14:19

Merge branch 'main' into Update-to-llvm-20

a3111e0

Add llvm 20 Emscripten jobs

13c69a5

Remove Windows 19 job

6e1a9ff

mcbarton changed the title ~~Upgrade CppInterOp to support llvm 20 (non-wasm)~~ Upgrade CppInterOp to support llvm 20 Apr 3, 2025

mcbarton added 2 commits April 13, 2025 21:28

Merge branch 'main' into Update-to-llvm-20

bb13b49

Merge branch 'main' into Update-to-llvm-20

8bccb8d

mcbarton added 7 commits April 18, 2025 16:36

Update documentation

37f4742

Update Windows-arm.yml to use llvm 20

780f02b

Merge branch 'main' into Update-to-llvm-20

a2fecb9

Add Windows Emscripten 20 cross compile llvm patch

4bf0c13

Update Emscripten-build-instructions.rst

82744ee

Update Emscripten-build-instructions.md

2f98909

Remove patch which is now in main llvm 20 Windows

271c0aa

anutosh491 reviewed Apr 25, 2025

View reviewed changes

patches/llvm/emscripten-clang20-2-shift-temporary-files-to-tmp-dir.patch Show resolved Hide resolved

Enable Emscripten tests which pass for llvm 20

7e4e4e5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade CppInterOp to support llvm 20 #491

Upgrade CppInterOp to support llvm 20 #491

mcbarton commented Jan 29, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading

github-actions bot left a comment

github-actions bot Jan 29, 2025

anutosh491 commented Feb 4, 2025 •

edited

Loading

mcbarton commented Feb 4, 2025

anutosh491 commented Mar 11, 2025

mcbarton commented Mar 11, 2025

anutosh491 commented Mar 11, 2025 •

edited

Loading

anutosh491 commented Mar 12, 2025 •

edited

Loading

vgvassilev Mar 12, 2025

mcbarton Mar 12, 2025

mcbarton Mar 19, 2025

anutosh491 commented Mar 18, 2025 •

edited

Loading

mcbarton commented Mar 18, 2025

anutosh491 commented Mar 19, 2025

anutosh491 commented Apr 2, 2025

mcbarton commented Apr 2, 2025 •

edited

Loading

anutosh491 commented Apr 2, 2025 •

edited

Loading

mcbarton commented Apr 17, 2025

Upgrade CppInterOp to support llvm 20 #491

Are you sure you want to change the base?

Upgrade CppInterOp to support llvm 20 #491

Conversation

mcbarton commented Jan 29, 2025 • edited Loading

Description

Type of change

Testing

Checklist

codecov bot commented Jan 29, 2025 • edited Loading

Codecov Report

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jan 29, 2025

Choose a reason for hiding this comment

anutosh491 commented Feb 4, 2025 • edited Loading

mcbarton commented Feb 4, 2025

anutosh491 commented Mar 11, 2025

mcbarton commented Mar 11, 2025

anutosh491 commented Mar 11, 2025 • edited Loading

anutosh491 commented Mar 12, 2025 • edited Loading

vgvassilev Mar 12, 2025

Choose a reason for hiding this comment

mcbarton Mar 12, 2025

Choose a reason for hiding this comment

mcbarton Mar 19, 2025

Choose a reason for hiding this comment

anutosh491 commented Mar 18, 2025 • edited Loading

mcbarton commented Mar 18, 2025

anutosh491 commented Mar 19, 2025

anutosh491 commented Apr 2, 2025

mcbarton commented Apr 2, 2025 • edited Loading

anutosh491 commented Apr 2, 2025 • edited Loading

mcbarton commented Apr 17, 2025

mcbarton commented Jan 29, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading

anutosh491 commented Feb 4, 2025 •

edited

Loading

anutosh491 commented Mar 11, 2025 •

edited

Loading

anutosh491 commented Mar 12, 2025 •

edited

Loading

anutosh491 commented Mar 18, 2025 •

edited

Loading

mcbarton commented Apr 2, 2025 •

edited

Loading

anutosh491 commented Apr 2, 2025 •

edited

Loading