[TableGen][RISCV] Use getAllDerivedDefinitionsIfDefined in RISCVTargetDefEmitter

[asb]

getAllDerivedDefinitions produces a fatal error if there are no definitions. In practice this isn't much of a problem for llvm/lib/Target/RISCV/*.td where it's hard to imagine not having at least one of the required defitions. But it limits our ability to structure and maintain tests (which is how I came across this issue).

This commit moves to using getAllDerivedDefinitionsIfDefined and aims to skip emission of data structures that makes no sense if no definitions were defined.

---

If desired I can add more testing to try to carefully capture the change in output, but as I'm about to get some coverage for this behaviour in a follow-up patch I'm not sure it's worthwhile (or consistent with the level of testing we tend to give to tablegen emitters).

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Alex Bradbury (asb)

Changes

getAllDerivedDefinitions produces a fatal error if there are no definitions. In practice this isn't much of a problem for llvm/lib/Target/RISCV/*.td where it's hard to imagine not having at least one of the required defitions. But it limits our ability to structure and maintain tests (which is how I came across this issue).

This commit moves to using getAllDerivedDefinitionsIfDefined and aims to skip emission of data structures that makes no sense if no definitions were defined.

---

If desired I can add more testing to try to carefully capture the change in output, but as I'm about to get some coverage for this behaviour in a follow-up patch I'm not sure it's worthwhile (or consistent with the level of testing we tend to give to tablegen emitters).

---

Full diff: https://github.com/llvm/llvm-project/pull/91941.diff

1 Files Affected:

• (modified) llvm/utils/TableGen/RISCVTargetDefEmitter.cpp (+33-27)

