[clang][Driver] Correct runtime path for Arm hard float targets

[DavidSpickett]

Summary

On armv8l and arm (armv7) the architecture in the library
path will be "armhf" if we have hard float enabled.

This fixes failures like:
https://lab.llvm.org/buildbot/#/builders/178/builds/591

Where we can't find the libraries since
https://reviews.llvm.org/D107799 landed.

Before:
$ ./bin/clang --target=armv8l-unknown-linux-gnueabihf -print-runtime-dir
<...>/build-llvm-arm/lib/clang/14.0.0/lib/linux

After:
$ ./bin/clang --target=armv8l-unknown-linux-gnueabihf -print-runtime-dir
<...>/build-llvm-arm/lib/clang/14.0.0/lib/armhf-unknown-linux-gnueabihf

[DavidSpickett]

getRuntimePath seems like the place to fix it given that https://reviews.llvm.org/D50547 calls it. This fixes the tests but I'm not sure if there's other places to consider.

[DavidSpickett]

I should probably *add* a test somewhere for this too.

[DavidSpickett]

Added a test.

I have not checked what adding an -mfloat-abi that doesn't
match the ABI part of the triple does because then
we'd need to modify the ABI part of the triple as well.

Which might be the correct thing to do in the end but I'll wait
and see what people think.

[MaskRay]

Do you know why --target=armv8l-unknown-linux-gnueabihf picks armhf-unknown-linux-gnueabihf instead of armv8l-unknown-linux-gnueabihf?
(For new tests, prefer --target= to -target ; space separated driver options are not the convention.)

The direction the Clang driver should move to is less magic. See D109727.
If you can take a look whether the various ARM*Triples in Gnu.cpp around line 2090 can be cleaned up (removed), that would be great!

clang/lib/Driver/ToolChain.cpp

492

Add a comment similar to

// --target=armv8l-unknown-linux-gnueabi prefers to change the machine part to arm{,hf}. The os part may change to gnueabi{,hf}.

[MaskRay]

clang/test/Driver/arm-float-abi-runtime-path.c

7

Having one of armv7/armv8l is sufficient.

We should use the fewest RUN lines to ensure coverage. As is, spawning 8 clang processes is too expensive.

14

Having one of armv7/armv8l is sufficient.

[phosek]

In D110142#3013138, @MaskRay wrote:

Do you know why --target=armv8l-unknown-linux-gnueabihf picks armhf-unknown-linux-gnueabihf instead of armv8l-unknown-linux-gnueabihf?
(For new tests, prefer --target= to -target ; space separated driver options are not the convention.)

The direction the Clang driver should move to is less magic. See D109727

I agree with @MaskRay, we discussed this when updating the Clang multiarch layout and the decision was to use normalized LLVM triples to avoid additional translations as is done in this change.

It also seems like armhf-unknown-linux-gnueabihf duplicates the hf bit (there's both armhf and gnueabihf) which seems unnecessary.

[DavidSpickett]

So would you expect to see libraries in lib/clang/14.0.0/lib/armv8l-unknown-linux-gnueabihf and not change clang's logic?

The output dir is worked out in cmake by get_compiler_rt_output_dir which calls get_compiler_rt_target which takes the arch given and adds it to the suffix of the triple. So it adds "armhf" to "-unknown-linux-gnueabihf". (which is why the "hf" is duplicated)

A few places look for "armhf" specifically as the arch, so it's not as easy as just not converting "armv8l" into that. But if we can confirm what the goal is here then I can find out how to properly handle those.

You can just bodge get_compiler_rt_target to just use the target triple but of course that breaks any build for multiple targets. If I'm reading test_targets correctly, it builds a list of architectures to build as opposed to finding the first working one.

[MaskRay]

In D110142#3015482, @DavidSpickett wrote:

So would you expect to see libraries in lib/clang/14.0.0/lib/armv8l-unknown-linux-gnueabihf and not change clang's logic?

I'd hope that it works this way (armv8l-*) :)

The output dir is worked out in cmake by get_compiler_rt_output_dir which calls get_compiler_rt_target which takes the arch given and adds it to the suffix of the triple. So it adds "armhf" to "-unknown-linux-gnueabihf". (which is why the "hf" is duplicated)

A few places look for "armhf" specifically as the arch, so it's not as easy as just not converting "armv8l" into that. But if we can confirm what the goal is here then I can find out how to properly handle those.

I took a glance at get_compiler_rt_target... It is complex. If you can figure it out, that will be great!

You can just bodge get_compiler_rt_target to just use the target triple but of course that breaks any build for multiple targets. If I'm reading test_targets correctly, it builds a list of architectures to build as opposed to finding the first working one.

[DavidSpickett]

I'd hope that it works this way (armv8l-*) :)

Cool I'm going to work with that goal in mind then.

I've turned it off by default for 32 bit Arm linux until it's made compatible: https://reviews.llvm.org/rG3c65d54ec3d2

clang/include/clang/Driver/ToolChain.h

 //===- ToolChain.h - Collections of tools for one platform ------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
public:
   getCompilerRTArgString(const llvm::opt::ArgList &Args, StringRef Component,
                          FileType Type = ToolChain::FT_Static) const;
 
   std::string getCompilerRTBasename(const llvm::opt::ArgList &Args,
                                     StringRef Component,
                                     FileType Type = ToolChain::FT_Static) const;
 
   // Returns target specific runtime path if it exists.
-  virtual std::string getRuntimePath() const;
+  virtual std::string getRuntimePath(const llvm::opt::ArgList &Args) const;
 
   // Returns target specific standard library path if it exists.
   virtual std::string getStdlibPath() const;
 
   // Returns <ResourceDir>/lib/<OSName>/<arch>.  This is used by runtimes (such
   // as OpenMP) to find arch-specific libraries.
   std::string getArchSpecificLibPath() const;
 

clang/lib/Driver/Driver.cpp

 //===--- Driver.cpp - Clang GCC Compatible Driver -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
for (const std::string &Path : TC.getFilePaths()) {
       else
         llvm::outs() << Path;
     }
     llvm::outs() << "\n";
     return false;
   }
 
   if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
