Enable lld explicitly for hexagon-unknown-linux-musl

[androm3da]

Tested with the https://github.com/quic/toolchain_for_hexagon/releases/tag/v17.0.0-rc3 release of the toolchain.

[rustbot]

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

These commits modify compiler targets.
(See the Target Tier Policy.)

[petrochenkov]

@androm3da
We got distracted by linker flavor details above, but could you give a bit more high level motivation behind the change?
Is the intention to use an arbitrary clang installed on the system, or some specific clang shipped with the toolchain you linked?

The second case is quite common for various targets, and targets typically don't put clang into the target spec in this case, people just specify it in -Clinker=clang or corresponding cargo options, or point cc to clang.

In that second case the only change required from the hexagon target to make it clang-compatible would be removing the base.linker_flavor = ... line entirely.
@rustbot author

[androm3da]

Is the intention to use an arbitrary clang installed on the system, or some specific clang shipped with the toolchain you linked?

Typical use case would probably be for users to download the cross toolchain and use that as the linker frontend. Although at some point it might be nice to make it work well for the native toolchain use case, too.

In that second case the only change required from the hexagon target to make it clang-compatible would be removing the base.linker_flavor = ... line entirely.

Okay, I can make this change.

[androm3da]

Is the intention to use an arbitrary clang installed on the system, or some specific clang shipped with the toolchain you linked?

Typical use case would probably be for users to download the cross toolchain and use that as the linker frontend. Although at some point it might be nice to make it work well for the native toolchain use case, too.

In that second case the only change required from the hexagon target to make it clang-compatible would be removing the base.linker_flavor = ... line entirely.

Okay, I can make this change.

Some more detail: I made this change because I got link errors from symbols that could be resolved to clang_rt.builtins-hexagon and figured that I could use clang as the linker driver and by doing so would get libraries like this one. For targets that don't use clang as the driver -- what do they do to resolve compiler-emitted calls to builtins like the ones in compiler-rt or libgcc?

[petrochenkov]

@androm3da

For targets that don't use clang as the driver -- what do they do to resolve compiler-emitted calls to builtins like the ones in compiler-rt or libgcc?

For all builtin calls emitted by the compiler there should be a definition in https://github.com/rust-lang/compiler-builtins (which is a Rust port/equivalent of compiler-rt).
If some architecture-specific symbols are missing there you may want to add them to that crate.