diff --git a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
index 6784514032eb3..bb409ea6ea692 100644
--- a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
+++ b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -48,37 +48,41 @@ static void emitRISCVExtensions(RecordKeeper &Records, raw_ostream &OS) {
   OS << "#undef GET_SUPPORTED_EXTENSIONS\n\n";
 
   std::vector<Record *> Extensions =
-      Records.getAllDerivedDefinitions("RISCVExtension");
+      Records.getAllDerivedDefinitionsIfDefined("RISCVExtension");
   llvm::sort(Extensions, [](const Record *Rec1, const Record *Rec2) {
     return getExtensionName(Rec1) < getExtensionName(Rec2);
   });
 
-  printExtensionTable(OS, Extensions, /*Experimental=*/false);
-  printExtensionTable(OS, Extensions, /*Experimental=*/true);
+  if (!Extensions.empty()) {
+    printExtensionTable(OS, Extensions, /*Experimental=*/false);
+    printExtensionTable(OS, Extensions, /*Experimental=*/true);
+  }
 
   OS << "#endif // GET_SUPPORTED_EXTENSIONS\n\n";
 
   OS << "#ifdef GET_IMPLIED_EXTENSIONS\n";
   OS << "#undef GET_IMPLIED_EXTENSIONS\n\n";
 
-  OS << "\nstatic constexpr ImpliedExtsEntry ImpliedExts[] = {\n";
-  for (Record *Ext : Extensions) {
-    auto ImpliesList = Ext->getValueAsListOfDefs("Implies");
-    if (ImpliesList.empty())
-      continue;
+  if (!Extensions.empty()) {
+    OS << "\nstatic constexpr ImpliedExtsEntry ImpliedExts[] = {\n";
+    for (Record *Ext : Extensions) {
+      auto ImpliesList = Ext->getValueAsListOfDefs("Implies");
+      if (ImpliesList.empty())
+        continue;
 
-    StringRef Name = getExtensionName(Ext);
+      StringRef Name = getExtensionName(Ext);
 
-    for (auto *ImpliedExt : ImpliesList) {
-      if (!ImpliedExt->isSubClassOf("RISCVExtension"))
-        continue;
+      for (auto *ImpliedExt : ImpliesList) {
+        if (!ImpliedExt->isSubClassOf("RISCVExtension"))
+          continue;
 
-      OS << "    { {\"" << Name << "\"}, \"" << getExtensionName(ImpliedExt)
-         << "\"},\n";
+        OS << "    { {\"" << Name << "\"}, \"" << getExtensionName(ImpliedExt)
+           << "\"},\n";
+      }
     }
-  }
 
-  OS << "};\n\n";
+    OS << "};\n\n";
+  }
 
   OS << "#endif // GET_IMPLIED_EXTENSIONS\n\n";
 }
@@ -122,19 +126,20 @@ static void emitRISCVProfiles(RecordKeeper &Records, raw_ostream &OS) {
   OS << "#ifdef GET_SUPPORTED_PROFILES\n";
   OS << "#undef GET_SUPPORTED_PROFILES\n\n";
 
-  OS << "static constexpr RISCVProfile SupportedProfiles[] = {\n";
+  auto Profiles = Records.getAllDerivedDefinitionsIfDefined("RISCVProfile");
 
-  auto Profiles = Records.getAllDerivedDefinitions("RISCVProfile");
-  llvm::sort(Profiles, LessRecordFieldName());
+  if (!Profiles.empty()) {
+    llvm::sort(Profiles, LessRecordFieldName());
+    OS << "static constexpr RISCVProfile SupportedProfiles[] = {\n";
+    for (const Record *Rec : Profiles) {
+      OS.indent(4) << "{\"" << Rec->getValueAsString("Name") << "\",\"";
+      printMArch(OS, Rec->getValueAsListOfDefs("Implies"));
+      OS << "\"},\n";
+    }
 
-  for (const Record *Rec : Profiles) {
-    OS.indent(4) << "{\"" << Rec->getValueAsString("Name") << "\",\"";
-    printMArch(OS, Rec->getValueAsListOfDefs("Implies"));
-    OS << "\"},\n";
+    OS << "};\n\n";
   }
 
-  OS << "};\n\n";
-
   OS << "#endif // GET_SUPPORTED_PROFILES\n\n";
 }
 
@@ -144,7 +149,8 @@ static void emitRISCVProcs(RecordKeeper &RK, raw_ostream &OS) {
      << "#endif\n\n";
 
   // Iterate on all definition records.
-  for (const Record *Rec : RK.getAllDerivedDefinitions("RISCVProcessorModel")) {
+  for (const Record *Rec :
+       RK.getAllDerivedDefinitionsIfDefined("RISCVProcessorModel")) {
     const std::vector<Record *> &Features =
         Rec->getValueAsListOfDefs("Features");
     bool FastScalarUnalignedAccess = any_of(Features, [&](auto &Feature) {
@@ -177,7 +183,7 @@ static void emitRISCVProcs(RecordKeeper &RK, raw_ostream &OS) {
      << "#endif\n\n";
 
   for (const Record *Rec :
-       RK.getAllDerivedDefinitions("RISCVTuneProcessorModel")) {
+       RK.getAllDerivedDefinitionsIfDefined("RISCVTuneProcessorModel")) {
     OS << "TUNE_PROC(" << Rec->getName() << ", "
        << "\"" << Rec->getValueAsString("Name") << "\")\n";
   }

[lukel97]

I'm not familiar with RISCVTargetDefEmitter.cpp, but makes sense to me. I presume this is so we can test extensions/profiles/CPUs independently?

[asb]

I'm not familiar with RISCVTargetDefEmitter.cpp, but makes sense to me. I presume this is so we can test extensions/profiles/CPUs independently?

Yes, writing a standalone test is what raised this issue for me. The precise testing scenario is related to what we were discussing offline about zero-length arrays when there are no experimental extensions or (when I add it) experimental profiles and a bit fiddly to explain here. It's possible the precise tests I was going to do don't make sense in the end, but I'd suggest this change probably still makes sense in isolation.

[topperc]

LGTM