[PATCH] D41778: Improve diagnostics for instruction mapping

Simon Dardis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 6 09:20:21 PST 2018

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

Some small nits inlined, otherwise LGTM. Please wait a few working days to let others chime in if they have suggestions before committing.

Comment at: test/TableGen/RelTest.td:3
+include "llvm/Target/Target.td"
Add a brief description of the purpose of this test. 

Comment at: utils/TableGen/CodeGenMapTable.cpp:246
     for (Init *RowField : RowFields->getValues()) {
-      Init *CurInstrVal = CurInstr->getValue(RowField)->getValue();
+      RecordVal *recVal = CurInstr->getValue(RowField);
+      if (recVal == nullptr)
recVal -> RecVal.

Comment at: utils/TableGen/CodeGenMapTable.cpp:248
+      if (recVal == nullptr)
+        PrintFatalError("No value " + RowField->getAsString() + " found in \"" +
+                        CurInstr->getName() + "\" instruction description.");
I believe you should use the overloaded variant of PrintFatalError that takes a source location of a record. i.e:

        PrintFatalError(CurInstr->getLoc(), "No value " + RowField->getAsString() + " found in \"" +
                        CurInstr->getName() + "\" instruction description.");

This gives the following error message:

   /Users/simon/dev/llvm/llvm/test/TableGen/RelTest.td:34:1: error: No value "BaseName" found in "SimpleInstr" instruction description.
   def SimpleInstr : SimpleRel, INSTR_DEF;

Which is a better in my opinion as directly points to the record which lacks the named value.



More information about the llvm-commits mailing list