[RISCV] Match GCC `-march`/`-mabi` driver defaults

Summary:
Clang/LLVM is a cross-compiler, and so we don't have to make a choice
about `-march`/`-mabi` at build-time, but we may have to compute a
default `-march`/`-mabi` when compiling a program. Until now, each
place that has needed a default `-march` has calculated one itself.

This patch adds a single place where a default `-march` is calculated,
in order to avoid calculating different defaults in different places.

This patch adds a new function `riscv::getRISCVArch` which encapsulates
this logic based on GCC's for computing a default `-march` value
when none is provided. This patch also updates the logic in
`riscv::getRISCVABI` to match the logic in GCC's build system for
computing a default `-mabi`.

This patch also updates anywhere that `-march` is used to now use the
new function which can compute a default. In particular, we now
explicitly pass a `-march` value down to the gnu assembler.

GCC has convoluted logic in its build system to choose a default
`-march`/`-mabi` based on build options, which would be good to match.
This patch is based on the logic in GCC 9.2.0. This commit's logic is
different to GCC's only for baremetal targets, where we default
to rv32imac/ilp32 or rv64imac/lp64 depending on the target triple.

Tests have been updated to match the new logic.

Reviewers: asb, luismarques, rogfer01, kito-cheng, khchen

Reviewed By: asb, luismarques

Subscribers: sameer.abuasal, rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, shiva0217, jrtc27, MaskRay, zzheng, edward-jones, MartinMosbeck, brucehoult, the_o, rkruppe, PkmX, jocewei, psnobl, benna, Jim, s.egerton, pzheng, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69383

6 files changed, 182 insertions, 29 deletions

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1139116ed10..aa0d88db1c9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -223,6 +223,15 @@ ABI Changes in Clang
   element. Clang now matches the gcc behavior on Linux and NetBSD. You can
   switch back to old API behavior with flag: -fclang-abi-compat=9.0.
 
+- RISC-V now chooses a default ``-march=`` and ``-mabi=`` to match (in almost
+  all cases) the GCC defaults. On baremetal targets, where neither ``-march=``
+  nor ``-mabi=`` are specified, Clang now differs from GCC by defaulting to
+  ``-march=rv32imac -mabi=ilp32`` or ``-march=rv64imac -mabi=lp64`` depending on
+  the architecture in the target triple. These do not always match the defaults
+  in Clang 9. We strongly suggest that you explicitly pass `-march=` and
+  `-mabi=` when compiling for RISC-V, due to how extensible this architecture
+  is.
+
 OpenMP Support in Clang
 -----------------------
 
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index a26f723a507..8c343b8693f 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -357,14 +357,9 @@ static bool getArchFeatures(const Driver &D, StringRef MArch,
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
                                    const ArgList &Args,
                                    std::vector<StringRef> &Features) {
-  llvm::Optional<StringRef> MArch;
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
-    MArch = A->getValue();
-  else if (Triple.getOS() == llvm::Triple::Linux)
-    // RISC-V Linux defaults to rv{32,64}gc.
-    MArch = Triple.getArch() == llvm::Triple::riscv32 ? "rv32gc" : "rv64gc";
+  StringRef MArch = getRISCVArch(Args, Triple);
 
-  if (MArch.hasValue() && !getArchFeatures(D, *MArch, Features, Args))
+  if (!getArchFeatures(D, MArch, Features, Args))
     return;
 
   // Handle features corresponding to "-ffixed-X" options
@@ -455,12 +450,132 @@ StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
           Triple.getArch() == llvm::Triple::riscv64) &&
          "Unexpected triple");
 
+  // GCC's logic around choosing a default `-mabi=` is complex. If GCC is not
+  // configured using `--with-abi=`, then the logic for the default choice is
+  // defined in config.gcc. This function is based on the logic in GCC 9.2.0. We
+  // deviate from GCC's default only on baremetal targets (UnknownOS) where
+  // neither `-march` nor `-mabi` is specified.
+  //
+  // The logic uses the following, in order:
+  // 1. Explicit choices using `--with-abi=`
+  // 2. A default based on `--with-arch=`, if provided
+  // 3. A default based on the target triple's arch
+  //
+  // The logic in config.gcc is a little circular but it is not inconsistent.
+  //
+  // Clang does not have `--with-arch=` or `--with-abi=`, so we use `-march=`
+  // and `-mabi=` respectively instead.
+
+  // 1. If `-mabi=` is specified, use it.
   if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
     return A->getValue();
 
