This is an archive of the discontinued LLVM Phabricator instance.

[mips][ias] Range check uimm2 operands and fix a bug this revealed.
ClosedPublic

Authored by dsanders on Oct 23 2015, 2:45 AM.

Details

Summary

The bug was that the MIPS32R6/MIPS64R6/microMIPS32R6 versions of LSA and DLSA
(unlike the MSA version) failed to account for the off-by-one encoding of the
immediate. The range is actually 1..4 rather than 0..3.

Depends on D14013

Diff Detail

Event Timeline

dsanders updated this revision to Diff 38222.Oct 23 2015, 2:45 AM
dsanders retitled this revision from to [mips][ias] Range check uimm2 operands and fix a bug this revealed..
dsanders updated this object.
dsanders added a reviewer: vkalintiris.
dsanders added a subscriber: llvm-commits.
dsanders updated this revision to Diff 38223.Oct 23 2015, 2:56 AM

Forgot to update the superclasses list for uimmz. The tests passed anyway but
this ensures that matchables are attempted in the right order.

vkalintiris accepted this revision.Oct 26 2015, 8:49 AM
vkalintiris edited edge metadata.

LGTM, but make sure that you really want the 2nd assert in getUImmWithOffsetEncoding().

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
3297–3300

Split the Error() lines as they are too big.

lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp
880

Do we really need this assert in this place?

lib/Target/Mips/MipsInstrInfo.td
475

Did you intend to use foreach over a single element list?

This revision is now accepted and ready to land.Oct 26 2015, 8:49 AM
vkalintiris added inline comments.Oct 26 2015, 9:10 AM
lib/Target/Mips/MipsInstrInfo.td
475

Never mind, I saw the review request you've marked as a dependency.

dsanders closed this revision.Nov 6 2015, 4:24 AM