Skip to content

Conversation

@arrowd
Copy link
Contributor

@arrowd arrowd commented Apr 7, 2025

The default LLVM version on FreeBSD Ports was switched to 19 a while ago. Current LDC bootstraps gets build against 15, which forces the port to pull in two LLVM versions, 15 and 19, at the same time.

@arrowd arrowd force-pushed the freebsd branch 6 times, most recently from 38ca68d to ea61174 Compare April 7, 2025 13:50
@kinke
Copy link
Member

kinke commented Apr 7, 2025

In #4865, I've tried LLVM 19: b884ed4

Then LLVM 18: 184d7e0

I finally gave up because of the lit-test failures (still available in the CI logs) and reverted to 15.

@arrowd
Copy link
Contributor Author

arrowd commented Apr 7, 2025

Yes, I now bumped into the same test failures. I'll look into them more.

@arrowd arrowd force-pushed the freebsd branch 5 times, most recently from a89857b to fb7aa12 Compare April 7, 2025 18:09
@arrowd
Copy link
Contributor Author

arrowd commented Apr 7, 2025

Ok, this is really a mess, unfortunately. There is so much logic trying to guess proper filepaths and names on different OSes. It is fragile and FreeBSD is just an example of this.

I'm not experienced enough to quickly devise a different approach, so I'm just leaving this PR as it is hoping someone will pick it up.

@arrowd arrowd force-pushed the freebsd branch 4 times, most recently from 90b014e to ba48c64 Compare April 9, 2025 14:38
@arrowd
Copy link
Contributor Author

arrowd commented Apr 9, 2025

All right, now that I made the CI green I noticed that I partly redid what #4665 does.

I think we should first land this PR, as it isn't as invasive as the first one and then try to overhaul the logic for searching the runtime libraries.

- LLVM_MAJOR: 18
- HOST_LDC_VERSION: 1.39.0
- EXTRA_CMAKE_FLAGS: "-DMULTILIB=ON -DRT_SUPPORT_SANITIZERS=ON -DBUILD_LTO_LIBS=ON"
- EXTRA_CMAKE_FLAGS: "-DMULTILIB=ON -DRT_SUPPORT_SANITIZERS=ON -DBUILD_LTO_LIBS=ON -DCOMPILER_RT_LIBDIR_OS=linux"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this DCOMPILER_RT_LIBDIR_OS now needed everywhere? I'm afraid that's unacceptable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could change it to linux here if it's the more common value

ldc/CMakeLists.txt

Lines 826 to 827 in 06b2e42

set(COMPILER_RT_LIBDIR_OS_DEFAULT "x86_64-unknown-linux-gnu")
set(COMPILER_RT_LIBDIR_OS "${COMPILER_RT_LIBDIR_OS_DEFAULT}" CACHE STRING "Non-Mac Posix: OS used as directory name for the compiler-rt source libraries, e.g., 'freebsd'.")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this DCOMPILER_RT_LIBDIR_OS now needed everywhere?

I made it a hard error if the directory does not exist

I'm afraid that's unacceptable.

Why? It serves as a safebelt for a user. Without this it took me a while to figure out why profiling tests are failing on FreeBSD - turns out that profiling functionality is silently disabled if the compiler can't locate the runtime dir properly.

Copy link
Member

@kinke kinke Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could change it to linux here if it's the more common value

Sure, but AFAICT, it has been working fine already - but maybe only accidentally? E.g., looking at the unsupported lit-tests for Cirrus CI job Ubuntu-24.04-multilib-rtSanitizers (edit: which is using the shared distro LLVM 18, so defaulting to LDC_INSTALL_LLVM_RUNTIME_LIBS=OFF), their number is still 32.

Alternatively (not sure what the current 'normal' convention is), we could also default to the default LLVM target triple (derivable from llvm-config --host-target).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this DCOMPILER_RT_LIBDIR_OS now needed everywhere?

I made it a hard error if the directory does not exist

Ah alright, so it really just worked accidentally, and the default value should be linux then.

I'm afraid that's unacceptable.

Why?

Because I thought then every user/maintainer needs to follow suit and set it now.

Without this it took me a while to figure out why profiling tests are failing on FreeBSD - turns out that profiling functionality is silently disabled if the compiler can't locate the runtime dir properly.

Understood. Ideally, the according lit-tests would actually be disabled ('unsupported') if the compiler-rt libs aren't available, e.g.:

if 'profile' in config.enabled_rt_libs:
config.available_features.add('PGO_RT')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah alright, so it really just worked accidentally, and the default value should be linux then.

Oh well, what's required for our LLVM builds is apparently a full triple on Linux, for both x86_64 and aarch64. And that's a pretty standard build, not customizing any compiler-rt paths or the like.

CMakeLists.txt Outdated
set(COMPILER_RT_LIBDIR_OS "${COMPILER_RT_LIBDIR_OS_DEFAULT}" CACHE STRING "Non-Mac Posix: OS used as directory name for the compiler-rt source libraries, e.g., 'freebsd'.")
set(COMPILER_RT_LIBDIR "${COMPILER_RT_LIBDIR}/${COMPILER_RT_LIBDIR_OS}")
if(NOT COMPILER_RT_LIBDIR_OS STREQUAL COMPILER_RT_LIBDIR_OS_DEFAULT)
set(WANT_COMPILER_RT_LIBDIR_CONFIG TRUE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see now, this controls whether the dir makes it to the config file. I guess it doesn't hurt to always add it here in this branch - if the dir exists. When using LDC_INSTALL_LLVM_RUNTIME_LIBS, it's disabled later anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this mainly for linux distros to:

  • encourage adding support for compiler-rt rather then hard-disabling the option to prevent the copies which are undersirable
  • avoid having to sed the cpp source files to allow ldc to find them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Yeah I very much like that in principle, forwarding CMake options to the compiler, to put less burden on the hardcoded compiler magic.]

