Page MenuHomeFreeBSD

lang/rust: restore PORT_LLVM
AbandonedPublic

Authored by evgeniy_khramtsov.org on Oct 26 2021, 2:19 AM.

Details

Reviewers
tobik
Group Reviewers
rust
Contributor Reviewers (ports)
Summary

Allow Rust to build against ports LLVM.

llvm-config in cargo.toml implies linking to LLVM libraries, -Lnative is passed for linking to proceed.

[1]: https://github.com/rust-lang/rust/issues/46437#issuecomment-426696032
[2]: https://github.com/rust-lang/llvm-project/commits/rustc/14.0-2022-06-20

Test Plan
  • Builds & runs: 13.1/amd64, 14 -CURRENT/amd64.
  • www/firefox on 14-CURRENT/amd64 built by lang/rust with LLVM_PORT=on builds and runs fine.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

12.2/amd64 needs -fPIC or -z notext for stage2, update planned.

It'd be absolutely fantastic if Rust would use LLVM from Ports, instead of compiling an own copy. Everyone using Poudriere to build stuff would benefit from it.

This comment was removed by jbeich.

12.2/amd64

Fixed.

llvm-config in config.toml resulted in rustc_codegen_ssa being passed LinkOutputKind::DynamicPicExe, which results in amended "-pie" for LD, failing to link with R_X86_64_32(S) relocations and the rest of non-PIE libraries:

error: can't create dynamic relocation R_X86_64_32 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output

Cosmetic changes (e.g. pass pre-link -no-pie in config.toml instead of extra patch) left. This now proved to work with more common non-PIE base/ports and might be committed, but I want to check if an extra patch can be avoided. I'll see this weekend.

  • Replace extra-patch with REINPLACE_CMD.

pass pre-link -no-pie in config.toml instead of extra patch

Passing '-no-pie' via config.toml affects the rest of rustc (e.g. stage0 build).
Replace '-pie' to '-no-lie' in the only place that breaks build. Note, this can
be removed once base (>=14) and ports start building as PIE by default.

  • Use ports LLD for WASM.

Don't build bundled LLD ("rust-lld", build enabled via config.toml as "lld=true")
during PORT_LLVM+WASM rustc build.

WASM target also hardcodes "rust-lld". While it can be overriden during runtime via
e.g. rustc -C linker=wasm-ld13, it can't be overriden via config.toml target spec.
Replace 'rust-lld' with wasm-ld from ports LLVM. Build tested via:

$ rustc --target wasm32-unknown-unknown -O hello.rs

Inspired by: Gentoo https://bugs.gentoo.org/715348

Last time I asked about this people mentioned this:
https://github.com/rust-lang/rust/issues/49653

Note, I am not a Rust developer, but this is how I understand this:

  1. "--print target-{cpus,features}": prints LLVM target feature(s).
  2. "-C target-{cpu,feature}": passes target feature(s) to LLVM backend.
  3. "#[target_feature]": guards by supported CPU features, e.g.: https://rust-lang.github.io/rfcs/2045-target-feature.html#unconditional-code-generation-target_feature.

"[rust] Add accessors for MCSubtargetInfo CPU and Feature tables":

