This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement BEQZ16 and BNEZ16 instructions
ClosedPublic

Authored by jkolek on Sep 9 2014, 8:17 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

jkolek updated this revision to Diff 13469.Sep 9 2014, 8:17 AM
jkolek retitled this revision from to [mips][microMIPS] Implement BEQZ16 and BNEZ16 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 13698.Sep 15 2014, 2:40 AM

Added test file 'test/MC/Mips/micromips-branch7.s'.

jkolek updated this revision to Diff 14410.Oct 3 2014, 2:45 PM
jkolek added a reviewer: sstankovic.
sstankovic added inline comments.Oct 20 2014, 9:45 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1078 ↗(On Diff #14410)

I think you can use here "if (Offset.getImm() % 2 != 0)" instead of a call to OffsetToAlignment.

lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp
241 ↗(On Diff #14410)

You can put fixup_MICROMIPS_PC7_S1 on the previous line - it's within the 80 columns limit.

lib/Target/Mips/MicroMipsInstrInfo.td
299 ↗(On Diff #14410)

Note that MipsLongBranch has to be fixed for begz16 and bnez16, because it currently assumes that all branches have 16-bit offsets, and will produce wrong code if branches whose allowed offsets are [-64, ..., 63] are present. If long branch for begz16 and bnez16 (when it's implemented) doesn't clobber AT, "let Defs = [AT]" won't be needed. (You might add a TODO comment for all this in MipsLongBranch.cpp or here).

jkolek added a subscriber: Unknown Object (MLST).Nov 18 2014, 5:42 AM
jkolek updated this revision to Diff 16458.Nov 20 2014, 3:20 PM
jkolek updated this revision to Diff 17607.Dec 23 2014, 1:40 PM
jkolek added a subscriber: petarj.

Implemented decoder method DecodeBranchTarget7MM() and added disassembler tests.

sstankovic added inline comments.Dec 30 2014, 8:41 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1219 ↗(On Diff #17607)

You should use 8 instead of 7.

lib/Target/Mips/MicroMipsInstrInfo.td
423 ↗(On Diff #17607)

MipsLongBranch.cpp file is better place for this comment. Also, I was wrong in the previous review comment - allowed offsets for beqz16 and bnez16 are [-128, -126, ..., 126]

jkolek updated this revision to Diff 17803.Jan 5 2015, 8:19 AM
sstankovic accepted this revision.Jan 12 2015, 3:05 AM
sstankovic edited edge metadata.
This revision is now accepted and ready to land.Jan 12 2015, 3:05 AM
This revision was automatically updated to reflect the committed changes.