This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement BC16, BEQZC16 and BNEZC16 instructions
ClosedPublic

Authored by zbuljan on Jul 14 2015, 1:50 AM.

Details

Summary

The patch implements microMIPSr6 BC16, BEQZC16 and BNEZC16 instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

zbuljan updated this revision to Diff 29653.Jul 14 2015, 1:50 AM
zbuljan retitled this revision from to [mips][microMIPS] Implement BC16, BEQZC16 and BNEZC16 instructions.
zbuljan updated this object.
zbuljan added subscribers: petarj, llvm-commits.
dsanders requested changes to this revision.Aug 18 2015, 3:51 AM
dsanders edited edge metadata.
dsanders added inline comments.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1705–1708 ↗(On Diff #29653)

Nit: These two cases are identical. Put both case-statements in front of one copy of the code.

2131–2134 ↗(On Diff #29653)

'|| hasMips64r6()' is redundant since the predicates are cumulative.

Also use the ternary operator:

if (inMicroMipsMode())
  Inst.setOpcode(hasMips32r6() ? Mips::BC16_MMR6 : Mips::BC16_MM)
2150 ↗(On Diff #29653)

This does the right thing but we shouldn't list all the compact branches explicitly. Test whether the instruction has a delay slot instead.

lib/Target/Mips/MicroMips32r6InstrInfo.td
151 ↗(On Diff #29653)

Don't use Predicates directly, it causes lots of trouble that is hard to debug. People try to add a predicate and end up accidentally removing some. For example, these instructions lack the predicates that ISA_MICROMIPS32R6 is supposed to add.

Inherit from PredicateControl (you already do this since you inherit from MipsR6Inst) and set each mutually exclusive group independently.

Likewise for the other occasions you override Predicates.

lib/Target/Mips/MicroMipsInstrInfo.td
931 ↗(On Diff #29653)

As above, don't override Predicates. Use PredicateControl's members or preferably, add a ISA_MICROMIPS and use that instead.

This revision now requires changes to proceed.Aug 18 2015, 3:51 AM
zbuljan updated this revision to Diff 32951.Aug 24 2015, 6:32 AM
zbuljan edited edge metadata.

Both case-statements placed in front of one copy of the code in MipsAsmParser::expandInstruction.
hasMips64r6() removed in MipsAsmParser::expandUncondBranchMMPseudo because it was redundant.
Added test for delay slot of instruction at the end of MipsAsmParser::expandUncondBranchMMPseudo.
Added MicroMipsR6Inst16 class for 16-bit instructions.
Classes changed so they inherit from MicroMipsR6Inst16 (PredicateControl) in MicroMips32r6InstrInfo.td.
Subsets of PredicateControl used instead of the Predicates list.
Added ISA_MICROMIPS and used instead of Predicates in MicroMipsInstrInfo.td.

dsanders accepted this revision.Aug 25 2015, 3:10 AM
dsanders edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 25 2015, 3:10 AM
zbuljan updated this revision to Diff 33405.Aug 28 2015, 2:46 AM
zbuljan edited edge metadata.

Added decoder table for microMIPS32R6 and microMIPS64R6 16-bit instructions.

This revision was automatically updated to reflect the committed changes.