This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement LBU16, LHU16, LW16, SB16, SH16 and SW16 instructions
ClosedPublic

Authored by jkolek on Aug 29 2014, 8:04 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

jkolek updated this revision to Diff 13090.Aug 29 2014, 8:04 AM
jkolek retitled this revision from to [mips][microMIPS] Implement LHU16, LW16, SB16, SH16 and SW16 instructions.
jkolek updated this object.
jkolek edited the test plan for this revision. (Show Details)
jkolek added reviewers: dsanders, vmedic.
jkolek added a subscriber: zoran.jovanovic.
jkolek updated this revision to Diff 13448.Sep 9 2014, 3:27 AM
jkolek retitled this revision from [mips][microMIPS] Implement LHU16, LW16, SB16, SH16 and SW16 instructions to [mips][microMIPS] Implement LBU16, LHU16, LW16, SB16, SH16 and SW16 instructions.
jkolek updated this object.

Added instruction LBU16. Fixed offset shifting for the instructions SBU16, SHU16, LHU16.

jkolek updated this revision to Diff 14272.Oct 1 2014, 4:37 AM
jkolek added a reviewer: sstankovic.
sstankovic added inline comments.Oct 3 2014, 8:00 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1174 ↗(On Diff #14272)

Allowed offsets for lbu16 and sb16 are different. lbu16 requires [-1, 0, ..., 14], sb16 requires [0, 1, ..., 15].

1183 ↗(On Diff #14272)

Allowed offsets for lhu16 and sh16 are [0, 2, ..., 30] (4-bit unsigned, shifted left by one).

1192 ↗(On Diff #14272)

Allowed offsets for lw16 and sw16 are [0, 4, ..., 60] (4-bit unsigned, shifted left by two).

lib/Target/Mips/MicroMipsInstrInfo.td
285 ↗(On Diff #14272)

For sb16, sh16 and sw16, allowed values for a source register ($rt) are $0, $2-$7 and $17, so GPRMM16Opnd (which represents $2-$7, $16 and $17) cannot be used.

test/MC/Mips/micromips-16-bit-instructions.s
83 ↗(On Diff #14272)

You should also check that $0 is allowed as a source register for store.

test/MC/Mips/micromips-invalid.s
27 ↗(On Diff #14272)

You should also check that error is reported if base reg is not one of $2-$7, $16 and $17.

jkolek updated this revision to Diff 14882.Oct 14 2014, 12:51 PM
  • Fixed operand ranges in MipsAsmParser::processInstruction() for the instructions: SB16, LHU16, SH16, LW16, SW16,
  • Also, SB16, SH16, SW16 are using GPRMM16OpndZero for register $rt,
  • Added check in micromips-16-bit-instructions.s to ensure that $0 is valid operand for $rt register of SW16 (i.e. SH16, SB16),
  • Added checks for invalid operands in micromips-invalid for LBU16, SB16, LHU16, SH16, LW16, SW16.
sstankovic added inline comments.Oct 21 2014, 5:55 AM
test/MC/Mips/micromips-invalid.s
30 ↗(On Diff #14882)

You still don't have a test that checks that error is reported if invalid base register is specified. These 3 lines do have invalid base register ($9 and $10), but they have invalid offset too, and the error that is reported is the error for the offset.

jkolek added a subscriber: Unknown Object (MLST).Nov 18 2014, 5:43 AM
jkolek updated this revision to Diff 16427.Nov 20 2014, 6:43 AM

Added test cases to check invalid base registers.

jkolek updated this revision to Diff 16483.Nov 21 2014, 4:59 AM
jkolek updated this revision to Diff 16486.Nov 21 2014, 5:23 AM
sstankovic edited edge metadata.Nov 24 2014, 6:05 AM

LGTM, with 2 changes.

lib/Target/Mips/MicroMipsInstrInfo.td
193 ↗(On Diff #16486)

This line doesn't fit in 80 columns.

364 ↗(On Diff #16486)

Move this line one space to the right.

sstankovic accepted this revision.Nov 24 2014, 6:05 AM
sstankovic edited edge metadata.
This revision is now accepted and ready to land.Nov 24 2014, 6:05 AM
jkolek closed this revision.Nov 24 2014, 6:39 AM
jkolek updated this revision to Diff 16561.

Closed by commit rL222653 (authored by @jkolek).