This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement CFC1, CFC2, CTC1 and CTC2 instructions
ClosedPublic

Authored by mamidzic on Jul 14 2016, 5:17 AM.

Details

Summary

The patch implements microMIPS32r6 CFC1, CFC2, CTC1 and CTC2 instructions.

Diff Detail

Event Timeline

mamidzic updated this revision to Diff 63956.Jul 14 2016, 5:17 AM
mamidzic retitled this revision from to [mips][microMIPS] Implement CFC1, CFC2, CTC1 and CTC2 instructions.
mamidzic updated this object.
mamidzic updated this revision to Diff 64094.EditedJul 14 2016, 11:55 PM
mamidzic edited edge metadata.

Forgot to add CFC1 and CTC1 to instruction mapping tables. CFC2 and CTC2 instructions don't have an existing mips implementation.

sdardis requested changes to this revision.Jul 21 2016, 5:39 AM
sdardis edited edge metadata.

Two major changes required.

a) cfc1 is already defined for microMIPS in MicroMipsInstrFPU.def and was not re-encoded for R6, so we don't need to reimplement it here.

b) We don't currently define cfc2 for microMIPS, so that instruction can be moved to MicroMipsInstrInfo.td and adjusted.

Thanks.

lib/Target/Mips/MicroMips32r6InstrInfo.td
78–81

We don't need to implement cfc for microMIPSR6 as it was not re-encoded for R6. cfc2 was also not re-encoded for R6, so it should be added to MicroMipsInstrInfo.td.

lib/Target/Mips/MipsInstrFPU.td
364–367

These instructions need to be marked as ISA_MIPS32, and the StdMMR6Rel can be dropped.

test/MC/Disassembler/Mips/micromips32r6/valid.txt
65–68

These should also be duplicated into the other micromips directories in disassembler.

test/MC/Mips/micromips32r6/valid.s
50–53

Likewise these tests should be duplicated into test/MC/Mips/micromips/.

This revision now requires changes to proceed.Jul 21 2016, 5:39 AM
mamidzic updated this revision to Diff 66458.Aug 2 2016, 4:36 AM
mamidzic edited edge metadata.

Removed my implementation of CFC1/CFC2, using existing micromips implementation.
CFC2 and CTC2 implementations are now in microMipsInstrInfo
Added dissasembler tests for microMipsr3
Added valid.s test file for microMipsr3 which somehow didn't exist.

Note: Didn't put ISA_MIPS32 to Mips CFC/CTC implementations because it causes Mips1 to Mips5 tests to fail.

sdardis accepted this revision.Aug 3 2016, 7:45 AM
sdardis edited edge metadata.

LGTM with the nit & instruction itinerary defined.

lib/Target/Mips/MicroMipsInstrInfo.td
664–671 ↗(On Diff #66458)

The let line should have the same indentation as the defs above. The defs inside the let block should be indented by a further two spaces. Similar to the change you made in MipsInstrFPU.td

Also, can you define the instruction itinerary for ctc2/cfc2 and use it here? You can copy the details from II_CTC1/II_CFC2.

This revision is now accepted and ready to land.Aug 3 2016, 7:45 AM
This revision was automatically updated to reflect the committed changes.