-    std::string CandidateRuntimePath = TC.getRuntimePath();
+    std::string CandidateRuntimePath = TC.getRuntimePath(C.getArgs());
     if (getVFS().exists(CandidateRuntimePath))
       llvm::outs() << CandidateRuntimePath << '\n';
     else
       llvm::outs() << TC.getCompilerRTPath() << '\n';
     return false;
   }
 
   // FIXME: The following handlers should use a callback mechanism, we don't

clang/lib/Driver/ToolChain.cpp

 //===- ToolChain.cpp - Collections of tools for one platform --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static ToolChain::RTTIMode CalculateRTTIMode(const ArgList &Args,
   // -frtti is default, except for the PS4 CPU.
   return (Triple.isPS4CPU()) ? ToolChain::RM_Disabled : ToolChain::RM_Enabled;
 }
 
 ToolChain::ToolChain(const Driver &D, const llvm::Triple &T,
                      const ArgList &Args)
     : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
       CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)) {
-  std::string RuntimePath = getRuntimePath();
+  std::string RuntimePath = getRuntimePath(Args);
   if (getVFS().exists(RuntimePath))
     getLibraryPaths().push_back(RuntimePath);
 
   std::string StdlibPath = getStdlibPath();
   if (getVFS().exists(StdlibPath))
     getFilePaths().push_back(StdlibPath);
 
   std::string CandidateLibPath = getArchSpecificLibPath();

 }
 
 const char *ToolChain::getCompilerRTArgString(const llvm::opt::ArgList &Args,
                                               StringRef Component,
                                               FileType Type) const {
   return Args.MakeArgString(getCompilerRT(Args, Component, Type));
 }
 
-std::string ToolChain::getRuntimePath() const {
+std::string ToolChain::getRuntimePath(const llvm::opt::ArgList &Args) const {
+  llvm::Triple triple = getTriple();
+  RegisterEffectiveTriple TripleRAII(*this, triple);
+  auto arch_name = getArchNameForCompilerRTLib(*this, Args);
+  triple.setArchName(arch_name);
   SmallString<128> P(D.ResourceDir);
-  llvm::sys::path::append(P, "lib", getTripleString());
+  llvm::sys::path::append(P, "lib", triple.getTriple());
   return std::string(P.str());
 }
 
 std::string ToolChain::getStdlibPath() const {
   SmallString<128> P(D.Dir);
   llvm::sys::path::append(P, "..", "lib", getTripleString());
   return std::string(P.str());
 }

clang/test/Driver/arm-float-abi-runtime-path.c

+// REQUIRES: arm-registered-target
+
+// RUN: %clang %s -target armv8l-unknown-linux-gnueabihf -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARMHF %s
+// RUN: %clang %s -target arm-unknown-linux-gnueabihf -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARMHF %s
+// RUN: %clang %s -target armv7-unknown-linux-gnueabihf -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARMHF %s
+
+// RUN: %clang %s -target armv8l-unknown-linux-gnueabi -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARM %s
+// RUN: %clang %s -target arm-unknown-linux-gnueabi -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARM %s
+// RUN: %clang %s -target armv7-unknown-linux-gnueabi -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARM %s
+
+// RUN: %clang %s -target arm-pc-windows-msvc -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=WINDOWS %s
+/// armhf-pc... isn't recognised so just check that the float-abi option is ignored
+// RUN: %clang %s -target arm-pc-windows-msvc -mfloat-abi=hard -print-runtime-dir \
+// RUN:        -resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=WINDOWS %s
+
+// ARMHF:   lib{{/|\\}}armhf-unknown-linux-gnueabihf{{$}}
+// ARM:     lib{{/|\\}}arm-unknown-linux-gnueabi{{$}}
+// WINDOWS: lib{{/|\\}}arm-pc-windows-msvc{{$}}