-  // RISC-V Linux defaults to ilp32d/lp64d
-  if (Triple.getOS() == llvm::Triple::Linux)
-    return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32d" : "lp64d";
-  else
-    return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
+  // 2. Choose a default based on `-march=`
+  //
+  // rv32g | rv32*d -> ilp32d
+  // rv32e -> ilp32e
+  // rv32* -> ilp32
+  // rv64g | rv64*d -> lp64d
+  // rv64* -> lp64
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
+    StringRef MArch = A->getValue();
+
+    if (MArch.startswith_lower("rv32")) {
+      // FIXME: parse `March` to find `D` extension properly
+      if (MArch.substr(4).contains_lower("d") ||
+          MArch.startswith_lower("rv32g"))
+        return "ilp32d";
+      else if (MArch.startswith_lower("rv32e"))
+        return "ilp32e";
+      else
+        return "ilp32";
+    } else if (MArch.startswith_lower("rv64")) {
+      // FIXME: parse `March` to find `D` extension properly
+      if (MArch.substr(4).contains_lower("d") ||
+          MArch.startswith_lower("rv64g"))
+        return "lp64d";
+      else
+        return "lp64";
+    }
+  }
+
+  // 3. Choose a default based on the triple
+  //
+  // We deviate from GCC's defaults here:
+  // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
+  // - On all other OSs we use the double floating point calling convention.
+  if (Triple.getArch() == llvm::Triple::riscv32) {
+    if (Triple.getOS() == llvm::Triple::UnknownOS)
+      return "ilp32";
+    else
+      return "ilp32d";
+  } else {
+    if (Triple.getOS() == llvm::Triple::UnknownOS)
+      return "lp64";
+    else
+      return "lp64d";
+  }
+}
+
+StringRef riscv::getRISCVArch(const llvm::opt::ArgList &Args,
+                              const llvm::Triple &Triple) {
+  assert((Triple.getArch() == llvm::Triple::riscv32 ||
+          Triple.getArch() == llvm::Triple::riscv64) &&
+         "Unexpected triple");
+
+  // GCC's logic around choosing a default `-march=` is complex. If GCC is not
+  // configured using `--with-arch=`, then the logic for the default choice is
+  // defined in config.gcc. This function is based on the logic in GCC 9.2.0. We
+  // deviate from GCC's default only on baremetal targets (UnknownOS) where
+  // neither `-march` nor `-mabi` is specified.
+  //
+  // The logic uses the following, in order:
+  // 1. Explicit choices using `--with-arch=`
+  // 2. A default based on `--with-abi=`, if provided
+  // 3. A default based on the target triple's arch
+  //
+  // The logic in config.gcc is a little circular but it is not inconsistent.
+  //
+  // Clang does not have `--with-arch=` or `--with-abi=`, so we use `-march=`
+  // and `-mabi=` respectively instead.
+  //
+  // Clang does not yet support MULTILIB_REUSE, so we use `rv{XLEN}imafdc`
+  // instead of `rv{XLEN}gc` though they are (currently) equivalent.
+
+  // 1. If `-march=` is specified, use it.
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
+    return A->getValue();
+
+  // 2. Choose a default based on `-mabi=`
+  //
+  // ilp32e -> rv32e
+  // ilp32 | ilp32f | ilp32d -> rv32imafdc
+  // lp64 | lp64f | lp64d -> rv64imafdc
+  if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) {
+    StringRef MABI = A->getValue();
+
+    if (MABI.equals_lower("ilp32e"))
+      return "rv32e";
+    else if (MABI.startswith_lower("ilp32"))
+      return "rv32imafdc";
+    else if (MABI.startswith_lower("lp64"))
+      return "rv64imafdc";
+  }
+
+  // 3. Choose a default based on the triple
+  //
+  // We deviate from GCC's defaults here:
+  // - On `riscv{XLEN}-unknown-elf` we default to `rv{XLEN}imac`
+  // - On all other OSs we use `rv{XLEN}imafdc` (equivalent to `rv{XLEN}gc`)
+  if (Triple.getArch() == llvm::Triple::riscv32) {
+    if (Triple.getOS() == llvm::Triple::UnknownOS)
+      return "rv32imac";
+    else
+      return "rv32imafdc";
+  } else {
+    if (Triple.getOS() == llvm::Triple::UnknownOS)
+      return "rv64imac";
+    else
+      return "rv64imafdc";
+  }
 }
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.h b/clang/lib/Driver/ToolChains/Arch/RISCV.h
index 10eaf3c897b..d4a519cdab3 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.h
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.h
@@ -24,6 +24,8 @@ void getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
                             std::vector<llvm::StringRef> &Features);
 StringRef getRISCVABI(const llvm::opt::ArgList &Args,
                       const llvm::Triple &Triple);
