[PATCH] D105555: [PoC][RISCV][Clang] Compute the default target-abi if it's empty.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: jrtc27, asb, luismarques.
Herald added subscribers: vkmr, frasercrmck, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar.
khchen requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Address https://reviews.llvm.org/D102582#2860849

Explicitly target-abi encoded in IR is clear than empty target-abi.

I have one more question When I was working on this feature.
In riscv::getRISCVABI, default ABI value is based on march, and the default march is base on mcpu if the march is empty.
So do we need to consider getting the default ABI according to target-cpu's default march?
If yes, does it mean we also need to update default target-feature in RISCVTargetInfo based on -target-cpu?

In clang driver, missing -march with mcpu is clear intention so we could use default march from mcpu.
But in cc1, I feel it's not clear when missing -target-feature with target-cpu option. maybe user really want to use the
default isa.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105555

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/RISCV/riscv-metadata.c


Index: clang/test/CodeGen/RISCV/riscv-metadata.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv-metadata.c
+++ clang/test/CodeGen/RISCV/riscv-metadata.c
@@ -1,14 +1,22 @@
+// RUN: %clang_cc1 -triple riscv32 -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-ILP32 %s
+// RUN: %clang_cc1 -triple riscv32 -emit-llvm -target-feature +f -target-feature +d -o - %s | FileCheck -check-prefix=EMPTY-ILP32D %s
 // RUN: %clang_cc1 -triple riscv32 -target-abi ilp32 -emit-llvm -o - %s | FileCheck -check-prefix=ILP32 %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm -o - %s | FileCheck -check-prefix=ILP32F %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +d -target-abi ilp32d -emit-llvm -o - %s | FileCheck -check-prefix=ILP32D %s
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-LP64 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-LP64D %s
 // RUN: %clang_cc1 -triple riscv64 -target-abi lp64 -emit-llvm -o - %s | FileCheck -check-prefix=LP64 %s
 // RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-abi lp64f -emit-llvm -o - %s | FileCheck -check-prefix=LP64F %s
 // RUN: %clang_cc1 -triple riscv64 -target-feature +d -target-abi lp64d -emit-llvm -o - %s | FileCheck -check-prefix=LP64D %s
 
+// EMPTY-ILP32: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32"}
+// EMPTY-ILP32D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32d"}
 // ILP32: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32"}
 // ILP32F: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32f"}
 // ILP32D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32d"}
 
+// EMPTY-LP64: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64"}
+// EMPTY-LP64D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64d"}
 // LP64: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64"}
 // LP64F: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64f"}
 // LP64D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64d"}
Index: clang/lib/Basic/Targets/RISCV.h
===================================================================
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -69,7 +69,6 @@
     return true;
   }
 
-  StringRef getABI() const override { return ABI; }
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 
@@ -129,6 +128,8 @@
     return false;
   }
 
+  StringRef getABI() const override;
+
   bool isValidCPUName(StringRef Name) const override;
   void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
@@ -158,6 +159,8 @@
     return false;
   }
 
+  StringRef getABI() const override;
+
   bool isValidCPUName(StringRef Name) const override;
   void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
Index: clang/lib/Basic/Targets/RISCV.cpp
===================================================================
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -332,6 +332,16 @@
   return true;
 }
 
+StringRef RISCV32TargetInfo::getABI() const {
+  if (!ABI.empty())
+    return ABI;
+  // Choose a default based on the target architecture.
+  // The logic sync with riscv::getRISCVABI
+  if (HasD)
+    return "ilp32d";
+  return "ilp32";
+}
+
 bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name),
                                    /*Is64Bit=*/false);
@@ -353,6 +363,16 @@
   llvm::RISCV::fillValidTuneCPUArchList(Values, false);
 }
 
+StringRef RISCV64TargetInfo::getABI() const {
+  if (!ABI.empty())
+    return ABI;
+  // Choose a default based on the target architecture.
+  // The logic sync with riscv::getRISCVABI
+  if (HasD)
+    return "lp64d";
+  return "lp64";
+}
+
 bool RISCV64TargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name),
                                    /*Is64Bit=*/true);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105555.356970.patch
Type: text/x-patch
Size: 4155 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210707/01efc259/attachment.bin>