Skip to content

Commit

Permalink
[Clang][LoongArch] Consume and check -mabi and -mfpu even if -m*-floa…
Browse files Browse the repository at this point in the history
…t is present

This kind of CLI flags duplication can sometimes be convenient for build
systems that may have to tinker with these.

For example, in the Linux kernel we almost always want to ensure no FP
instruction is emitted, so `-msoft-float` is present by default; but
sometimes we do want to allow FPU usage (e.g. certain parts of amdgpu DC
code), in which case we want the `-msoft-float` stripped and `-mfpu=64`
added. Here we face a dilemma without this change:

* Either `-mabi` is not supplied by `arch/loongarch` Makefile, in which
  case the correct ABI has to be supplied by the driver Makefile
  (otherwise the ABI will become double-float due to `-mfpu`), which is
  arguably not appropriate for a driver;
* Or `-mabi` is still supplied by `arch/loongarch` Makefile, and the
  build immediately errors out because
  `-Werror=unused-command-line-argument` is unconditionally set for
  Clang builds.

To solve this, simply make sure to check `-mabi` and `-mfpu` (and gain
some useful diagnostics in case of conflicting settings) when
`-m*-float` is successfully parsed.

Reviewed By: SixWeining, MaskRay

Differential Revision: https://reviews.llvm.org/D153707
  • Loading branch information
xen0n authored and SixWeining committed Jun 26, 2023
1 parent 6617778 commit f693200
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 25 deletions.
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,9 @@ def warn_drv_sarif_format_unstable : Warning<
def err_drv_riscv_unsupported_with_linker_relaxation : Error<
"%0 is unsupported with RISC-V linker relaxation (-mrelax)">;

def warn_drv_loongarch_conflicting_implied_val : Warning<
"ignoring '%0' as it conflicts with that implied by '%1' (%2)">,
InGroup<OptionIgnored>;
def err_drv_loongarch_invalid_mfpu_EQ : Error<
"invalid argument '%0' to -mfpu=; must be one of: 64, 32, none, 0 (alias for none)">;

Expand Down
77 changes: 60 additions & 17 deletions clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,75 @@ StringRef loongarch::getLoongArchABI(const Driver &D, const ArgList &Args,
"Unexpected triple");
bool IsLA32 = Triple.getArch() == llvm::Triple::loongarch32;

// Record -mabi value for later use.
const Arg *MABIArg = Args.getLastArg(options::OPT_mabi_EQ);
StringRef MABIValue;
if (MABIArg) {
MABIValue = MABIArg->getValue();
}

// Parse -mfpu value for later use.
const Arg *MFPUArg = Args.getLastArg(options::OPT_mfpu_EQ);
int FPU = -1;
if (MFPUArg) {
StringRef V = MFPUArg->getValue();
if (V == "64")
FPU = 64;
else if (V == "32")
FPU = 32;
else if (V == "0" || V == "none")
FPU = 0;
else
D.Diag(diag::err_drv_loongarch_invalid_mfpu_EQ) << V;
}

// Check -m*-float firstly since they have highest priority.
if (const Arg *A = Args.getLastArg(options::OPT_mdouble_float,
options::OPT_msingle_float,
options::OPT_msoft_float)) {
if (A->getOption().matches(options::OPT_mdouble_float))
return IsLA32 ? "ilp32d" : "lp64d";
if (A->getOption().matches(options::OPT_msingle_float))
return IsLA32 ? "ilp32f" : "lp64f";
if (A->getOption().matches(options::OPT_msoft_float))
return IsLA32 ? "ilp32s" : "lp64s";
StringRef ImpliedABI;
int ImpliedFPU = -1;
if (A->getOption().matches(options::OPT_mdouble_float)) {
ImpliedABI = IsLA32 ? "ilp32d" : "lp64d";
ImpliedFPU = 64;
}
if (A->getOption().matches(options::OPT_msingle_float)) {
ImpliedABI = IsLA32 ? "ilp32f" : "lp64f";
ImpliedFPU = 32;
}
if (A->getOption().matches(options::OPT_msoft_float)) {
ImpliedABI = IsLA32 ? "ilp32s" : "lp64s";
ImpliedFPU = 0;
}

// Check `-mabi=` and `-mfpu=` settings and report if they conflict with
// the higher-priority settings implied by -m*-float.
//
// ImpliedABI and ImpliedFPU are guaranteed to have valid values because
// one of the match arms must match if execution can arrive here at all.
if (!MABIValue.empty() && ImpliedABI != MABIValue)
D.Diag(diag::warn_drv_loongarch_conflicting_implied_val)
<< MABIArg->getAsString(Args) << A->getAsString(Args) << ImpliedABI;

if (FPU != -1 && ImpliedFPU != FPU)
D.Diag(diag::warn_drv_loongarch_conflicting_implied_val)
<< MFPUArg->getAsString(Args) << A->getAsString(Args) << ImpliedFPU;

return ImpliedABI;
}

