[llvm] [TableGen] Add PrintError family overload that take a print function (PR #107333)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 5 14:51:22 PDT 2024


https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/107333

>From 1c9380586aeef639fde154b9c16d21f5fbba3e63 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Wed, 4 Sep 2024 14:04:38 -0700
Subject: [PATCH] [TableGen] Add PrintError family overload that take a print
 function

Add PrintError and family overload that accepts a print function.
This avoids constructing potentially long strings for passing into
these print functions.
---
 llvm/include/llvm/TableGen/Error.h            |  8 ++-
 llvm/lib/TableGen/Error.cpp                   | 51 ++++++++++---------
 llvm/test/TableGen/intrinsic-prefix-error.td  | 14 +++++
 .../TableGen/Common/CodeGenDAGPatterns.cpp    | 17 +++----
 llvm/utils/TableGen/DecoderEmitter.cpp        | 11 ++--
 llvm/utils/TableGen/IntrinsicEmitter.cpp      | 20 ++++----
 6 files changed, 70 insertions(+), 51 deletions(-)
 create mode 100644 llvm/test/TableGen/intrinsic-prefix-error.td

diff --git a/llvm/include/llvm/TableGen/Error.h b/llvm/include/llvm/TableGen/Error.h
index 04618995e0fe6d..a51ed48dba0e86 100644
--- a/llvm/include/llvm/TableGen/Error.h
+++ b/llvm/include/llvm/TableGen/Error.h
@@ -14,12 +14,16 @@
 #ifndef LLVM_TABLEGEN_ERROR_H
 #define LLVM_TABLEGEN_ERROR_H
 
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/SourceMgr.h"
-#include "llvm/TableGen/Record.h"
 
 namespace llvm {
+class Record;
+class RecordVal;
+class Init;
 
 void PrintNote(const Twine &Msg);
+void PrintNote(function_ref<void(raw_ostream &OS)> PrintMsg);
 void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg);
 
 [[noreturn]] void PrintFatalNote(const Twine &Msg);
@@ -32,6 +36,7 @@ void PrintWarning(ArrayRef<SMLoc> WarningLoc, const Twine &Msg);
 void PrintWarning(const char *Loc, const Twine &Msg);
 
 void PrintError(const Twine &Msg);
+void PrintError(function_ref<void(raw_ostream &OS)> PrintMsg);
 void PrintError(ArrayRef<SMLoc> ErrorLoc, const Twine &Msg);
 void PrintError(const char *Loc, const Twine &Msg);
 void PrintError(const Record *Rec, const Twine &Msg);
@@ -41,6 +46,7 @@ void PrintError(const RecordVal *RecVal, const Twine &Msg);
 [[noreturn]] void PrintFatalError(ArrayRef<SMLoc> ErrorLoc, const Twine &Msg);
 [[noreturn]] void PrintFatalError(const Record *Rec, const Twine &Msg);
 [[noreturn]] void PrintFatalError(const RecordVal *RecVal, const Twine &Msg);
+[[noreturn]] void PrintFatalError(function_ref<void(raw_ostream &OS)> PrintMsg);
 
 void CheckAssert(SMLoc Loc, Init *Condition, Init *Message);
 void dumpMessage(SMLoc Loc, Init *Message);
diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp
index 4b99ff89da3b93..260d66d7b298fe 100644
--- a/llvm/lib/TableGen/Error.cpp
+++ b/llvm/lib/TableGen/Error.cpp
@@ -40,12 +40,22 @@ static void PrintMessage(ArrayRef<SMLoc> Loc, SourceMgr::DiagKind Kind,
                         "instantiated from multiclass");
 }
 
+[[noreturn]] inline static void exit() {
+  // The following call runs the file cleanup handlers.
+  sys::RunInterruptHandlers();
+  std::exit(1);
+}
+
 // Functions to print notes.
 
 void PrintNote(const Twine &Msg) {
   WithColor::note() << Msg << "\n";
 }
 
+void PrintNote(function_ref<void(raw_ostream &OS)> PrintMsg) {
+  PrintMsg(WithColor::note());
+}
+
 void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
   PrintMessage(NoteLoc, SourceMgr::DK_Note, Msg);
 }
