This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS]Implement CFC*, CTC* and LDC* instructions
ClosedPublic

Authored by hvarga on Mar 31 2016, 3:47 AM.

Details

Summary

This patch implements microMIPS32r6 CFC1, CFC2, CTC1, CTC2, LDC1 and LDC2 instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

hvarga updated this revision to Diff 52184.Mar 31 2016, 3:47 AM
hvarga retitled this revision from to [mips][microMIPS]Implement CFC*, CTC* and LDC* instructions.
hvarga updated this object.
hvarga added subscribers: llvm-commits, petarj, dsanders.
sdardis requested changes to this revision.Apr 15 2016, 4:16 AM
sdardis edited edge metadata.

Minor nits.

For the (in)valid tests, can you integrate them alphabetically into the corresponding instruction groups.

Can you rebase this to ToT?

lib/Target/Mips/MicroMipsInstrFPU.td
150 ↗(On Diff #52184)

Space between the FGR_32 and the {.

lib/Target/Mips/MipsInstrInfo.td
1088 ↗(On Diff #52184)

Formatting, line length should not exceed 80 characters if possible. Start a newline after the new parameter (Operand MemOpnd).

1647 ↗(On Diff #52184)

Formatting, line length should not exceed 80 characters. Start a new line after "load>,".

test/MC/Mips/mips32r6/invalid.s
16 ↗(On Diff #52184)

What is being tested here that line 15 does not cover?

test/MC/Mips/mips64r6/invalid.s
16 ↗(On Diff #52184)

See my comment on mips32r6/invalid.s.

This revision now requires changes to proceed.Apr 15 2016, 4:16 AM
hvarga updated this revision to Diff 54169.Apr 19 2016, 2:11 AM
hvarga edited edge metadata.

Updated according to comments received from sdardis and also rebased to work with TOT.

sdardis accepted this revision.Apr 19 2016, 5:31 AM
sdardis edited edge metadata.

LGTM with the highlighted nits addressed.

lib/Target/Mips/MicroMips32r6InstrInfo.td
680–682 ↗(On Diff #54169)

Formatting should be like:

class LDWC1_DESC_BASE<string opstr, RegisterOperand RC, InstrItinClass Itin,                                           
                      SDPatternOperator OpNode = null_frag> : MipsR6Inst,                                              
                      HARDFLOAT {

You can start a newline in the parameter list.

698–700 ↗(On Diff #54169)

The formatting should be like:

class COP2LD_MMR6_DESC_BASE<string opstr, RegisterOperand COPOpnd,
                            InstrItinClass Itin,
                            SDPatternOperator OpNode = null_frag> {
test/MC/Mips/micromips32r6/valid.s
42–45 ↗(On Diff #54169)

Line up the "# encoding: .." with the others.

62–63 ↗(On Diff #54169)

And here.

test/MC/Mips/mips32r5/invalid.s
14 ↗(On Diff #54169)

Line up this check line with the rest. The top one is ok.

test/MC/Mips/mips64r5/invalid.s
20 ↗(On Diff #54169)

Same again here.

test/MC/Mips/mips64r6/invalid.s
12 ↗(On Diff #54169)

Restore the jalr.hb lines and submit that as a separate NFC change.

35–38 ↗(On Diff #54169)

Submit this as a separate change.

This revision is now accepted and ready to land.Apr 19 2016, 5:31 AM
This revision was automatically updated to reflect the committed changes.