Add support for LLVM 15 by thedataking · Pull Request #677 · immunant/c2rust

thedataking · 2022-09-23T08:13:07Z

clang::StringLiteral::StringKind::Ascii was renamed to clang::StringLiteral::StringKind::Ordinary. Add release keys and update build_translator.py. Closes #676.

chrysn · 2022-09-28T10:08:41Z

Unrelated trouble with earlier Debian versions that's not this PR's fault

Testers on not-perfectly-recent Debian beware that the change to LLVM 15 that requires this (the StringLiteral::StringKind renaming) came in quite late in the LLVM release process. Users of Debian experimental will find that the new version will not build on some versions of llvm-15-dev, but that the current master does work. This is no fault of this PR, but of the Debian package claiming something to be -15 when it doesn't have all of -15's changes (but to be fair, it is an experimental pre-release, and needs to be opted in to).

With the very latest experimental release (llvm-15 1:15.0.1-1~exp2 from Debian, including mlir-15-tools, and a bunch of packages the build process will conveniently ask for), this passes compilation, but fails at link time:

  = note: /usr/bin/ld: /usr/lib/llvm-15/lib/libclangSema.a(SemaRISCVVectorLookup.cpp.o): in function `clang::CreateRISCVIntrinsicManager(clang::Sema&)':
          (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1b8): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'
          /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1f9): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'

I think this would be resolved by not only linking in libclangSema.a but also libclangSupport.a, but I don't know where that's stated (yet?).

chrysn · 2022-09-28T10:17:40Z

... and I have a patch:

diff --git a/c2rust-ast-exporter/build.rs b/c2rust-ast-exporter/build.rs
index 4dd17e88..22581b66 100644
--- a/c2rust-ast-exporter/build.rs
+++ b/c2rust-ast-exporter/build.rs
@@ -172,6 +172,7 @@ fn build_native(llvm_info: &LLVMInfo) {
             "clangParse",
             "clangSerialization",
             "clangSema",
+            "clangSupport",
             "clangEdit",
             "clangAnalysis",
             "clangDriver",

(Can't test whether that needs version sensing to decide whether or not to add that, as I don't want to shuffle around all my local packages once more -- I trust CI would give clear indications here)

[edit: With that patch, this branch not only builds but passes all my application tests so far, except where there are other open PRs]

thedataking · 2022-09-30T10:25:56Z

Thanks @chrysn, I've incorporated your suggestion. Let me know if you hit any other issues.

c2rust-ast-exporter/build.rs

clang::StringLiteral::StringKind::Ascii was renamed to clang::StringLiteral::StringKind::Ordinary. Add release keys and update build_translator.py. Closes #676.

The static libraries for the homebrew version of clang/LLVM 15 are build with a feature (opaque pointers) not understood by earlier versions of clang/LLVM. Switching to dynamic linking on MacOS sidesteps the issue. We still link against static libraries if this is requested via the `llvm-static` feature flag.

Disable support for iOS in compiler-rt. Don't pass --wildcard to tar since the version shipping with MacOS doesn't support it.

kkysen · 2022-10-05T22:36:16Z

scripts/common.py

+        def llvm_major_ver_ge(test_ver) -> bool:
+            (major, _, _) = self.LLVM_VER.split(".")
+            return int(major) >= test_ver


I think llvm_major_ver() -> int would be better, and do the >= at call site. It's much easier to read and more flexible, and test_ver isn't otherwise used in llvm_major_ver_ge.

kkysen · 2022-10-05T22:36:43Z

c2rust-ast-exporter/build.rs

+        if llvm_info.llvm_major_version >= 15 {
+            // insert after clangSema
+            let sema_pos = clang_libs.iter().position(|&r| r == "clangSema").unwrap();
+            clang_libs.insert(sema_pos + 1, "clangSupport");
+        }


I would put this inline as something like if llvm_info.llvm_major_version >= 15 { "clangSupport" } else { "" } and then filter out "". It's much cleaner, easier to understand the order of libraries, and more extensible if we need to add conditional libraries where order matters.

kkysen · 2022-10-05T22:36:49Z

c2rust-ast-exporter/build.rs

+        if cfg!(feature = "llvm-static") {
+            false
+        } else {
+            true


Do we not need to do a similar libclang_path.exists() check for macOS like we do for linux?

thedataking marked this pull request as draft September 23, 2022 09:43

chrysn mentioned this pull request Sep 26, 2022

Update crates.io release #690

Closed

kkysen mentioned this pull request Sep 28, 2022

Tolerate RISC-V vector types #693

Merged

thedataking force-pushed the feature/llvm-15.0.0 branch from 60e9104 to cadf395 Compare September 30, 2022 10:24

thedataking force-pushed the feature/llvm-15.0.0 branch 2 times, most recently from 5b7ac17 to 95b62c3 Compare October 3, 2022 23:26

kkysen reviewed Oct 3, 2022

View reviewed changes

c2rust-ast-exporter/build.rs Show resolved Hide resolved

thedataking added 3 commits October 3, 2022 18:35

Add support for LLVM 15

8d2fbc8

clang::StringLiteral::StringKind::Ascii was renamed to clang::StringLiteral::StringKind::Ordinary. Add release keys and update build_translator.py. Closes #676.

Add support for LLVM 15 in Python build scripts

1773890

Conditionally link against LLVM 15 libraries

c3bbeee

thedataking force-pushed the feature/llvm-15.0.0 branch from 96e50cb to eae47ac Compare October 4, 2022 02:17

thedataking added 2 commits October 3, 2022 20:09

Fix developer build with LLVM 15 on MacOS

b2b8b13

Disable support for iOS in compiler-rt. Don't pass --wildcard to tar since the version shipping with MacOS doesn't support it.

thedataking force-pushed the feature/llvm-15.0.0 branch from eae47ac to b2b8b13 Compare October 4, 2022 03:11

thedataking marked this pull request as ready for review October 4, 2022 06:14

thedataking merged commit 8014cd4 into master Oct 5, 2022

kkysen reviewed Oct 5, 2022

View reviewed changes

This was referenced Nov 1, 2022

failed to run custom build command for c2rust-ast-exporter v0.16.0 #713

Closed

c2rust not installing on Ubuntu 22.10 #717

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for LLVM 15#677

Add support for LLVM 15#677
thedataking merged 5 commits intomasterfrom
feature/llvm-15.0.0

thedataking commented Sep 23, 2022 •

edited by kkysen

Loading

Uh oh!

chrysn commented Sep 28, 2022

Uh oh!

chrysn commented Sep 28, 2022 •

edited

Loading

Uh oh!

thedataking commented Sep 30, 2022

Uh oh!

Uh oh!

kkysen Oct 5, 2022

Uh oh!

kkysen Oct 5, 2022

Uh oh!

kkysen Oct 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thedataking commented Sep 23, 2022 • edited by kkysen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrysn commented Sep 28, 2022

Uh oh!

chrysn commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thedataking commented Sep 30, 2022

Uh oh!

Uh oh!

kkysen Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

kkysen Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

kkysen Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thedataking commented Sep 23, 2022 •

edited by kkysen

Loading

chrysn commented Sep 28, 2022 •

edited

Loading