@@ -54,34 +64,26 @@ void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
 
 void PrintFatalNote(const Twine &Msg) {
   PrintNote(Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 void PrintFatalNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
   PrintNote(NoteLoc, Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // This method takes a Record and uses the source location
 // stored in it.
 void PrintFatalNote(const Record *Rec, const Twine &Msg) {
   PrintNote(Rec->getLoc(), Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // This method takes a RecordVal and uses the source location
 // stored in it.
 void PrintFatalNote(const RecordVal *RecVal, const Twine &Msg) {
   PrintNote(RecVal->getLoc(), Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // Functions to print warnings.
@@ -100,6 +102,10 @@ void PrintWarning(const char *Loc, const Twine &Msg) {
 
 void PrintError(const Twine &Msg) { WithColor::error() << Msg << "\n"; }
 
+void PrintError(function_ref<void(raw_ostream &OS)> PrintMsg) {
+  PrintMsg(WithColor::error());
+}
+
 void PrintError(ArrayRef<SMLoc> ErrorLoc, const Twine &Msg) {
   PrintMessage(ErrorLoc, SourceMgr::DK_Error, Msg);
 }
@@ -124,34 +130,31 @@ void PrintError(const RecordVal *RecVal, const Twine &Msg) {
 
 void PrintFatalError(const Twine &Msg) {
   PrintError(Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
+}
+
+void PrintFatalError(function_ref<void(raw_ostream &OS)> PrintMsg) {
+  PrintError(PrintMsg);
+  exit();
 }
 
 void PrintFatalError(ArrayRef<SMLoc> ErrorLoc, const Twine &Msg) {
   PrintError(ErrorLoc, Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // This method takes a Record and uses the source location
 // stored in it.
 void PrintFatalError(const Record *Rec, const Twine &Msg) {
   PrintError(Rec->getLoc(), Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // This method takes a RecordVal and uses the source location
 // stored in it.
 void PrintFatalError(const RecordVal *RecVal, const Twine &Msg) {
   PrintError(RecVal->getLoc(), Msg);
-  // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
-  std::exit(1);
+  exit();
 }
 
 // Check an assertion: Obtain the condition value and be sure it is true.
diff --git a/llvm/test/TableGen/intrinsic-prefix-error.td b/llvm/test/TableGen/intrinsic-prefix-error.td
new file mode 100644
index 00000000000000..3869bde3a4ba76
--- /dev/null
+++ b/llvm/test/TableGen/intrinsic-prefix-error.td
@@ -0,0 +1,14 @@
+// RUN: not llvm-tblgen -gen-intrinsic-enums --intrinsic-prefix=gen3 -I %p/../../include %s -DTEST_INTRINSICS_SUPPRESS_DEFS 2>&1 | FileCheck %s
+
+include "llvm/IR/Intrinsics.td"
+
+// CHECK: error: tried to generate intrinsics for unknown target gen3
+// CHECK-NEXT: Known targets are: gen1, gen2
+
+let TargetPrefix = "gen1" in {
+  def int_gen1_int0 : Intrinsic<[llvm_i32_ty]>;
+}
+
+let TargetPrefix = "gen2" in {
+  def int_gen2_int0 : Intrinsic<[llvm_i32_ty]>;
+}
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index df3f72ff2ec7f5..2c5b88b5b9ead5 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -1600,18 +1600,13 @@ static TreePatternNode &getOperandNum(unsigned OpNo, TreePatternNode &N,
     return N;
   }
 
-  OpNo -= NumResults;
-
-  if (OpNo >= N.getNumChildren()) {
-    std::string S;
-    raw_string_ostream OS(S);
-    OS << "Invalid operand number in type constraint " << (OpNo + NumResults)
-       << " ";
-    N.print(OS);
-    PrintFatalError(S);
+  if (OpNo - NumResults >= N.getNumChildren()) {
+    PrintFatalError([&N, OpNo](raw_ostream &OS) {
+      OS << "Invalid operand number " << OpNo << " in type constraint ";
+      N.print(OS);
+    });
   }
-
-  return N.getChild(OpNo);
+  return N.getChild(OpNo - NumResults);
 }
 
 /// ApplyTypeConstraint - Given a node in a pattern, apply this type
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index f46a8d1e9f081d..e367fad5838aae 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -1961,12 +1961,11 @@ void parseVarLenInstOperand(const Record &Def,
 }
 
 static void debugDumpRecord(const Record &Rec) {
-  // Dump the record, so we can see what's going on...
-  std::string E;
-  raw_string_ostream S(E);
-  S << "Dumping record for previous error:\n";
-  S << Rec;
-  PrintNote(E);
+  // Dump the record, so we can see what's going on.
+  PrintNote([&Rec](raw_ostream &OS) {
+    OS << "Dumping record for previous error:\n";
+    OS << Rec;
+  });
 }
 
 /// For an operand field named OpName: populate OpInfo.InitValue with the
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index bda97c61d3d581..71a6ecd0b4882a 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -14,7 +14,6 @@
 #include "Basic/SequenceToOffsetTable.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
@@ -122,7 +121,8 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
   // Find the TargetSet for which to generate enums. There will be an initial
   // set with an empty target prefix which will include target independent
   // intrinsics like dbg.value.
-  const CodeGenIntrinsicTable::TargetSet *Set = nullptr;
+  using TargetSet = CodeGenIntrinsicTable::TargetSet;
+  const TargetSet *Set = nullptr;
   for (const auto &Target : Ints.Targets) {
     if (Target.Name == IntrinsicPrefix) {
       Set = &Target;
@@ -130,13 +130,15 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
     }
   }
   if (!Set) {
-    std::vector<std::string> KnownTargets;
-    for (const auto &Target : Ints.Targets)
-      if (!Target.Name.empty())
-        KnownTargets.push_back(Target.Name.str());
-    PrintFatalError("tried to generate intrinsics for unknown target " +
-                    IntrinsicPrefix +
-                    "\nKnown targets are: " + join(KnownTargets, ", ") + "\n");
+    // The first entry is for target independent intrinsics, so drop it.
+    auto KnowTargets = ArrayRef<TargetSet>(Ints.Targets).drop_front();
+    PrintFatalError([KnowTargets](raw_ostream &OS) {
+      OS << "tried to generate intrinsics for unknown target "
+         << IntrinsicPrefix << "\nKnown targets are: ";
+      interleaveComma(KnowTargets, OS,
+                      [&OS](const TargetSet &Target) { OS << Target.Name; });
+      OS << '\n';
+    });
   }
 
   // Generate a complete header for target specific intrinsics.



More information about the llvm-commits mailing list