meson icon indicating copy to clipboard operation
meson copied to clipboard

cmake: allow dynamic linking with LLVM

Open rilian-la-te opened this issue 1 year ago • 17 comments

This patch allows to link dynamically with LLVM when CMake requested.

For now, only linking with libLLVM.so is supported (not linking to individual libraries). There are certainly some bugs.

Fixes #10592

rilian-la-te avatar Jul 12 '22 14:07 rilian-la-te

Codecov Report

Merging #10595 (8b018e2) into master (01275fb) will increase coverage by 0.01%. The diff coverage is 90.47%.

@@            Coverage Diff             @@
##           master   #10595      +/-   ##
==========================================
+ Coverage   68.85%   68.86%   +0.01%     
==========================================
  Files         414      414              
  Lines       90284    90314      +30     
  Branches    20709    20721      +12     
==========================================
+ Hits        62163    62194      +31     
  Misses      23434    23434              
+ Partials     4687     4686       -1     
Impacted Files Coverage Δ
mesonbuild/dependencies/dev.py 56.73% <90.47%> (+1.61%) :arrow_up:
dependencies/dev.py 60.57% <0.00%> (+1.72%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 12 '22 14:07 codecov[bot]

If you want to test this locally ./run_single_test.py "test cases/framework/15 llvm" -- -Dmethod=cmake -Dlink-static=(true|false) will do what you want.

dcbaker avatar Jul 12 '22 17:07 dcbaker

Also, I don't want to support linking to individual .sos, unless upstream LLVM has changed their policy and support that (last I knew they didn't support that, and I don't want to take on supporting something they don't)

dcbaker avatar Jul 12 '22 17:07 dcbaker

@dcbaker this PR does not want to support to linking to individual .so files. This PR want to support linking to libLLVM.so, and check if requested components is present in libLLVM.so. (if dynamic is requested, obliviously)

No individual component linking should be involved.

rilian-la-te avatar Jul 12 '22 17:07 rilian-la-te

Right, sorry, you had mentiond that wasn't supported by your PR, and I just wanted to clarify we don't want that anyway :)

dcbaker avatar Jul 12 '22 17:07 dcbaker

@dcbaker do you know how to disable single test on Ubuntu Bionic? It have LLVM 6.0, which cannot find dynamically linked modules by CMake dep (LLVM limitation).

rilian-la-te avatar Jul 12 '22 22:07 rilian-la-te

You’ll probably need to add a check inside the meson.build file that checks the llvm version and calls error() with MESON_SKIP_TEST message. There’s other tests that do this you can look at. You may need to do something like just call dependency(llvm) to get the version info

dcbaker avatar Jul 12 '22 22:07 dcbaker

@dcbaker I want to remove UNEXPSKIP on Ubuntu Bionic and make skip expected via test.json, but I cannot find how to do it properly and not skip tests on other targets.

rilian-la-te avatar Jul 12 '22 22:07 rilian-la-te

I don't think we have the tools in the harness to describe "link-shared + cmake will skip on bionic", only that "cmake skips on bionic" or "link-shared skps on bionic"

dcbaker avatar Jul 15 '22 19:07 dcbaker

@dcbaker and what should I do to pass a CI? I tried to install llvm-7 on bionic, but images cannot be created. And for llvm < 7 we cannot support dynamic linking with LLVM < 7 with cmake without random bugs.

rilian-la-te avatar Jul 16 '22 12:07 rilian-la-te

I started looking at the testing code, and I I can add support for skipping specific ci configurations, I just don’t get it done Friday

dcbaker avatar Jul 16 '22 17:07 dcbaker

@dcbaker I think it is better just to install llvm-7 to bionic CI image.

rilian-la-te avatar Jul 17 '22 21:07 rilian-la-te

@mensinda is my approach to make CMake with LLVM working is correct?

rilian-la-te avatar Jul 18 '22 15:07 rilian-la-te

