This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Add CodeGen support for SUBU16, SUB, SUBU, DSUB and DSUBU instructions
ClosedPublic

Authored by zbuljan on Jan 28 2016, 1:21 AM.

Details

Summary

The patch adds CodeGen support for microMIPSr6 SUBU16, SUB, SUBU, DSUB and DSUBU instructions:

  • updated sub.ll with tests for microMIPSr6 SUBU16, SUB, SUBU, DSUB and DSUBU instruction selection
  • separated microMIPS64r6 DSUB and DSUBU instructions from equivalent MIPS64 instructions using NotInMicroMips predicate
  • added DAG patterns using MipsPat for proper selection of instructions
  • implemented DSUB and DSUBU instructions for microMIPS64r6 and added tests for the standard encodings

Diff Detail

Repository
rL LLVM

Event Timeline

zbuljan updated this revision to Diff 46241.Jan 28 2016, 1:21 AM
zbuljan retitled this revision from to [mips][microMIPS] Add CodeGen support for SUBU16, SUB, SUBU, DSUB and DSUBU instructions.
zbuljan updated this object.
zbuljan added subscribers: petarj, llvm-commits.
sdardis requested changes to this revision.Apr 20 2016, 4:00 AM
sdardis edited edge metadata.

Comments inlined.

Can you add invalid tests for dneg(u) with an immediate operand?

In MipsSEDAGToDAGISel::selectNode(SDNode *Node), there is an expansion for the ISD:SUBE node which currently expands to Mips::DSUBu / Mips::SUBu. You will need to extend that to cover micromips.

lib/Target/Mips/MicroMips64r6InstrFormats.td
87–102 ↗(On Diff #46241)

Rather than adding this specific class for dsub(u), can you use POOL32SARITH_FM_MMR6 instead of POOL32S_DSUB? They share the same major opcode.

lib/Target/Mips/MicroMips64r6InstrInfo.td
31–32 ↗(On Diff #46241)

See my previous comment.

169 ↗(On Diff #46241)

One more pattern required, "dnegu $rt".

lib/Target/Mips/Mips64InstrInfo.td
124–129 ↗(On Diff #46241)

After you rebase, you can include DSUBu and DSUB in the same block ass DADD and DADDU.

This revision now requires changes to proceed.Apr 20 2016, 4:00 AM

I've looked farther and you can ignore:

In MipsSEDAGToDAGISel::selectNode(SDNode *Node), there is an expansion for the ISD:SUBE node which currently expands to Mips::DSUBu / Mips::SUBu. You will need to extend that to cover micromips.

We have tables which perform the correct translation to microMIPS(R6).

zbuljan updated this revision to Diff 54635.Apr 22 2016, 5:36 AM
zbuljan edited edge metadata.

Rebased to work with TOT.
Removed redundant POOL32S_DSUB_FM_MMR6 class and replaced with existing POOL32S_ARITH_FM_MMR6.
Added alias for dnegu $rt.
Added invalid tests for dneg and dnegu with immediate operands.

sdardis accepted this revision.Apr 22 2016, 7:16 AM
sdardis edited edge metadata.

LGTM with one very important point.

hvarga has changed the definition of a tablegen class in D19354, so you'll need to make a similar change for SUBU16_MMR6_DESC that was done for ADDU16_MMR6_DESC. Otherwise the instruction selection table is ordered suboptimially and subu16 cannot be chosen.

Thanks.

This revision is now accepted and ready to land.Apr 22 2016, 7:16 AM
This revision was automatically updated to reflect the committed changes.