This is an archive of the discontinued LLVM Phabricator instance.

Improve diagnostics for instruction mapping
ClosedPublic

Authored by abeserminji on Jan 5 2018, 10:13 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

abeserminji created this revision.Jan 5 2018, 10:13 AM
sdardis accepted this revision.Jan 6 2018, 9:20 AM

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.

This revision is now accepted and ready to land.Jan 6 2018, 9:20 AM

Comments addressed.

abeserminji marked 3 inline comments as done.Jan 8 2018, 6:29 AM
This revision was automatically updated to reflect the committed changes.