This is an archive of the discontinued LLVM Phabricator instance.

[mips] Enable the mnemonic spell corrector
ClosedPublic

Authored by atanasyan on Nov 30 2017, 4:35 AM.

Details

Summary

This change implements suggesting alternative mnemonics when an invalid one is specified. For example addru $9, $6, 17767 leads to the following error message:

error: unknown instruction, did you mean: add, addiu, addu, maddu?

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan created this revision.Nov 30 2017, 4:35 AM
sdardis edited edge metadata.Nov 30 2017, 10:46 AM
sdardis added a subscriber: SjoerdMeijer.

I looked at enabling this when @SjoerdMeijer developed the spell checker in the first place but hit upon a nasty error case:

llvmgitsvnbuild $cat test-spellcheck.s
  swk $3, $4
llvmgitsvnbuild $./bin/llvm-mc -arch=mips -mcpu=mips32r6 test-spellcheck.s -o - -mattr=+micromips
  .text
test-spellcheck.s:1:3: error: unknown instruction, did you mean: sw, swe, swl, swm, swp, swr, usw?
  swk $3, $4
  ^

The swe there is potentially refering to the micromips or the micromipsr6 version. swl and swr should not be on the list.

There's a number of instructions which don't have the correct predicates, so the spelling checker will deliver the wrong results, as the set of possible corrections is dependant on the feature bits that are enabled.

I have been working on a patch set to correct / rework the instruction predicates which I should start posting next week.

I think we should hold off on enabling this until the instruction predicates have been fixed.

test/MC/Mips/invalid-instructions-spellcheck.s
2 ↗(On Diff #124913)

This does not require -show-encoding as the test does not actually check any instruction encodings.

The swe there is potentially refering to the micromips or the micromipsr6 version. swl and swr should not be on the list.

There's a number of instructions which don't have the correct predicates, so the spelling checker will deliver the wrong results, as the set of possible corrections is dependant on the feature bits that are enabled.

I have been working on a patch set to correct / rework the instruction predicates which I should start posting next week.

I think we should hold off on enabling this until the instruction predicates have been fixed.

Good point. Let's wait for the instruction predicates fixing.

Does it have a sense to continue to work on this patch now?

Not at the moment, but don't abandon this patch. Over the next week or so I'll be uploading more of patches which fix various incorrect predicates in the MIPS backend.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 13 2018, 1:39 AM
This revision was automatically updated to reflect the committed changes.