This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Remove unneeded *-suse-* triples
Needs ReviewPublic

Authored by MaskRay on Sep 13 2021, 3:35 PM.

Details

Summary

CollectLibDirsAndTriples has a number of *Triples elements (hack) so that
--target=aarch64-linux-gnu can auto detect lib/aarch64-suse-linux. IMO
trouble from the magic behavior outweighs its convenience.

For most users (who don't specify --target), the CMake
LLVM_DEFAULT_TARGET_TRIPLE variable should just point to the correct triple
(e.g. aarch64-suse-linux), and Clang driver will find the right
lib/aarch64-suse-linux directory without a hard coded element in AArch64LibDirs.

For people who specify --target for cross compilation, they should specify the
full target triple instead of relying on --target=riscv64 expanding to
--target=riscv64-suse-linux.

Also revert rL304670 (normalize gnueabi to gnueabihf)

Note: the test added by D63497 works even without riscv64-suse-linux
in RISCV64Triples

I have kept powerpc64-suse-linux in PPCTriples because it somehow uses the
multilib behavior for native Clang. That doesn't look right to me.

Diff Detail

Event Timeline

MaskRay created this revision.Sep 13 2021, 3:35 PM
MaskRay requested review of this revision.Sep 13 2021, 3:35 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 13 2021, 3:35 PM

Regarding D74399, cmake on a fedora RISC-V host still detects a generic triple:

-- LLVM host triple: riscv64-unknown-linux-gnu
-- LLVM default target triple: riscv64-unknown-linux-gnu

as opposed to the gcc -dumpmachine triple of riscv64-redhat-linux (with the vendor and no -gnu suffix), needed for a working toolchain. (I don't know if that cmake behaviour is uncommon or not).

Are you saying that it's fine for D74399 to remain because of that, or does your argument also imply that D74399 should be reverted? Would the answer change if cmake started detecting the "proper" triple? (and what would the deprecation path for that be, if any?).

clang/lib/Driver/ToolChains/Gnu.cpp
2108–2113

Ugh, these lines with multiple entries make mentally parsing the diff rather annoying.

lenary removed a subscriber: lenary.Sep 16 2021, 4:54 AM

Regarding D74399, cmake on a fedora RISC-V host still detects a generic triple:

-- LLVM host triple: riscv64-unknown-linux-gnu
-- LLVM default target triple: riscv64-unknown-linux-gnu

as opposed to the gcc -dumpmachine triple of riscv64-redhat-linux (with the vendor and no -gnu suffix), needed for a working toolchain. (I don't know if that cmake behaviour is uncommon or not).

This is due to issues in config.guess. D109837 will fix it.

Are you saying that it's fine for D74399 to remain because of that, or does your argument also imply that D74399 should be reverted? Would the answer change if cmake started detecting the "proper" triple? (and what would the deprecation path for that be, if any?).

The code change of D74399 should be reverted. The test can stay, but I won't think we need too many riscv64-$distro-linux-gnu tests.

The code change of D74399 should be reverted. The test can stay, but I won't think we need too many riscv64-$distro-linux-gnu tests.

Makes sense. Do you want to update this patch to also remove that?

The problem here is that triples are not just used to look up the GCC installation, but the individual components have some meaning. Changing the OS vendor from "unknown" to "suse" doesn't worry me, but dropping "-gnu" might lead us to the problems described in D109837#3034457. (I'll have to test that.)

Doesn't mean we can't have this change, but I think we need to add something like if (Vendor == Triple::SUSE) Environment = Triple::GNU; somewhere. Or perhaps it's there and I'm just missing it?

Regarding gnueabi[hf]: maybe the problem was that the GCC triple is different from the LLVM triple for some historic reason? So if we configure LLVM with target armv7-suse-linux-gnueabihf it might just not find the GCC installation, but if we leave it as is, LLVM will generate incompatible code?

This patch should depend on D109837, otherwise clang builds will be broken on SUSE if you don't pass -DLLVM_DEFAULT_TARGET_TRIPLE to cmake when configuring, and I don't think we should require users to pass this option.

Basically this should be Ok. We set the LLVM_HOST_TRIPLE to match the GCC triple on almost all platforms now.

But we'll need to patch isGNUEnvironment like D110900 does and the previously mentioned config.guess changes.

clang/lib/Driver/ToolChains/Gnu.cpp
2091–2092

Not sure if we can remove them. We now use gnueabihf in the default triple, but I think the GCC installation is still at -gnueabi. Also we're using armv6kz instead of armv6hl for some reason that's beyond my understanding.

llvm/lib/Support/Triple.cpp
989–991

We have that included in out LLVM_HOST_TRIPLE now, so this should be fine. But this means we'll need to keep the translation to GCC triple alive.

Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2023, 7:52 AM