This is an archive of the discontinued LLVM Phabricator instance.

[mips][micromips] Implement DCLO, DCLZ, DROTR, DROTR32 and DROTRV instructions
ClosedPublic

Authored by hvarga on Feb 5 2016, 12:55 AM.

Details

Summary

Patch implements DCLO, DCLZ, DROTR, DROTR32 and DROTRV instructions for microMIPS64r6.

Diff Detail

Repository
rL LLVM

Event Timeline

hvarga updated this revision to Diff 46999.Feb 5 2016, 12:55 AM
hvarga retitled this revision from to [mips][micromips] Implement DCLO, DCLZ, DROTR, DROTR32 and DROTRV instructions.
hvarga updated this object.
hvarga added subscribers: llvm-commits, petarj.
sdardis requested changes to this revision.Apr 22 2016, 3:44 AM
sdardis edited edge metadata.

Can you rebase and repost this patch?

You should also extend test/CodeGen/Mips/countleading.ll and test/CodeGen/Mips/mips64shift.ll for checking microMIPS64R6 for these instructions.

lib/Target/Mips/MicroMips64r6InstrFormats.td
118 ↗(On Diff #46999)

Rename this to POOL32S_2R_SA5_FM_MMR6. Putting a '_' between the 2R and SA5 makes it a bit clearer that this class describes a instruction with a 2 register and shift amount of 5 form.

lib/Target/Mips/Mips64InstrInfo.td
260–261 ↗(On Diff #46999)

These need to be marked NoInMicroMips too.

test/MC/Mips/micromips64r6/valid.s
157 ↗(On Diff #46999)

Please restore the blank line at the end.

This revision now requires changes to proceed.Apr 22 2016, 3:44 AM
hvarga updated this revision to Diff 59517.EditedJun 3 2016, 2:53 AM
hvarga updated this object.
hvarga edited edge metadata.

Changed FM classes so that they inherit from MipsR6Inst and MMR6Arch.
Changed DESC classes for DCLO, DCLZ, DSBH_DSHD so that they don't inherit from MipsR6Inst and MMR6Arch.
Added BaseOpcode to desc classes that were missing it.
Added two SHIFT_ROTATE classes to serve as DESC_BASE to all shift and rotate instructions in MM64R6.
Added CodeGen support and tests for DROTR, DROTRV, DCLZ, DCLO in MM64R6.

This update has been contributed by @mamidzic.

hvarga added a subscriber: mamidzic.Jun 3 2016, 2:54 AM
hvarga edited edge metadata.Jun 3 2016, 2:57 AM
sdardis requested changes to this revision.Jun 6 2016, 9:26 AM
sdardis edited edge metadata.

Some nits and one minor change+addition.

For convenience, mips64 assemblers treat instructions like drotr as taking an immediate value in the range of 0..63 and then picking the d<op>32 form if required.

You will need to modify MipsMCCodeEmitter::encodeInstruction to do this.

lib/Target/Mips/MicroMips64r6InstrInfo.td
250–251 ↗(On Diff #59517)

Like DSLL_, DSRA_, DROTR_MM64R6_DESC should take a uimm6 & immZExt6.

You will then have to modify MipsMCCodeEmitter::encodeInstruction & LowerLargeShift as well to pick the correct D<op> or D<op>32.

lib/Target/Mips/Mips64InstrInfo.td
165–179 ↗(On Diff #59517)

Join these two blocks together.

286–288 ↗(On Diff #59517)

These two blocks can also be joined together.

lib/Target/Mips/Mips64r6InstrInfo.td
103–113 ↗(On Diff #59517)

Join these two block together.

test/CodeGen/Mips/mips64shift.ll
100–101 ↗(On Diff #59517)

Once the above change is made, this can be simplified back to drotr $... 54.

This revision now requires changes to proceed.Jun 6 2016, 9:26 AM

One further thing: drotr and dclo, dclz do not appear in the instruction mapping tables.

dsanders added inline comments.Jun 7 2016, 2:09 AM
lib/Target/Mips/MicroMips64r6InstrInfo.td
250–251 ↗(On Diff #59517)

An alternative way to do this is to make the DROTR32_*_DESC class use immZExtPlus32 so that SelectionDAG picks the correct instruction up front. Then the assembler part can be added using an appropriate InstAlias using the uimm5_plus32 operand. The main benefit of this approach is that we can leave the implementation to tablegen instead of writing custom C++.

I have a slight preference for the tablegen approach but if you go that route then you should also change the MIPS64 version for consistency. If you don't want to change the MIPS64 version right now then I'm ok with using the uimm6 and MipsMCCodeEmitter approach Simon describes for now.

hvarga updated this revision to Diff 60154.Jun 9 2016, 4:08 AM
hvarga edited edge metadata.

Added MM64R6 cases to LowerLargeShift and encodeInstruction for DSLL, DSRA and DROTR instructions.
Instruction mapping tables:

Added StdMMR6Rel to DSLL, DSRA, DCLO, and DCLZ Mips64 instructions.
Added R6MMR6Rel to DCLO and DCLZ Mips64R6 instructions.

Joined NotInMicroMips predicate blocks where possible.

This update has been contributed by @mamidzic.

sdardis accepted this revision.Jun 14 2016, 3:19 AM
sdardis edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Jun 14 2016, 3:19 AM
This revision was automatically updated to reflect the committed changes.