This is an archive of the discontinued LLVM Phabricator instance.

[mips][microMIPS] Implement SLT, SLTI, SLTIU, SLTU microMIPS32r6 instructions
ClosedPublic

Authored by hvarga on Apr 21 2016, 12:08 AM.

Details

Summary

This patch implements SLT, SLTI, SLTIU and SLTU microMIPS32r6 instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

hvarga updated this revision to Diff 54463.Apr 21 2016, 12:08 AM
hvarga retitled this revision from to [mips][microMIPS] Implement SLT, SLTI, SLTIU, SLTU microMIPS32r6 instructions.
hvarga updated this object.
hvarga added subscribers: llvm-commits, petarj, dsanders.
sdardis accepted this revision.Apr 21 2016, 2:55 AM
sdardis edited edge metadata.

LGTM with those 2 nits addressed.

lib/Target/Mips/MicroMips32r6InstrInfo.td
984–986 ↗(On Diff #54463)

This change should be separate. Also, what's the reason behind this change?

lib/Target/Mips/MipsInstrInfo.td
1651–1661 ↗(On Diff #54463)

Join these blocks together.

This revision is now accepted and ready to land.Apr 21 2016, 2:55 AM
hvarga added inline comments.Apr 21 2016, 4:48 AM
lib/Target/Mips/MicroMips32r6InstrInfo.td
984–986 ↗(On Diff #54463)

Well actually, It can't be in a separate patch. :)

The reason is that I needed to remove setting list<dag> Pattern = []; in MMArch class. Removing this caused test test/CodeGen/Mips/llvm-ir/add.ll to fail because of wrong instruction selection. In this test, it was expected to select the instruction ADDU16 but instead, instruction ADDU is selected. So, ADDu_MM is selected instead of ADDU16_MMR6.

Using the llvm-tblgen tool I have discovered that ADDu_MM is selected because this definition now, after removing the Pattern in MMArch, has a correct Pattern.

So the MMArch class is actually removing the Pattern which was, for ADDu_MM, defined in a class ArithLogicR. From my point, this is not the desired behavior.

It can be seen from the llvm-tblgen output that for the ADDu_MM the MMArch is set after the ArithLogicR in the inheritance chain. Hence, MMArch overrides the Pattern which was set in ArithLogicR class.

I am certain that this problem is also manifesting for SLT_MM and probably couple more instructions from this patch and the rest of codebase.

lib/Target/Mips/MipsInstrInfo.td
1651–1661 ↗(On Diff #54463)

Will do.

Thank you for the explanation. Go ahead and commit with the one nit addressed.

This revision was automatically updated to reflect the committed changes.