[PATCH] D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple.

[Craig Topper via Phabricator via llvm-commits]

craig.topper updated this revision to Diff 330046.
craig.topper added a comment.

Go back to my original patch plus the std::string cleanup. Add explanation about why I've done it this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98307/new/

https://reviews.llvm.org/D98307

Files:
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp


Index: llvm/lib/Target/RISCV/RISCVSubtarget.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -47,17 +47,26 @@
 
 void RISCVSubtarget::anchor() {}
 
-RISCVSubtarget &RISCVSubtarget::initializeSubtargetDependencies(
-    const Triple &TT, StringRef CPU, StringRef TuneCPU, StringRef FS, StringRef ABIName) {
+RISCVSubtarget &
+RISCVSubtarget::initializeSubtargetDependencies(const Triple &TT, StringRef CPU,
+                                                StringRef TuneCPU, StringRef FS,
+                                                StringRef ABIName) {
   // Determine default and user-specified characteristics
   bool Is64Bit = TT.isArch64Bit();
-  std::string CPUName = std::string(CPU);
-  std::string TuneCPUName = std::string(TuneCPU);
-  if (CPUName.empty())
-    CPUName = Is64Bit ? "generic-rv64" : "generic-rv32";
-  if (TuneCPUName.empty())
-    TuneCPUName = CPUName;
-  ParseSubtargetFeatures(CPUName, TuneCPUName, FS);
+  // Remap "generic" to a RV32/RV64 specific name. "generic" is supported by
+  // most targets, outside users may assume it works with RISCV.
+  // NOTE: A warning will still be printed about the CPU name being ignored
+  // because the MCSubtargetInfo constructor runs before this. Changing the
+  // name serves to make sure that the 64Bit feature gets set correctly so
+  // we can pass the RISCVFeatures::validate check. We could do this with
+  // other CPU names that have rv32/rv64 versions, but that's misleading
+  // because the message says we ignored the CPU.
+  if (CPU.empty() || CPU == "generic")
+    CPU = Is64Bit ? "generic-rv64" : "generic-rv32";
+  if (TuneCPU.empty())
+    TuneCPU = CPU;
+
+  ParseSubtargetFeatures(CPU, TuneCPU, FS);
   if (Is64Bit) {
     XLenVT = MVT::i64;
     XLen = 64;
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
===================================================================
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
@@ -65,7 +65,7 @@
 static MCSubtargetInfo *createRISCVMCSubtargetInfo(const Triple &TT,
                                                    StringRef CPU, StringRef FS) {
   std::string CPUName = std::string(CPU);
-  if (CPUName.empty())
+  if (CPUName.empty() || CPUName == "generic")
     CPUName = TT.isArch64Bit() ? "generic-rv64" : "generic-rv32";
   return createRISCVMCSubtargetInfoImpl(TT, CPUName, /*TuneCPU*/ CPUName, FS);
 }
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
===================================================================
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
@@ -88,6 +88,10 @@
 namespace RISCVFeatures {
 
 void validate(const Triple &TT, const FeatureBitset &FeatureBits) {
+  if (TT.isArch64Bit() && !FeatureBits[RISCV::Feature64Bit])
+    report_fatal_error("RV64 target requires an RV64 CPU");
+  if (!TT.isArch64Bit() && FeatureBits[RISCV::Feature64Bit])
+    report_fatal_error("RV32 target requires an RV32 CPU");
   if (TT.isArch64Bit() && FeatureBits[RISCV::FeatureRV32E])
     report_fatal_error("RV32E can't be enabled for an RV64 target");
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98307.330046.patch
Type: text/x-patch
Size: 3300 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210311/dcc75107/attachment.bin>