Improve diagnostics for instruction mapping

[abeserminji]

Summary

While mapping instructions for microMIPS, I made an error in capitalization of one field which led to a segfault. This patch improves diagnostic for this case, displaying reasonable message instead of segfault.

[sdardis]

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

test/TableGen/RelTest.td

3 ↗

(On Diff #128769)

Add a brief description of the purpose of this test.

utils/TableGen/CodeGenMapTable.cpp

246 ↗

(On Diff #128769)

recVal -> RecVal.

248 ↗

(On Diff #128769)

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.

[abeserminji]

Comments addressed.

llvm/trunk/test/TableGen/RelTest.td

+// RUN: not llvm-tblgen -gen-instr-info -I %p/../../include %s 2>&1 | FileCheck %s
+
+// This test verifies that TableGen is displaying an error when mapped instruction
+// does not contain a field listed under RowFields.
+
+include "llvm/Target/Target.td"
+
+class SimpleReg<string n> : Register<n> {
+  let Namespace = "Simple";
+}
+def R0 : SimpleReg<"r0">;
+def SimpleRegClass : RegisterClass<"Simple",[i32],0,(add R0)>;
+def SimpleInstrInfo : InstrInfo;
+
+def SimpleTarget : Target {
+  let InstructionSet = SimpleInstrInfo;
+}
+
+class SimpleRel;
+
+def REL_DEF : InstrMapping {
+  let FilterClass = "SimpleRel";
+  let RowFields = ["BaseName"];
+  let ColFields = ["Col"];
+  let KeyCol = ["KeyCol"];
+  let ValueCols = [["ValCol"]];
+}
+
+class INSTR_DEF : Instruction {
+  let Namespace = "Simple";
+  let OutOperandList = (outs);
+  let InOperandList = (ins);
+  string Basename = "";
+  string Col = "";
+}
+
+def SimpleInstr : SimpleRel, INSTR_DEF;
+
+// CHECK: error: No value "BaseName" found in "SimpleInstr" instruction description.
+// CHECK: def SimpleInstr : SimpleRel, INSTR_DEF;

llvm/trunk/utils/TableGen/CodeGenMapTable.cpp

 // part of the same key-value pair.
 //===----------------------------------------------------------------------===//
 
 void MapTableEmitter::buildRowInstrMap() {
   for (Record *CurInstr : InstrDefs) {
     std::vector<Init*> KeyValue;
     ListInit *RowFields = InstrMapDesc.getRowFields();
     for (Init *RowField : RowFields->getValues()) {
-      Init *CurInstrVal = CurInstr->getValue(RowField)->getValue();
+      RecordVal *RecVal = CurInstr->getValue(RowField);
+      if (RecVal == nullptr)
+        PrintFatalError(CurInstr->getLoc(), "No value " +
+                        RowField->getAsString() + " found in \"" +
+                        CurInstr->getName() + "\" instruction description.");
+      Init *CurInstrVal = RecVal->getValue();
       KeyValue.push_back(CurInstrVal);
     }
 
     // Collect key instructions into KeyInstrVec. Later, these instructions are
     // processed to assign column position to the instructions sharing
     // their KeyValue in RowInstrMap.
     if (isKeyColInstr(CurInstr))
       KeyInstrVec.push_back(CurInstr);