This is an archive of the discontinued LLVM Phabricator instance.

Clang/MIPS: Use correct cross env if both r6 and legacy are installed
Needs ReviewPublic

Authored by wzssyqa on Aug 16 2023, 10:33 AM.

Details

Summary

On Debian, if we have both r6 and legacy cross env are installed,
clang will always try to use the legacy one. For example, if we have
both gcc-mipsel-linux-gnu and gcc-mipsisa32r6el-linux-gnu is installed,
clang -target mipsisa32r6el-linux-gnu will use mipsel-linux-gnu's header
and libraries.

It is due to in MIPS*Triples variables, the leagcies are always list before r6 ones.

Let's split them to _r6 and _legacy lists.

At the same time, we also set the internal triple to r6 ones, if -march=mips32r6/mips64r6
is passed.

Diff Detail

Event Timeline

wzssyqa created this revision.Aug 16 2023, 10:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 10:33 AM
wzssyqa requested review of this revision.Aug 16 2023, 10:33 AM
wzssyqa updated this revision to Diff 550957.Aug 16 2023, 7:13 PM

Swap MIPSTriples_legacy and MIPSTriples_r6.

See the commend I added in a07727199db0525e9d2df41e466a2a1611b3c8e1. " Please don't add more elements to *Triples".

These variables are workarounds so that, for example, --target=aarch64-unknown-linux-gnu can detect a aarch64-linux-gnu GCC installation (Debian target triples omit "vendor").

If mips*-img-linux-gnu cause trouble, we should just remove them (D4436). clang --target=mips-img-linux-gnu will still be able to pick up mips-img-linux-gnu GCC installations.

See the commend I added in a07727199db0525e9d2df41e466a2a1611b3c8e1. " Please don't add more elements to *Triples".

These variables are workarounds so that, for example, --target=aarch64-unknown-linux-gnu can detect a aarch64-linux-gnu GCC installation (Debian target triples omit "vendor").

Is it a good idea to add some code to add aarch64-linux-gnu first to TripleAliases if the vendor of original triple is 'unknown'?

If mips*-img-linux-gnu cause trouble, we should just remove them (D4436). clang --target=mips-img-linux-gnu will still be able to pick up mips-img-linux-gnu GCC installations.

Yes. We should remove mips*-img-* here.

See the commend I added in a07727199db0525e9d2df41e466a2a1611b3c8e1. " Please don't add more elements to *Triples".

These variables are workarounds so that, for example, --target=aarch64-unknown-linux-gnu can detect a aarch64-linux-gnu GCC installation (Debian target triples omit "vendor").

Is it a good idea to add some code to add aarch64-linux-gnu first to TripleAliases if the vendor of original triple is 'unknown'?

AArch64Triples[] should not contain aarch64-linux-gnu. Unfortunately, Debian patches GCC so that their GCC packages are installed to lib/gcc{,-cross}/aarch64-linux-gnu instead of lib/gcc{,-cross}/aarch64-{pc,unknown}-linux-gnu, so AArch64Triples[] has to contain aarch64-linux-gnu to support LLVM_DEFAULT_TARGET_TRIPLE (always normalized with "vendor") aarch64-unknown-linux-gnu.

wzssyqa added a comment.EditedAug 17 2023, 12:22 AM

AArch64Triples[] should not contain aarch64-linux-gnu. Unfortunately, Debian patches GCC so that their GCC packages are installed to lib/gcc{,-cross}/aarch64-linux-gnu instead of lib/gcc{,-cross}/aarch64-{pc,unknown}-linux-gnu, so AArch64Triples[] has to contain aarch64-linux-gnu to support LLVM_DEFAULT_TARGET_TRIPLE (always normalized with "vendor") aarch64-unknown-linux-gnu.

Yes. I know about it.

I mean, can we add some code like:

if (Triple.getVendor() == unknown) {
    TripleTMP = drop_vendor(Triple);
    TripleAliases.append(TripleTMP);
}
MaskRay added subscribers: aaronpuchert, tstellar.EditedAug 17 2023, 8:54 AM

AArch64Triples[] should not contain aarch64-linux-gnu. Unfortunately, Debian patches GCC so that their GCC packages are installed to lib/gcc{,-cross}/aarch64-linux-gnu instead of lib/gcc{,-cross}/aarch64-{pc,unknown}-linux-gnu, so AArch64Triples[] has to contain aarch64-linux-gnu to support LLVM_DEFAULT_TARGET_TRIPLE (always normalized with "vendor") aarch64-unknown-linux-gnu.

Yes. I know about it.

I mean, can we add some code like:

if (Triple.getVendor() == unknown) {
    TripleTMP = drop_vendor(Triple);
    TripleAliases.append(TripleTMP);
}

If this helps removing elements from *Triples* arrays, I will be in favor.

We still have *-redhat-* and *-suse-* elements, perhaps @tstellar and @aaronpuchert can help.

(Note: for newer architectures riscv64-{redhat,suse}-linux, we've reverted attempts trying to add such elements: D74399. ).

For Debian, I wish that they can at least create some symlinks like lib/gcc/aarch64-unknown-linux-gnu -> aarch64-linux-gnu, so that Clang Driver no longer needs to do the hacks, at least for newer Debian versions.

If this helps removing elements from *Triples* arrays, I will be in favor.

Let's talk about here: https://reviews.llvm.org/D158183
Ohh, it meet some fails on riscv...
I will try to fix it.

We still have *-redhat-* and *-suse-* elements, perhaps @tstellar and @aaronpuchert can help.

(Note: for newer architectures riscv64-{redhat,suse}-linux, we've reverted attempts trying to add such elements: D74399. ).

For Debian, I wish that they can at least create some symlinks like lib/gcc/aarch64-unknown-linux-gnu -> aarch64-linux-gnu, so that Clang Driver no longer needs to do the hacks, at least for newer Debian versions.

wzssyqa updated this revision to Diff 551175.Aug 17 2023, 9:34 AM

We still have *-redhat-* and *-suse-* elements, perhaps @tstellar and @aaronpuchert can help.

For that we should probably fix config.guess or whatever replaces it (D109837). My current favorite is to have the triple match the GCC triple, because then it is easy to find the GCC installation, and add -gnu "implicitly" like D110900 does it.

(Note: for newer architectures riscv64-{redhat,suse}-linux, we've reverted attempts trying to add such elements: D74399.)

This has nothing to do with an architecture being new or old. SUSE consistently uses ${ARCH}-suse-linux (except for ARM 6/7 because of ABI shenanigans). Since we don't list riscv64-suse-linux, one has to configure with -DLLVM_HOST_TRIPLE=riscv64-suse-linux (or at least LLVM_DEFAULT_TRIPLE). The package that we ship should do the right thing, but if someone were to check out the source tree and build it, they'd have a compiler with a broken default triple.

For Debian, I wish that they can at least create some symlinks like lib/gcc/aarch64-unknown-linux-gnu -> aarch64-linux-gnu, so that Clang Driver no longer needs to do the hacks, at least for newer Debian versions.

From Debian's point of view, unknown is probably just a weird artifact in how we organize triples.

If https://reviews.llvm.org/D158509 is OK, this patch won't be needed.
In fact, we can remove MIPS*LTriples[], I guess.