Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/lib/Basic/Targets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
return std::make_unique<OHOSTargetInfo<ARMleTargetInfo>>(Triple, Opts);
case llvm::Triple::FreeBSD:
return std::make_unique<FreeBSDTargetInfo<ARMleTargetInfo>>(Triple, Opts);
case llvm::Triple::Fuchsia:
return std::make_unique<FuchsiaTargetInfo<ARMleTargetInfo>>(Triple, Opts);
case llvm::Triple::NetBSD:
return std::make_unique<NetBSDTargetInfo<ARMleTargetInfo>>(Triple, Opts);
case llvm::Triple::OpenBSD:
Expand Down Expand Up @@ -254,6 +256,8 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
return std::make_unique<AppleMachOARMTargetInfo>(Triple, Opts);

switch (os) {
case llvm::Triple::Fuchsia:
return std::make_unique<FuchsiaTargetInfo<ARMbeTargetInfo>>(Triple, Opts);
case llvm::Triple::Linux:
return std::make_unique<LinuxTargetInfo<ARMbeTargetInfo>>(Triple, Opts);
case llvm::Triple::NetBSD:
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Basic/Targets/ARM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ ARMTargetInfo::ARMTargetInfo(const llvm::Triple &Triple,
: TargetInfo(Triple), FPMath(FP_Default), IsAAPCS(true), LDREX(0),
HW_FP(0) {
bool IsFreeBSD = Triple.isOSFreeBSD();
bool IsFuchsia = Triple.isOSFuchsia();
bool IsOpenBSD = Triple.isOSOpenBSD();
bool IsNetBSD = Triple.isOSNetBSD();
bool IsHaiku = Triple.isOSHaiku();
Expand Down Expand Up @@ -332,7 +333,7 @@ ARMTargetInfo::ARMTargetInfo(const llvm::Triple &Triple,
default:
if (IsNetBSD)
setABI("apcs-gnu");
else if (IsFreeBSD || IsOpenBSD || IsHaiku || IsOHOS)
else if (IsFreeBSD || IsFuchsia || IsOpenBSD || IsHaiku || IsOHOS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't actually been able to discern how aapcs-linux actually differs from aapcs or aapcs-vfp.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resources I found when researching this is https://wiki.debian.org/ArmEabiPort and https://kanj.github.io/elfs/book/armMusl/cross-tools/abi.html and according to those the only difference is enum size: aapcs defines enums to be a variable sized type, while with aapcs-linux they are always ints (4 bytes).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, that is the default for -fshort-enum IOW. I don't know why Linux long ago diverged from the clearly-published AAPCS on this, but for us matching Linux ABIs for this sort of detail (things likely relevant to porting code from arm-linux distinct from direct OS dependencies) is more important than published standards or CPU vendor's recommendations. It would be nice if the code made it easier to discover that aapcs-linux means aapcs + -fshort-enum.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's especially notable if not only the glibc-based Linux configurations, but musl, android, etc. are also still doing -fshort-enum / aapcs-linux then there is no question that Fuchsia wants to follow them all. If e.g. Android differed from -gnueabihf on this then we might consider whether to align with one or the other.

setABI("aapcs-linux");
else
setABI("aapcs");
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Driver/ToolChains/Arch/ARM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ void arm::setArchNameInTriple(const Driver &D, const ArgList &Args,
// Thumb2 is the default for V7 on Darwin.
(llvm::ARM::parseArchVersion(Suffix) == 7 &&
Triple.isOSBinFormatMachO()) ||
// Thumb2 is the default for Fuchsia.
Triple.isOSFuchsia() ||
// FIXME: this is invalid for WindowsCE
Triple.isOSWindows();

Expand Down Expand Up @@ -452,6 +454,9 @@ arm::FloatABI arm::getDefaultFloatABI(const llvm::Triple &Triple) {
case llvm::Triple::OpenBSD:
return FloatABI::SoftFP;

case llvm::Triple::Fuchsia:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering about this. It seems like it shouldn't be needed because the default case should already derive it correctly from Triple.getEnvironment().

But that's because I'm assuming that it should be GNUEABIHF, as it is in the most-similar existing target, arm*-linux-gnueabihf. In looking through what that's used for, it's frankly hard to tell. There are few checks for it directly. Things like isTargetEHABICompatible() definitely need to come up true, but I'm not sure what getEnvironment() value we have, and many of them are matched there.

Then there's isTargetGNUAEABI(), and isGNUEnvironment(). Those are tested in a bunch of places. Most of those we want to match what GNUEABIHF would give us. But in a few we might not (glibc-specific assumptions).

We can probably iterate to get all these corners right. But we'll probably need to actually investigate each use proactively and get the knobs right for the Fuchsia context rather than only tweak things as problems arise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Fuchsia, on other architectures we don't specify any environment and I felt that requiring environment only for ARM (i.e. using arm-fuchsia-gnueabihf) would be inconsistent. Furthermore, as you pointed out, there are also place where GNUEABIHF behavior is likely undesirable for Fuchsia so I decide to instead directly conditionalize this and other places on OS rather than environment. I do expect we might need to another pass to make sure we get the right behavior, but at least for the initial version this change should be sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't realize that this "environment" was tied to the normalized triple text itself. I agree, we want the maximal triple to be something like armv8-unknown-fuchsia at most and canonically would tell people to just use --target=arm-fuchsia for sure. I just meant that it seemed likely that more actual choices would match what ::GNUEABIHF is used for than not, at a guess. (We are going for pretty much the armv7-linux-gnueabihf ABI in terms of most things: ISA support, code-gen details, C-visible core ABI issues, though armv8+... for -march and some differences in libc ABI expectations.)

return FloatABI::Hard;

default:
if (Triple.isOHOSFamily())
return FloatABI::Soft;
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Driver/arm-abi.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
// FreeBSD / OpenBSD default to aapcs-linux
// RUN: %clang -target arm--freebsd- %s -### -o %t.o 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK-AAPCS-LINUX %s
// RUN: %clang -target arm--fuchsia- %s -### -o %t.o 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK-AAPCS-LINUX %s
// RUN: %clang -target arm--openbsd- %s -### -o %t.o 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK-AAPCS-LINUX %s
// RUN: %clang -target arm--haiku- %s -### -o %t.o 2>&1 \
Expand Down
9 changes: 9 additions & 0 deletions clang/test/Driver/fuchsia.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
// RUN: %clang -### %s --target=arm-unknown-fuchsia \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
// RUN: | FileCheck -check-prefixes=CHECK,CHECK-ARMV8A %s
// RUN: %clang -### %s --target=aarch64-unknown-fuchsia \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
Expand All @@ -14,6 +18,10 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
// RUN: %clang -### %s --target=arm-fuchsia \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
// RUN: | FileCheck -check-prefixes=CHECK,CHECK-ARMV8A %s
// RUN: %clang -### %s --target=aarch64-fuchsia \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
Expand All @@ -24,6 +32,7 @@
// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
// CHECK: "-cc1"
// CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
// CHECK-ARMV8A: "-triple" "thumbv8a-unknown-fuchsia"
// CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
// CHECK-RISCV64: "-triple" "riscv64-unknown-fuchsia"
// CHECK: "-funwind-tables=2"
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/IR/RuntimeLibcalls.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ struct RuntimeLibcallsInfo {

static bool hasAEABILibcalls(const Triple &TT) {
return TT.isTargetAEABI() || TT.isTargetGNUAEABI() ||
TT.isTargetMuslAEABI() || TT.isAndroid();
TT.isTargetMuslAEABI() || TT.isOSFuchsia() || TT.isAndroid();
}

LLVM_READONLY
Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/TargetParser/Triple.h
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,8 @@ class Triple {
getEnvironment() == Triple::GNUEABIHF ||
getEnvironment() == Triple::GNUEABIHFT64 ||
getEnvironment() == Triple::OpenHOS ||
getEnvironment() == Triple::MuslEABIHF || isAndroid()) &&
getEnvironment() == Triple::MuslEABIHF ||
isAndroid() || isOSFuchsia()) &&
isOSBinFormatELF() && !isOSNetBSD();
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/ARM/ARMISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM_,

// Register based DivRem for AEABI (RTABI 4.2)
if (TT.isTargetAEABI() || TT.isAndroid() || TT.isTargetGNUAEABI() ||
TT.isTargetMuslAEABI() || TT.isOSWindows()) {
TT.isTargetMuslAEABI() || TT.isOSFuchsia() || TT.isOSWindows()) {
setOperationAction(ISD::SREM, MVT::i64, Custom);
setOperationAction(ISD::UREM, MVT::i64, Custom);
HasStandaloneRem = false;
Expand Down Expand Up @@ -20574,7 +20574,7 @@ static TargetLowering::ArgListTy getDivRemArgList(
SDValue ARMTargetLowering::LowerDivRem(SDValue Op, SelectionDAG &DAG) const {
assert((Subtarget->isTargetAEABI() || Subtarget->isTargetAndroid() ||
Subtarget->isTargetGNUAEABI() || Subtarget->isTargetMuslAEABI() ||
Subtarget->isTargetWindows()) &&
Subtarget->isTargetFuchsia() || Subtarget->isTargetWindows()) &&
"Register-based DivRem lowering only");
unsigned Opcode = Op->getOpcode();
assert((Opcode == ISD::SDIVREM || Opcode == ISD::UDIVREM) &&
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/ARM/ARMSubtarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
bool isTargetWatchOS() const { return TargetTriple.isWatchOS(); }
bool isTargetWatchABI() const { return TargetTriple.isWatchABI(); }
bool isTargetDriverKit() const { return TargetTriple.isDriverKit(); }
bool isTargetFuchsia() const { return TargetTriple.isOSFuchsia(); }
bool isTargetLinux() const { return TargetTriple.isOSLinux(); }
bool isTargetNetBSD() const { return TargetTriple.isOSNetBSD(); }
bool isTargetWindows() const { return TargetTriple.isOSWindows(); }
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/TargetParser/ARMTargetParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,8 @@ StringRef ARM::computeDefaultTargetABI(const Triple &TT) {
default:
if (TT.isOSNetBSD())
return "apcs-gnu";
if (TT.isOSFreeBSD() || TT.isOSOpenBSD() || TT.isOSHaiku() ||
TT.isOHOSFamily())
if (TT.isOSFreeBSD() || TT.isOSFuchsia() || TT.isOSOpenBSD() ||
TT.isOSHaiku() || TT.isOHOSFamily())
return "aapcs-linux";
return "aapcs";
}
Expand Down Expand Up @@ -648,6 +648,8 @@ StringRef ARM::getARMCPUForArch(const llvm::Triple &Triple, StringRef MArch) {
}
case llvm::Triple::OpenBSD:
return "cortex-a8";
case llvm::Triple::Fuchsia:
return "cortex-a53";
default:
switch (Triple.getEnvironment()) {
case llvm::Triple::EABIHF:
Expand Down
Loading