[clang] 1d51c69 - [clang][Arm] Fix handling of -Wa,-march=
David Spickett via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 4 08:36:22 PST 2021
Author: David Spickett
Date: 2021-02-04T16:36:15Z
New Revision: 1d51c699b9e2ebc5bcfdbe85c74cc871426333d4
URL: https://github.com/llvm/llvm-project/commit/1d51c699b9e2ebc5bcfdbe85c74cc871426333d4
DIFF: https://github.com/llvm/llvm-project/commit/1d51c699b9e2ebc5bcfdbe85c74cc871426333d4.diff
LOG: [clang][Arm] Fix handling of -Wa,-march=
This fixes Bugzilla #48894 for Arm, where it
was reported that -Wa,-march was not being handled
by the integrated assembler.
This was previously fixed for -Wa,-mthumb by
parsing the argument in ToolChain::ComputeLLVMTriple
instead of CollectArgsForIntegratedAssembler.
It has to be done in the former because the Triple
is read only by the time we get to the latter.
Previously only mcpu would work via -Wa but only because
"-target-cpu" is it's own option to cc1, which we were
able to modify. Target architecture is part of "-target-triple".
This change applies the same workaround to -march and cleans up
handling of -Wa,-mcpu at the same time. There were some
places where we were not using the last instance of an argument.
The existing -Wa,-mthumb code was doing this correctly,
so I've just added tests to confirm that.
Now the same rules will apply to -Wa,-march/-mcpu as would
if you just passed them to the compiler:
* -Wa/-Xassembler options only apply to assembly files.
* Architecture derived from mcpu beats any march options.
* When there are multiple mcpu or multiple march, the last
one wins.
* If there is a compiler option and an assembler option of
the same type, we prefer the one that fits the input type.
* If there is an applicable mcpu option but it is overruled
by an march, the cpu value is still used for the "-target-cpu"
cc1 option.
Reviewed By: nickdesaulniers
Differential Revision: https://reviews.llvm.org/D95872
Added:
clang/test/Driver/arm-target-as-march-mcpu.s
Modified:
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/Driver/arm-target-as-mthumb.s
Removed:
################################################################################
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index b2ddef141a75..c83638086048 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -786,15 +786,26 @@ std::string ToolChain::ComputeLLVMTriple(const ArgList &Args,
else {
// Ideally we would check for these flags in
// CollectArgsForIntegratedAssembler but we can't change the ArchName at
- // that point. There is no assembler equivalent of -mno-thumb, -marm, or
- // -mno-arm.
+ // that point.
+ llvm::StringRef WaMArch, WaMCPU;
for (const auto *A :
Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
for (StringRef Value : A->getValues()) {
+ // There is no assembler equivalent of -mno-thumb, -marm, or -mno-arm.
if (Value == "-mthumb")
IsThumb = true;
+ else if (Value.startswith("-march="))
+ WaMArch = Value.substr(7);
+ else if (Value.startswith("-mcpu="))
+ WaMCPU = Value.substr(6);
}
}
+
+ if (WaMCPU.size() || WaMArch.size()) {
+ // The way this works means that we prefer -Wa,-mcpu's architecture
+ // over -Wa,-march. Which matches the compiler behaviour.
+ Suffix = tools::arm::getLLVMArchSuffixForARM(WaMCPU, WaMArch, Triple);
+ }
}
// Assembly files should start in ARM mode, unless arch is M-profile, or
// -mthumb has been passed explicitly to the assembler. Windows is always
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index ef590db1eecd..d0606eb882f1 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -50,11 +50,14 @@ void arm::getARMArchCPUFromArgs(const ArgList &Args, llvm::StringRef &Arch,
for (const Arg *A :
Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
- StringRef Value = A->getValue();
- if (Value.startswith("-mcpu="))
- CPU = Value.substr(6);
- if (Value.startswith("-march="))
- Arch = Value.substr(7);
+ // Use getValues because -Wa can have multiple arguments
+ // e.g. -Wa,-mcpu=foo,-mcpu=bar
+ for (StringRef Value : A->getValues()) {
+ if (Value.startswith("-mcpu="))
+ CPU = Value.substr(6);
+ if (Value.startswith("-march="))
+ Arch = Value.substr(7);
+ }
}
}
@@ -290,8 +293,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args);
- const Arg *WaCPU = nullptr, *WaFPU = nullptr;
- const Arg *WaHDiv = nullptr, *WaArch = nullptr;
+ llvm::Optional<std::pair<const Arg *, StringRef>> WaCPU, WaFPU, WaHDiv,
+ WaArch;
// This vector will accumulate features from the architecture
// extension suffixes on -mcpu and -march (e.g. the 'bar' in
@@ -325,15 +328,18 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
// to the assembler correctly.
for (const Arg *A :
Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
- StringRef Value = A->getValue();
- if (Value.startswith("-mfpu=")) {
- WaFPU = A;
- } else if (Value.startswith("-mcpu=")) {
- WaCPU = A;
- } else if (Value.startswith("-mhwdiv=")) {
- WaHDiv = A;
- } else if (Value.startswith("-march=")) {
- WaArch = A;
+ // We use getValues here because you can have many options per -Wa
+ // We will keep the last one we find for each of these
+ for (StringRef Value : A->getValues()) {
+ if (Value.startswith("-mfpu=")) {
+ WaFPU = std::make_pair(A, Value.substr(6));
+ } else if (Value.startswith("-mcpu=")) {
+ WaCPU = std::make_pair(A, Value.substr(6));
+ } else if (Value.startswith("-mhwdiv=")) {
+ WaHDiv = std::make_pair(A, Value.substr(8));
+ } else if (Value.startswith("-march=")) {
+ WaArch = std::make_pair(A, Value.substr(7));
+ }
}
}
}
@@ -353,8 +359,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
if (CPUArg)
D.Diag(clang::diag::warn_drv_unused_argument)
<< CPUArg->getAsString(Args);
- CPUName = StringRef(WaCPU->getValue()).substr(6);
- CPUArg = WaCPU;
+ CPUName = WaCPU->second;
+ CPUArg = WaCPU->first;
} else if (CPUArg)
CPUName = CPUArg->getValue();
@@ -363,11 +369,12 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
if (ArchArg)
D.Diag(clang::diag::warn_drv_unused_argument)
<< ArchArg->getAsString(Args);
- ArchName = StringRef(WaArch->getValue()).substr(7);
- checkARMArchName(D, WaArch, Args, ArchName, CPUName, ExtensionFeatures,
- Triple, ArchArgFPUID);
- // FIXME: Set Arch.
- D.Diag(clang::diag::warn_drv_unused_argument) << WaArch->getAsString(Args);
+ ArchName = WaArch->second;
+ // This will set any features after the base architecture.
+ checkARMArchName(D, WaArch->first, Args, ArchName, CPUName,
+ ExtensionFeatures, Triple, ArchArgFPUID);
+ // The base architecture was handled in ToolChain::ComputeLLVMTriple because
+ // triple is read only by this point.
} else if (ArchArg) {
ArchName = ArchArg->getValue();
checkARMArchName(D, ArchArg, Args, ArchName, CPUName, ExtensionFeatures,
@@ -399,8 +406,7 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
if (FPUArg)
D.Diag(clang::diag::warn_drv_unused_argument)
<< FPUArg->getAsString(Args);
- (void)getARMFPUFeatures(D, WaFPU, Args, StringRef(WaFPU->getValue()).substr(6),
- Features);
+ (void)getARMFPUFeatures(D, WaFPU->first, Args, WaFPU->second, Features);
} else if (FPUArg) {
FPUID = getARMFPUFeatures(D, FPUArg, Args, FPUArg->getValue(), Features);
} else if (Triple.isAndroid() && getARMSubArchVersionNumber(Triple) >= 7) {
@@ -423,8 +429,7 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
if (HDivArg)
D.Diag(clang::diag::warn_drv_unused_argument)
<< HDivArg->getAsString(Args);
- getARMHWDivFeatures(D, WaHDiv, Args,
- StringRef(WaHDiv->getValue()).substr(8), Features);
+ getARMHWDivFeatures(D, WaHDiv->first, Args, WaHDiv->second, Features);
} else if (HDivArg)
getARMHWDivFeatures(D, HDivArg, Args, HDivArg->getValue(), Features);
diff --git a/clang/test/Driver/arm-target-as-march-mcpu.s b/clang/test/Driver/arm-target-as-march-mcpu.s
new file mode 100644
index 000000000000..31c027bd0fa6
--- /dev/null
+++ b/clang/test/Driver/arm-target-as-march-mcpu.s
@@ -0,0 +1,104 @@
+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+/// Also we check that the same priority rules apply to compiler and
+/// assembler options.
+///
+/// Note that the cortex-a8 is armv7-a, the cortex-a32 is armv8-a
+/// and clang's default Arm architecture is armv4t.
+
+/// Sanity check how the options behave when passed to the compiler
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+
+/// -Wa/-Xassembler doesn't apply to non assembly files
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV4 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV4 %s
+
+/// -Wa/-Xassembler does apply to assembler input
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+
+/// Check that arch name is still canonicalised
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7 %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -Wa,-march=armv7a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a -Wa,-march=armv8-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// mcpu to compiler and march to assembler, we use the assembler's architecture for assembly files.
+/// We use the target CPU for both.
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -Wa,-march=armv8a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV8,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -Wa,-march=armv8-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// march to compiler and mcpu to assembler, we use the one that matches the file type
+/// (again both get the target-cpu option either way)
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -Wa,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -Wa,-mcpu=cortex-a8 \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV8 %s
+
+/// march and mcpu to the compiler, mcpu wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+/// not dependent on order
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+/// or file type
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -mcpu=cortex-a8 \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// If we pass mcpu and march to the assembler then mcpu's arch wins
+/// (matches the compiler behaviour)
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8 -Wa,-march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv8-a -Xassembler -mcpu=cortex-a8 \
+// RUN: %s 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last mcpu to assembler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a32,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a32 -Wa,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 --check-prefix=CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -mcpu=cortex-a32 -Xassembler -mcpu=cortex-a8 \
+// RUN: %s 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last mcpu to compiler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a32 -mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last march to assembler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv8-a,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv8-a -Wa,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv8-a -Xassembler -march=armv7-a \
+// RUN: %s 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// Last march to compiler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+// TRIPLE-ARMV4: "-triple" "armv4t-unknown-linux-gnueabi"
+// TRIPLE-ARMV7: "-triple" "armv7-unknown-linux-gnueabi"
+// TRIPLE-ARMV8: "-triple" "armv8-unknown-linux-gnueabi"
+// CPU-A8: "-target-cpu" "cortex-a8"
+// EXT-CRC: "-target-feature" "+crc"
diff --git a/clang/test/Driver/arm-target-as-mthumb.s b/clang/test/Driver/arm-target-as-mthumb.s
index 7e014bb9d43f..4bfef8dbf235 100644
--- a/clang/test/Driver/arm-target-as-mthumb.s
+++ b/clang/test/Driver/arm-target-as-mthumb.s
@@ -5,12 +5,18 @@
// RUN: %clang -target armv7a-linux-gnueabi -### -c -mthumb %s 2>&1 | \
// RUN: FileCheck -check-prefix=TRIPLE-ARM %s
// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mthumb \
-// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-mthumb \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
// TRIPLE-ARM: "-triple" "armv7-unknown-linux-gnueabi"
// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mthumb %s 2>&1 | \
// RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-mthumb %s 2>&1 | \
+// RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8 -Wa,-mthumb %s 2>&1 | \
+// RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
// RUN: %clang -target armv7a-linux-gnueabi -### -c -Xassembler -mthumb %s \
// RUN: 2>&1 | FileCheck -check-prefix=TRIPLE-THUMB %s
More information about the cfe-commits
mailing list