$ rg 'getFeatureTable'
src/llvm-project/llvm/include/llvm/MC/MCSubtargetInfo.h
236: ArrayRef<SubtargetFeatureKV> getFeatureTable() const {

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
416: const ArrayRef<SubtargetFeatureKV> FeatTable = MCInfo->getFeatureTable();
424: const ArrayRef<SubtargetFeatureKV> FeatTable = MCInfo->getFeatureTable();

PassWrapper.cpp, guarded by #ifdef LLVM_RUSTLLVM e.g.:

extern "C" void LLVMRustGetTargetFeature(LLVMTargetMachineRef TM, size_t Index,

                                       const char** Feature, const char** Desc) {
[...]
const ArrayRef<SubtargetFeatureKV> FeatTable = MCInfo->getFeatureTable();
const SubtargetFeatureKV Feat = FeatTable[Index];
*Feature = Feat.Key;
*Desc = Feat.Desc;

}

$ rg 'LLVMRustGetTargetFeature'

compiler/rustc_codegen_llvm/src/llvm_util.rs
229: let len = unsafe { llvm::LLVMRustGetTargetFeaturesCount(tm) };
235: llvm::LLVMRustGetTargetFeature(tm, i, &mut feature, &mut desc);

compiler/rustc_codegen_llvm/src/llvm/ffi.rs (FFI interface)
2136: pub fn LLVMRustGetTargetFeaturesCount(T: &TargetMachine) -> size_t;
2137: pub fn LLVMRustGetTargetFeature(

llvm_util.rs, e.g.:

llvm_target_features(tm: &llvm::TargetMachine) -> Vec<(&str, &str)>
[...]

llvm::LLVMRustGetTargetFeature(tm, i, &mut feature, &mut desc);
if feature.is_null() || desc.is_null() {
    bug!("LLVM returned a `null` target feature string");
}
let feature = CStr::from_ptr(feature).to_str().unwrap_or_else(|e| {
    bug!("LLVM returned a non-utf8 feature string: {}", e);
});
let desc = CStr::from_ptr(desc).to_str().unwrap_or_else(|e| {
    bug!("LLVM returned a non-utf8 feature string: {}", e);
});

[...]

$ rg 'llvm_target_features'

compiler/rustc_codegen_llvm/src/llvm_util.rs
228:fn llvm_target_features(tm: &llvm::TargetMachine) -> Vec<(&str, &str)> {
252: let mut target_features = llvm_target_features(tm);

llvm_util.rs:

fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) {

let mut target_features = llvm_target_features(tm);

[...]

println!("Features supported by rustc for this target:");
for (feature, desc) in &rustc_target_features {
    println!("    {1:0$} - {2}.", max_feature_len, feature, desc);
}

[...]

The sole user of getFeatureTable() of Rust's LLVM fork is rustc target
feature printer (help). Repeating the same to trace getCPUTable() use
is not necessary because the only getCPUTable() call is guarded
with LLVM_RUSTLLVM.

  1. and 3. is tedious to trace, but likely not needed since Rust's LLVM fork

does not maintain related patches, and target_feature was switched from getFeatureTable
and LLVM_RUSTLLVM on LLVM 6+ [1]. Note, upstream has issues with non-stable rustc/LLVM
mapping [2], but SIMD for amd64 is stabilized [3][4].

[1]: https://github.com/rust-lang/rust/pull/49428
[2]: https://github.com/rust-lang/rust/pull/84004#issue-853651627
[3]: https://github.com/rust-lang/rust/blob/69e1d22ddbc6/compiler/rustc_codegen_ssa/src/target_features.rs#L54
[4]: https://github.com/rust-lang/rust/pull/49664

Also, -C target-cpu=skylake SIMD codegen here:
https://gist.github.com/mmatyas/639fc151dda34bb6dd07ee28120677a5

$ rustc -C target-cpu=skylake --emit asm mat4.rs
$ rg 'vmovss' mat4.s
[...]
vmovss %xmm0, 8(%rsp)
[...]

What I am concerned with:

If LLVM doesn't maintain ABI compatibility per major release (I don't know),
then lang/rust with PORT_LLVM will need a rebuild for each ABI breakage in a
minor release. LLVM soname versioning depends on LLVM_MAJOR (e.g. so.12 and so.13),
so I guess ABI compatibility during a major release is more likely to be than not.

Technical issues aside: Is there a committer who is going to own/support this?

lang/rust/Makefile
68

PORT_LLVM_MAKE_ENV

155

missing blank line

Technical issues aside: Is there a committer who is going to own/support this?

Not me.

Is there a committer who is going to own/support this?

Is being a committer a hard requirement?

I am interested to use this as long as I use FreeBSD, and plan to fix possible
breakage as I can. And I'm not the only one that can do this, FreeBSD is likely a community project.

PORT_LLVM was a non-default option for years before removal, and it wasn't always
QA'ed according to git log --grep='PORT_LLVM'.

Besides, I marked this as "experimental" to avoid implying maintainance burden.

Also, why aren't you interested? PORT_LLVM benefits are:

  • Faster patch testing and bisection.
  • Tier-2/3 machines with no package builders e.g. RISC-V can benefit from faster lang/rust builds, including the initial bring-up work.
  • Binary packages aren't always an option, and PORT_LLVM helps when:
    • poudriere-devel pkg seeding doesn't fetch on dependency mismatch e.g. after ca_root_nss or curl updates when project builders can't catch up for days.
    • The above also applies for port maintainers building ports which dependency graph includes lang/rust, resulting in a lang/rust build for each jail, e.g. every supported release sometimes combined with i386.

One would rather avoid building bundled LLVM when ports LLVM was already built.

I get faster lang/rust build time with PORT_LLVM=on (~36 min. vs ~20 min. with on).
The impact might be more noticeable on slower machines like riscv64 boards.

Just put "unsupported" after the option description. Let's hope somebody supports it once it's in. Some of the interested committers in here can commit it.

This revision is now accepted and ready to land.Nov 8 2021, 8:29 AM

Just put "unsupported" after the option description. Let's hope somebody supports it once it's in. Some of the interested committers in here can commit it.

Unsupported might be understood as "not working". At least, that's what I would thought. Maybe "experimental"?

I mean just use "community supported" then?

Anyway use your own judgement. I'm not insisting on it.

Is there a committer who is going to own/support this?

I am interested to use this as long as I use FreeBSD, and plan to fix possible
breakage as I can. And I'm not the only one that can do this, FreeBSD is likely a community project.

I need to warn that now I have no time for FOSS at all until somewhere in 2022Q2-2022Q3. I stopped
updating beyond security fixes, and I no longer do local QA against unmodified base and ports. If anyone
interested commits this, you likely can't expect help from me until somewhere in 2022Q2-2022Q3.

evgeniy_khramtsov.org edited the summary of this revision. (Show Details)
evgeniy_khramtsov.org edited the test plan for this revision. (Show Details)
  • Rebase.
  • Change LIB_DEPENDS to BUILD_DEPENDS (and RUN_DEPENDS if also WASM=on)

External LLVM is static by default in Rust's LLVM wrapper, e.g.:

[...]
[rustc_llvm 0.0.0] cargo:rustc-link-lib=static=LLVMX86CodeGen
[rustc_llvm 0.0.0] cargo:rustc-link-lib=static=LLVMDemangle CodeGen
[rustc_llvm 0.0.0] cargo:rustc-link-lib=dylib=rt
[...]
[rustc_llvm 0.0.0] cargo:rustc-link-search=native=/usr/local/llvm14/lib
[rustc_llvm 0.0.0] cargo:rerun-if-env-changed=LLVM_LINKER_FLAGS
[rustc_llvm 0.0.0] cargo:rerun-if-env-changed=LLVM_STATIC_STDCPP
[rustc_llvm 0.0.0] cargo:rerun-if-env-changed=LLVM_USE_LIBCXX
[rustc_llvm 0.0.0] cargo:rustc-link-lib=c++
[...]
-l static=llvm-wrapper -l static=LLVMX86TargetMCA -l static=LLVMX86Disassembler -l static=LLVMX86AsmParser -l static=LLVMX86CodeGen
[...]

  • Rename PORT_LLVM to LLVM_PORT. Dynamic linking of external LLVM is possible, and base LLVM with base modification in theory could also be used, so leave room for different names for possible future radio menu, e.g. LLVM_BUNDLED, LLVM_PORT, LLVM_PORT_DYN, LLVM_BASE. Dynamic and base LLVM can reduce LLVM proliferation and have their use for runtime patching without rebuilding (e.g. LD_PRELOAD).
This revision now requires review to proceed.Jul 5 2022, 10:16 AM

Commit message was lost and I have yet to learn to use Phabricator beyond Web frontend:

From: Evgeniy Khramtsov <evgeniy@khramtsov.org>
Date: Tue, 5 Jul 2022 09:32:14 +0000
Subject: [PATCH] lang/rust: allow experimental static linking against
LLVM from ports

Rust bundles vendored LLVM for rarely essential changes,
or features guarded behind LLVM_RUSTLLVM, that may not
be required for a user.

Upstream also provides a way to link against external LLVM,
which can save build time when devel/llvmN was already built.

Provide experimental tunable "LLVM_PORT" to allow building
against LLVM from the FreeBSD ports collection. This tunable
is unsupported and its use is discouraged.

Inspired by:            Gentoo
Reviewed by:            tobik
Differential Revision:  https://reviews.freebsd.org/D32654

---

Intermediate commit messages are not kept even when using arcinator (phab's command line interface), you have to explicitly put them in the comment/summary section

Commit message was lost and I have yet to learn to use Phabricator beyond Web frontend:

From: Evgeniy Khramtsov <evgeniy@khramtsov.org>
Date: Tue, 5 Jul 2022 09:32:14 +0000
Subject: [PATCH] lang/rust: allow experimental static linking against
LLVM from ports

Rust bundles vendored LLVM for rarely essential changes,
or features guarded behind LLVM_RUSTLLVM, that may not
be required for a user.
---

does these statements come from the rust project or is it your personal opinion?

Rust bundles vendored LLVM for rarely essential changes,
or features guarded behind LLVM_RUSTLLVM, that may not
be required for a user.
---

does these statements come from the rust project or is it your personal opinion?

Obviously, not from www.rust-lang.org. Feel free to reword.
I don't see anything essential in Rust's fork https://github.com/rust-lang/llvm-project/commits/rustc/14.0-2022-06-20 to build ports on FreeBSD.

This is Rust's documentation regarding updating and fixing bugs in their LLVM: https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html

Enthusiasm to upstream lost.

In FreeBSD, it seems that the separation between contributors and committers is too strong, as the latter
would prefer to avoid landing a change that could be problematic (even for not that complicated
non-default option that is also used in Gentoo), the burden is on the committer, not the author
of change, so no one wants to land. pkg.FreeBSD.org-centric culture also doesn't help.

...I am interested to use this as long as I use FreeBSD, and plan to fix possible
breakage as I can. And I'm not the only one that can do this, FreeBSD is likely a community project...

I also revoke this as I lost enthusiasm to fire poudriere for nothing, as there is no perspective of upstreaming.

Without QA against baseline vanilla ports, I abandon this revision. Anyone interested is free to commandeer revision,
but don't expect me to maintain it. I did enough work to explain and QA this change, and I have other >100 local changes that may have different fate.

I may reevaluate in future.