I support this. Running llvm-config really does not work when cross-compiling.

kanavin avatar Nov 14 '22 16:11 kanavin

since bionic has been dropped from our CI this may Just Work™ if you rebase it.

dcbaker avatar Nov 14 '22 18:11 dcbaker

@dcbaker I rebased it, but bionic is still here, and ArchLinux MPI unrelated failure too.

rilian-la-te avatar Nov 15 '22 07:11 rilian-la-te

@dcbaker can you remove Bionic from meson CI at last or just update LLVM in its image?

rilian-la-te avatar Nov 15 '22 08:11 rilian-la-te

@mensinda will this be addresed? I think dynamic LLVM is cruical for cross-compilation. For example, Mesa can greatly benefit from it)

rilian-la-te avatar Jan 24 '23 11:01 rilian-la-te

@rilian-la-te Can you rebase this, and I’ll have a look at reviewing it again?

dcbaker avatar Jan 24 '23 15:01 dcbaker

@dcbaker already rebased. One failing CI check - skipping (UNEXPSKIP) LLVM tests on Bionic (because cmake dynamic llvm can be used only on LLVM 7+ and Bionic have 6.0).

rilian-la-te avatar Jan 24 '23 15:01 rilian-la-te

Ohhh that’s right. I started working on some infrastructure changes for dealing with that ash’s never finished…

dcbaker avatar Jan 24 '23 16:01 dcbaker

@dcbaker can you review this changes? It seems than this PR was forgotten, but it can be very useful for cross-compilation with distros' builtin LLVM (I tested it in Debian - it is usable for cross-compiling there with their multiarch approach).

rilian-la-te avatar Jan 24 '23 16:01 rilian-la-te

I think we're currently unable to mark a single cell in a matrix as skip_on_jobname. Only an entire row or column.

eli-schwartz avatar Jan 24 '23 21:01 eli-schwartz

@mensinda can it be upstreamed before CI fix will landed?

Most simple fix - just install llvm 7 or 8 on Bionic instead of 6. I tried, but was unsuccessful in generating a new image.

rilian-la-te avatar Jan 24 '23 21:01 rilian-la-te

Ubuntu Bionic (18.04) goes out of support in about two months. I would just drop testing with it altogether, or replace with a more recent release.

kanavin avatar Jan 25 '23 07:01 kanavin

Since bionic is EOL soon anyway, I agree with @kanavin that we could just skip this test on bionic (if installing llvm-7 does not work out).

I have tested it locally with the docker image and this should work:

diff --git a/test cases/frameworks/15 llvm/test.json b/test cases/frameworks/15 llvm/test.json
index b39844d73..e88419e84 100644
--- a/test cases/frameworks/15 llvm/test.json
+++ b/test cases/frameworks/15 llvm/test.json
@@ -12,5 +12,8 @@
       ]
     }
   },
-  "skip_on_jobname": ["azure", "cygwin"]
+  "skip_on_jobname": ["azure", "cygwin"],
+  "tools": {
+    "llvm-config": ">=7.0"
+  }
 }

mensinda avatar Jan 28 '23 14:01 mensinda

@mensinda it seems than installing llvm-7 works. I think it can be merged after CI passes (Strangely, it will not pass, because it uses old image).

rilian-la-te avatar Jan 28 '23 14:01 rilian-la-te

@mensinda do I need a separate PR for updating a CI image?

rilian-la-te avatar Jan 28 '23 14:01 rilian-la-te

Looking at the history, the sheduled CI image builder has been failing for some time now: https://github.com/mesonbuild/meson/actions/runs/3977183901

Ideally a separate PR would be better to keep things separated.

mensinda avatar Jan 28 '23 14:01 mensinda

@mensinda is Meson capable to generate CMake toolchain file from cross file?

It seems than CMake version is only way how to skip LLVM test from running on bionic. It is weird but most likely work.

rilian-la-te avatar Jan 28 '23 15:01 rilian-la-te