+StringRef getRISCVArch(const llvm::opt::ArgList &Args,
+                       const llvm::Triple &Triple);
 } // end namespace riscv
 } // namespace tools
 } // end namespace driver
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 6a0c756031d..2dd3450dd1b 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -709,11 +709,9 @@ void tools::gnutools::Assembler::ConstructJob(Compilation &C,
     StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
     CmdArgs.push_back("-mabi");
     CmdArgs.push_back(ABIName.data());
-    if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-      StringRef MArch = A->getValue();
-      CmdArgs.push_back("-march");
-      CmdArgs.push_back(MArch.data());
-    }
+    StringRef MArchName = riscv::getRISCVArch(Args, getToolChain().getTriple());
+    CmdArgs.push_back("-march");
+    CmdArgs.push_back(MArchName.data());
     break;
   }
   case llvm::Triple::sparc:
diff --git a/clang/test/Driver/riscv-abi.c b/clang/test/Driver/riscv-abi.c
index 1a4c7ed477b..ef3913ee3c4 100644
--- a/clang/test/Driver/riscv-abi.c
+++ b/clang/test/Driver/riscv-abi.c
@@ -16,6 +16,10 @@
 
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd -mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-linux-gnu -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 
 // CHECK-ILP32D: "-target-abi" "ilp32d"
 
@@ -42,6 +46,10 @@
 
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D  %s
 
 // CHECK-LP64D: "-target-abi" "lp64d"
 
diff --git a/clang/test/Driver/riscv-gnutools.c b/clang/test/Driver/riscv-gnutools.c
index afcb5052aa3..b5ea737ba78 100644
--- a/clang/test/Driver/riscv-gnutools.c
+++ b/clang/test/Driver/riscv-gnutools.c
@@ -1,19 +1,40 @@
 // Check gnutools are invoked with propagated values for -mabi and -march.
+//
+// This test also checks the default -march/-mabi for certain targets.
 
-// RUN: %clang -target riscv32 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// 32-bit checks
 
-// RUN: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// Check default on riscv32-unknown-elf
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32IMAC-ILP32 %s
 
-// RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+// Check default on riscv32-unknown-linux-gnu
+// RUN: %clang -target riscv32-unknown-linux-gnu -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32IMAFDC-ILP32D %s
 
-// RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+// Check default when -march=rv32g specified
+// RUN: %clang -target riscv32 -fno-integrated-as %s -### -c -march=rv32g \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32G-ILP32D %s
 
-// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
-// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+// CHECK-RV32IMAC-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32imac"
+// CHECK-RV32IMAFDC-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32imafdc"
+// CHECK-RV32G-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
 
-// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
-// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
+
+// 64-bit checks
+
+// Check default on riscv64-unknown-elf
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64IMAC-LP64 %s
+
+// Check default on riscv64-unknown-linux-gnu
+// RUN: %clang -target riscv64-unknown-linux-gnu -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64IMAFDC-LP64D %s
+
+// Check default when -march=rv64g specified
+// RUN: %clang -target riscv64 -fno-integrated-as %s -### -c -march=rv64g \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV64G-LP64D %s
+
+// CHECK-RV64IMAC-LP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64imac"
+// CHECK-RV64IMAFDC-LP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64imafdc"
+// CHECK-RV64G-LP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"