(That's why most targets use no_default_libraries: true and do not rely on C/C++ compiler pulling additional libraries. If something is not linked from compiler-builtins, it will be linked from libc or libpanic_unwind on demand.)

[androm3da]

@androm3da

For targets that don't use clang as the driver -- what do they do to resolve compiler-emitted calls to builtins like the ones in compiler-rt or libgcc?

For all builtin calls emitted by the compiler there should be a definition in https://github.com/rust-lang/compiler-builtins (which is a Rust port/equivalent of compiler-rt). If some architecture-specific symbols are missing there you may want to add them to that crate.

Gee -- most if not all of the target-specific builtins that I am thinking of for hexagon architecture are implemented in assembly. I'm assuming that wouldn't need to change - so we can just contribute the same implementation to rust-lang/compiler-builtins that's in compiler-rt already?

(That's why most targets use no_default_libraries: true and do not rely on C/C++ compiler pulling additional libraries. If something is not linked from compiler-builtins, it will be linked from libc or libpanic_unwind on demand.)

Okay, so no_default_libraries: true would cause the rust/llvm compiler to not emit calls to the builtins like clang does with -fno-builtin? Or does it still emit those but change the libraries that we expect to resolve those calls to?

[petrochenkov]

Gee -- most if not all of the target-specific builtins that I am thinking of for hexagon architecture are implemented in assembly. I'm assuming that wouldn't need to change - so we can just contribute the same implementation to rust-lang/compiler-builtins that's in compiler-rt already?

AFAIK, compiler-builtins pull some assembly (and C?) code from compiler-rt, so hexagon support may amount so some build system tweaks.

Okay, so no_default_libraries: true would cause the rust/llvm compiler to not emit calls to the builtins like clang does with -fno-builtin? Or does it still emit those but change the libraries that we expect to resolve those calls to?

no_default_libraries: true asks linker (clang in this case) to not pull its default libraries, like libc or libgcc or compiler-rt.
So it's an equivalent of passing -nodefaultlibs (which IS passed by rustc by default).
So calls to builtins will still be emitted and will be satisfied by compiler-builtins.

[androm3da]

compiler-builtins pull some assembly (and C?) code from compiler-rt, so hexagon support may amount so some build system tweaks.

@petrochenkov I think I understand it now -- I made some local change like this and it appears to build a libcompiler-rt.a with the expected symbols. So I will try to test this out and if it works well make a PR for that repo. Thanks.

diff --git a/build.rs b/build.rs
index 0486116..ce36d0b 100644
--- a/build.rs
+++ b/build.rs
@@ -282,24 +282,55 @@ mod c {
             ("__negdi2", "negdi2.c"),
             ("__negvdi2", "negvdi2.c"),
             ("__negvsi2", "negvsi2.c"),
             ("__paritydi2", "paritydi2.c"),
             ("__paritysi2", "paritysi2.c"),
             ("__popcountdi2", "popcountdi2.c"),
             ("__popcountsi2", "popcountsi2.c"),
             ("__subvdi3", "subvdi3.c"),
             ("__subvsi3", "subvsi3.c"),
             ("__ucmpdi2", "ucmpdi2.c"),
         ]);
 
+        if target_arch == "hexagon" {
+            sources.extend(&[
+                ("__hexagon_moddi3", "hexagon/moddi3.S"),
+                ("__hexagon_umodsi3", "hexagon/umodsi3.S"),
+                ("__hexagon_udivsi3", "hexagon/udivsi3.S"),
+                ("__hexagon_udivmodsi4", "hexagon/udivmodsi4.S"),
+                ("__hexagon_udivmoddi4", "hexagon/udivmoddi4.S"),
+                ("__hexagon_udivdi3", "hexagon/udivdi3.S"),
+                ("__hexagon_modsi3", "hexagon/modsi3.S"),
+                ("__hexagon_umoddi3", "hexagon/umoddi3.S"),
+                ("hexagon_memcpy_forward_vp4cp4n2", "hexagon/memcpy_forward_vp4cp4n2.S"),
+                ("__hexagon_memcpy_likely_aligned_min32bytes_mult8bytes", "hexagon/memcpy_likely_aligned.S"),
+                ("fast2_dadd_asm", "hexagon/fastmath2_dlib_asm.S"),
+                ("fast2_ldadd_asm", "hexagon/fastmath2_ldlib_asm.S"),
+                ("__hexagon_divsi3", "hexagon/divsi3.S"),
+                ("__hexagon_divdi3", "hexagon/divdi3.S"),
+                ("__hexagon_sqrtf", "hexagon/sfsqrt_opt.S"),
+                ("__hexagon_divsf3", "hexagon/sfdiv_opt.S"),
+                ("__hexagon_sqrtdf2", "hexagon/dfsqrt.S"),
+                ("__hexagon_sqrt", "hexagon/dfsqrt.S"),
+                ("__hexagon_mindf3", "hexagon/dfminmax.S"),
+                ("__hexagon_maxdf3", "hexagon/dfminmax.S"),
+                ("__hexagon_muldf3", "hexagon/dfmul.S"),
+                ("__hexagon_divdf3", "hexagon/dfdiv.S"),
+                ("__hexagon_adddf3", "hexagon/dfaddsub.S"),
+                ("__hexagon_subdf3", "hexagon/dfaddsub.S"),
+                ("__hexagon_fmadf4", "hexagon/dffma.S"),
+                ("__hexagon_fmadf5", "hexagon/dffma.S"),
+            ]);
+        }
+
         if consider_float_intrinsics {
             sources.extend(&[
                 ("__divdc3", "divdc3.c"),
                 ("__divsc3", "divsc3.c"),
                 ("__divxc3", "divxc3.c"),
                 ("__extendhfsf2", "extendhfsf2.c"),
                 ("__muldc3", "muldc3.c"),
                 ("__mulsc3", "mulsc3.c"),
                 ("__mulxc3", "mulxc3.c"),
                 ("__negdf2", "negdf2.c"),
                 ("__negsf2", "negsf2.c"),
                 ("__powixf2", "powixf2.c"),

[bors]

☔ The latest upstream changes (presumably #117716) made this pull request unmergeable. Please resolve the merge conflicts.

[JohnCSimon]

@androm3da

ping from triage - can you post your status on this PR? This PR has not received an update in a few months.

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

Or if you're not going to continue, please close it. Thank you!

[androm3da]

Thanks for the reminder @JohnCSimon - I was waiting to see a stable release with my changes but that's not necessary.

This PR is obsolete since d5c39d0 landed. The intent was to fix link errors due to unresolved symbols for compiler-emitted dependencies that would be resolved to libclang_rt-builtins in a clang/llvm toolchain. But this PR to change the driver was not the right approach, adding the symbols to libcompiler_builtins was.