This is an archive of the discontinued LLVM Phabricator instance.

[mips][ias] Range check uimm5 operands and fix several bugs this revealed.
ClosedPublic

Authored by dsanders on Nov 16 2015, 9:30 AM.

Details

Summary

The bugs were:

  • append, prepend, and balign were not tested
  • balign takes a uimm2 not a uimm5.
  • drotr32 was correctly implemented with a uimm5 but the tests expected '52' to be valid.
  • li/la were implemented with a uimm5 instead of simm32. simm32 isn't completely correct either but I'll fix that when I get to simm32.

A notable omission are some of the shift instructions. Several of these
have been implemented using a single uimm6 instruction (rather than two
uimm5 instructions and a CodeGen-only uimm6 pseudo). These will be updated
in the uimm6 patch.

Diff Detail

Event Timeline

dsanders updated this revision to Diff 40306.Nov 16 2015, 9:30 AM
dsanders retitled this revision from to [mips][ias] Range check uimm5 operands and fix several bugs this revealed..
dsanders updated this object.
dsanders added a reviewer: vkalintiris.
dsanders added a subscriber: llvm-commits.
vkalintiris accepted this revision.Nov 23 2015, 7:24 AM
vkalintiris edited edge metadata.

LGTM.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1031

I don't think we need isImm() because isConstantImm() calls it internally.

This revision is now accepted and ready to land.Nov 23 2015, 7:24 AM
dsanders added inline comments.Nov 26 2015, 3:27 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1031

I agree. I'll fix this in the commit

dsanders closed this revision.Nov 26 2015, 8:38 AM