[PATCH] D83588: [TableGen][CGS] Print better errors on overlapping InstRW

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs updated this revision to Diff 279600.
jroelofs added a comment.

- Add back the "also matches previous" part of the diagnostics.
- Fix formatting that the linter complains about.

Started a thread here [1] to resolve the difference in preference re: notes vs all-in-one-line errors.

1: http://lists.llvm.org/pipermail/llvm-dev/2020-July/143597.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83588

Files:
  llvm/include/llvm/TableGen/Error.h
  llvm/lib/TableGen/Error.cpp
  llvm/utils/TableGen/CodeGenSchedule.cpp


Index: llvm/utils/TableGen/CodeGenSchedule.cpp
===================================================================
--- llvm/utils/TableGen/CodeGenSchedule.cpp
+++ llvm/utils/TableGen/CodeGenSchedule.cpp
@@ -248,8 +248,7 @@
     }
 
     PrintError(R->getLoc(), "STIPredicate " + Name + " multiply declared.");
-    PrintNote(It->second->getLoc(), "Previous declaration was here.");
-    PrintFatalError(R->getLoc(), "Invalid STIPredicateDecl found.");
+    PrintFatalNote(It->second->getLoc(), "Previous declaration was here.");
   }
 
   // Disallow InstructionEquivalenceClasses with an empty instruction list.
@@ -454,10 +453,8 @@
 
     PrintError(TIIPred->getLoc(),
                "TIIPredicate " + Name + " is multiply defined.");
-    PrintNote(It->second->getLoc(),
-              " Previous definition of " + Name + " was here.");
-    PrintFatalError(TIIPred->getLoc(),
-                    "Found conflicting definitions of TIIPredicate.");
+    PrintFatalNote(It->second->getLoc(),
+                   " Previous definition of " + Name + " was here.");
   }
 }
 
@@ -1083,13 +1080,13 @@
             if (RWD->getValueAsDef("SchedModel") == RWModelDef &&
                 RWModelDef->getValueAsBit("FullInstRWOverlapCheck")) {
               assert(!InstDefs.empty()); // Checked at function start.
-              PrintFatalError
-                  (InstRWDef->getLoc(),
-                   "Overlapping InstRW definition for \"" +
-                   InstDefs.front()->getName() +
-                   "\" also matches previous \"" +
-                   RWD->getValue("Instrs")->getValue()->getAsString() +
-                   "\".");
+              PrintError(InstRWDef->getLoc(),
+                         "Overlapping InstRW definition for \"" +
+                         InstDefs.front()->getName() +
+                         "\" also matches previous \"" +
+                         RWD->getValue("Instrs")->getValue()->getAsString() +
+                         "\".");
+              PrintFatalNote(RWD->getLoc(), "Previous match was here.");
             }
           }
           LLVM_DEBUG(dbgs() << "InstRW: Reuse SC " << OldSCIdx << ":"
@@ -1118,13 +1115,13 @@
       for (Record *OldRWDef : SchedClasses[OldSCIdx].InstRWs) {
         if (OldRWDef->getValueAsDef("SchedModel") == RWModelDef) {
           assert(!InstDefs.empty()); // Checked at function start.
-          PrintFatalError
-              (InstRWDef->getLoc(),
-               "Overlapping InstRW definition for \"" +
-               InstDefs.front()->getName() +
-               "\" also matches previous \"" +
-               OldRWDef->getValue("Instrs")->getValue()->getAsString() +
-               "\".");
+          PrintError(InstRWDef->getLoc(),
+                     "Overlapping InstRW definition for \"" +
+                     InstDefs.front()->getName() +
+                     "\" also matches previous \"" +
+                     OldRWDef->getValue("Instrs")->getValue()->getAsString() +
+                     "\".");
+          PrintFatalNote(OldRWDef->getLoc(), "Previous match was here.");
         }
         assert(OldRWDef != InstRWDef &&
                "SchedClass has duplicate InstRW def");
Index: llvm/lib/TableGen/Error.cpp
===================================================================
--- llvm/lib/TableGen/Error.cpp
+++ llvm/lib/TableGen/Error.cpp
@@ -45,6 +45,13 @@
   PrintMessage(NoteLoc, SourceMgr::DK_Note, Msg);
 }
 
+void PrintFatalNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
+  PrintNote(NoteLoc, Msg);
+  // The following call runs the file cleanup handlers.
+  sys::RunInterruptHandlers();
+  std::exit(1);
+}
+
 void PrintWarning(ArrayRef<SMLoc> WarningLoc, const Twine &Msg) {
   PrintMessage(WarningLoc, SourceMgr::DK_Warning, Msg);
 }
Index: llvm/include/llvm/TableGen/Error.h
===================================================================
--- llvm/include/llvm/TableGen/Error.h
+++ llvm/include/llvm/TableGen/Error.h
@@ -20,6 +20,8 @@
 
 void PrintNote(const Twine &Msg);
 void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg);
+LLVM_ATTRIBUTE_NORETURN void PrintFatalNote(ArrayRef<SMLoc> ErrorLoc,
+                                            const Twine &Msg);
 
 void PrintWarning(ArrayRef<SMLoc> WarningLoc, const Twine &Msg);
 void PrintWarning(const char *Loc, const Twine &Msg);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83588.279600.patch
Type: text/x-patch
Size: 4339 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200721/6fa59bd9/attachment.bin>