// If `-mabi=` is specified, use it.
if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
return A->getValue();
if (!MABIValue.empty())
return MABIValue;

// Select abi based on -mfpu=xx.
if (const Arg *A = Args.getLastArg(options::OPT_mfpu_EQ)) {
StringRef FPU = A->getValue();
if (FPU == "64")
return IsLA32 ? "ilp32d" : "lp64d";
if (FPU == "32")
return IsLA32 ? "ilp32f" : "lp64f";
if (FPU == "0" || FPU == "none")
return IsLA32 ? "ilp32s" : "lp64s";
D.Diag(diag::err_drv_loongarch_invalid_mfpu_EQ) << FPU;
switch (FPU) {
case 64:
return IsLA32 ? "ilp32d" : "lp64d";
case 32:
return IsLA32 ? "ilp32f" : "lp64f";
case 0:
return IsLA32 ? "ilp32s" : "lp64s";
}

// Choose a default based on the triple.
Expand Down
12 changes: 9 additions & 3 deletions clang/test/Driver/loongarch-mdouble-float.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
// RUN: %clang --target=loongarch64 -mdouble-float -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1
// RUN: %clang --target=loongarch64 -mdouble-float -mfpu=64 -mabi=lp64d -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,NOWARN
// RUN: %clang --target=loongarch64 -mdouble-float -mfpu=0 -mabi=lp64s -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,WARN
// RUN: FileCheck %s --check-prefixes=CC1,WARN,WARN-FPU0
// RUN: %clang --target=loongarch64 -mdouble-float -mfpu=none -mabi=lp64s -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,WARN,WARN-FPUNONE
// RUN: %clang --target=loongarch64 -mdouble-float -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR

// WARN: warning: argument unused during compilation: '-mfpu=0'
// WARN: warning: argument unused during compilation: '-mabi=lp64s'
// NOWARN-NOT: warning:
// WARN: warning: ignoring '-mabi=lp64s' as it conflicts with that implied by '-mdouble-float' (lp64d)
// WARN-FPU0: warning: ignoring '-mfpu=0' as it conflicts with that implied by '-mdouble-float' (64)
// WARN-FPUNONE: warning: ignoring '-mfpu=none' as it conflicts with that implied by '-mdouble-float' (64)

// CC1: "-target-feature" "+f"{{.*}} "-target-feature" "+d"
// CC1: "-target-abi" "lp64d"
Expand Down
9 changes: 6 additions & 3 deletions clang/test/Driver/loongarch-msingle-float.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// RUN: %clang --target=loongarch64 -msingle-float -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1
// RUN: %clang --target=loongarch64 -msingle-float -mfpu=0 -mabi=lp64s -fsyntax-only %s -### 2>&1 | \
// RUN: %clang --target=loongarch64 -msingle-float -mfpu=32 -mabi=lp64f -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,NOWARN
// RUN: %clang --target=loongarch64 -msingle-float -mfpu=64 -mabi=lp64s -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,WARN
// RUN: %clang --target=loongarch64 -msingle-float -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR

// WARN: warning: argument unused during compilation: '-mfpu=0'
// WARN: warning: argument unused during compilation: '-mabi=lp64s'
// NOWARN-NOT: warning:
// WARN: warning: ignoring '-mabi=lp64s' as it conflicts with that implied by '-msingle-float' (lp64f)
// WARN: warning: ignoring '-mfpu=64' as it conflicts with that implied by '-msingle-float' (32)

// CC1: "-target-feature" "+f"{{.*}} "-target-feature" "-d"
// CC1: "-target-abi" "lp64f"
Expand Down
7 changes: 5 additions & 2 deletions clang/test/Driver/loongarch-msoft-float.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// RUN: %clang --target=loongarch64 -msoft-float -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefix=CC1
// RUN: %clang --target=loongarch64 -msoft-float -mfpu=0 -mabi=lp64s -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,NOWARN
// RUN: %clang --target=loongarch64 -msoft-float -mfpu=64 -mabi=lp64d -fsyntax-only %s -### 2>&1 | \
// RUN: FileCheck %s --check-prefixes=CC1,WARN
// RUN: %clang --target=loongarch64 -msoft-float -S -emit-llvm %s -o - | \
// RUN: FileCheck %s --check-prefix=IR

// WARN: warning: argument unused during compilation: '-mfpu=64'
// WARN: warning: argument unused during compilation: '-mabi=lp64d'
// NOWARN-NOT: warning:
// WARN: warning: ignoring '-mabi=lp64d' as it conflicts with that implied by '-msoft-float' (lp64s)
// WARN: warning: ignoring '-mfpu=64' as it conflicts with that implied by '-msoft-float' (0)

// CC1: "-target-feature" "-f"{{.*}} "-target-feature" "-d"
// CC1: "-target-abi" "lp64s"
Expand Down

0 comments on commit f693200

Please sign in to comment.