@arrowd arrowd force-pushed the freebsd branch 2 times, most recently from d1cb4a9 to 16e4e19 Compare April 10, 2025 08:24
@arrowd
Copy link
Contributor Author

arrowd commented Apr 10, 2025

  • Made linux the default.
  • Made the hard error a warning.

It feels like macOS failure isn't my fault?

@kinke
Copy link
Member

kinke commented Apr 10, 2025

It feels like macOS failure isn't my fault?

That's almost certainly #4899, now that GHA bumped their macos-15 image to v15.4. Thx Apple, greatly appreciated!

@kinke
Copy link
Member

kinke commented Apr 10, 2025

Made linux the default.

I guess that still needs investigating - a 'normal' LLVM build (with in-tree compiler-rt sources) seems to default to the full triple. As said above, that's what our LLVM builds require, and the FreeBSD port too. So it looks like the majority (?) of Linux distros went with linux, but that diverges from the LLVM default.

Edit: I guess we could also support both, preferring the full triple and checking if the dir exists; if it doesn't, trying to fall back to linux if that dir exists.

@the-horo
Copy link
Contributor

I guess that still needs investigating - a 'normal' LLVM build (with in-tree compiler-rt sources) seems to default to the full triple. As said above, that's what our LLVM builds require, and the FreeBSD port too. So it looks like the majority (?) of Linux distros went with linux, but that diverges from the LLVM default.

The jist is that when building compiler-rt separately you end up, by default, with /linux and when building it alongside llvm you end up with /<triple>. Relevant llvm cmake code for this is:

https://github.com/llvm/llvm-project/blob/5c8ba28c751bf0e7cb5c3ac9d223acba4cefc739/compiler-rt/cmake/base-config-ix.cmake#L106-L118

https://github.com/llvm/llvm-project/blob/5c8ba28c751bf0e7cb5c3ac9d223acba4cefc739/compiler-rt/cmake/Modules/CompilerRTUtils.cmake#L512-L519

With LLVM_ENABLE_PER_TARGET_RUNTIME_DIR being set at https://github.com/llvm/llvm-project/blob/5c8ba28c751bf0e7cb5c3ac9d223acba4cefc739/llvm/CMakeLists.txt#L959-L965

I guess I should have wrote these in #4665. It would have saved me for re-checking the llvm logic.

@kinke
Copy link
Member

kinke commented Apr 10, 2025

Thx a lot Andrei. - So to me this is clearly the way to go then:

we could also support both, preferring the full triple and checking if the dir exists; if it doesn't, trying to fall back to linux if that dir exists

(Doesn't have to be in this PR, can create one myself.)

@arrowd
Copy link
Contributor Author

arrowd commented Apr 14, 2025

Anything required from me to move this forward?

@kinke
Copy link
Member

kinke commented Apr 14, 2025

At most a rebase after #4665 has landed, then you shouldn't have to deal with compiler-rt here anymore.

As according to earlier tests, it's only LLD that needs libzstd.
Copy link
Member

@kinke kinke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kinke kinke merged commit 87ae838 into ldc-developers:master Apr 16, 2025
20 checks passed
@arrowd
Copy link
Contributor Author

arrowd commented Apr 16, 2025

Am I understand it right that the FreeBSD binaries of the next LDC release will be linked to LLVM 19?

@kinke
Copy link
Member

kinke commented Apr 16, 2025

Yeah, beta1 coming up this week. - If you wanna make the bootstrapping of the LDC FreeBSD port more independent, I suggest using GDC via its gdmd wrapper as host compiler.

@arrowd
Copy link
Contributor Author

arrowd commented Apr 16, 2025

It was the other way around - we switched from gdmd to binary LDC bootstraps because GDC 12+ also needs a bootstrap compiler. So using LDC bootstraps is lesser of two evils and we're actually depending on you guys.

@kinke
Copy link
Member

kinke commented Apr 16, 2025

Mhm. The official FreeBSD package from GitHub is the only prebuilt package which doesn't link LLVM statically. I assumed the majority of those users are fine with a slim package, having the LLVM port installed already or quickly installing it if needed.

If static linking is feasible with the LLVM port, and it would make things easier for you guys, we could IMO switch.

@arrowd
Copy link
Contributor Author

arrowd commented Apr 16, 2025

As long as bootstrap is linked to the same LLVM version that we're building against, the static linking wouldn't change anything for us. So I don't think you should bother with static linking unless it is actually easier for you.

@arrowd arrowd deleted the freebsd branch April 16, 2025 15:50
@kinke
Copy link
Member

kinke commented Apr 16, 2025

For us it wouldn't be any easier/harder. As long as you won't be needing bleeding-edge LLVM version support, you should be good then - as you probably know, it can take a bit of time until a new LLVM version is supported here in master, and some more time until a release with prebuilt packages is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants