This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement CVT.D.fmt, CVT.L.fmt, CVT.S.fmt, CVT.W.fmt, MAX.fmt, MIN.fmt, MAXA.fmt, MINA.fmt and CMP.condn.fmt instructions
ClosedPublic

Authored by zbuljan on Aug 19 2015, 1:50 AM.

Details

Summary

The patch implements microMIPS32r6 and microMIPS64r6 CVT.D.fmt, CVT.L.fmt, CVT.S.fmt, CVT.W.fmt, MAX.fmt, MIN.fmt, MAXA.fmt, MINA.fmt and CMP.condn.fmt instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

zbuljan updated this revision to Diff 32520.Aug 19 2015, 1:50 AM
zbuljan retitled this revision from to [mips][microMIPS] Implement CVT.D.fmt, CVT.L.fmt, CVT.S.fmt, CVT.W.fmt, MAX.fmt, MIN.fmt, MAXA.fmt, MINA.fmt and CMP.condn.fmt instructions.
zbuljan updated this object.
zbuljan added subscribers: petarj, llvm-commits.
dsanders accepted this revision.Aug 21 2015, 3:17 AM
dsanders edited edge metadata.

LGTM with some indentation nits.

There's currently no clang-format for tablegen files but for consistency with the existing definitions, try to format tablegen definitions the same way clang-format would do C/C++. For example, the inheritance lists (the right hand side of the ':') are formatted roughly the same way as initialization lists on a C++ constructor, class arguments are formatted the same way as arguments on a C/C++ function, curly braces increase indentation, etc.

lib/Target/Mips/MicroMips32r6InstrInfo.td
346–348 ↗(On Diff #32520)

Indentation nit: A hanging indent like this makes sense given the amount of wrapping that's needed but either all arguments should be hanging or none. In this case the first arguments should be hanging too.

class CVT_MMR6_DESC_BASE<
    string instr_asm, RegisterOperand DstRC,
    RegisterOperand SrcRC, InstrItinClass Itin,
    ...
358 ↗(On Diff #32520)

Indentation nit: Should be aligned to the first argument.

Likewise for the others

380–382 ↗(On Diff #32520)

Indentation nit: As mentioned above, hanging indents are sensible here but it should be all arguments or none.

lib/Target/Mips/Mips32r6InstrInfo.td
710–717 ↗(On Diff #32520)

Indentation Nit: Should be indented because of the let statement

lib/Target/Mips/MipsInstrFPU.td
309–312 ↗(On Diff #32520)

Indentation Nit: Should be indented because of the let statement

326–327 ↗(On Diff #32520)

Indentation Nit: Should be indented because of the let statement

This revision is now accepted and ready to land.Aug 21 2015, 3:17 AM
This revision was automatically updated to reflect the committed changes.