Page MenuHomePhabricator

[mips] Addition of a third operand to the instructions [d]div, [d]divu
ClosedPublic

Authored by obucina on Feb 4 2016, 8:20 AM.

Details

Summary

Related to http://reviews.llvm.org/D15772

Adds support for third operand for [D]DIV[U] instructions. Additional test for case when destination reg is zero register.

Diff Detail

Repository
rL LLVM

Event Timeline

obucina updated this revision to Diff 46921.Feb 4 2016, 8:20 AM
obucina retitled this revision from to [mips] Addition of a third operand to the instructions [d]div, [d]divu .
obucina updated this object.
obucina added reviewers: dsanders, zoran.jovanovic.
obucina added a subscriber: llvm-commits.
dsanders requested changes to this revision.Feb 12 2016, 9:27 AM
dsanders edited edge metadata.

Could you make the summary more like a commit message?

I don't see any code to handle things like 'ddiv $zero, $4, $5' but it's possible that the real instruction has priority over the macro because $zero is more specific. Could you add tests for 'ddiv $zero, $4, $5' and check that it only expands to a single 'ddiv $zero, $4, $5' instruction? Similarly for ddivu/dmod/dmodu

This revision now requires changes to proceed.Feb 12 2016, 9:27 AM
obucina updated this revision to Diff 48932.Feb 24 2016, 7:10 AM
obucina updated this object.
obucina edited edge metadata.

Review changes applied. Added additional test for case when destination register is zero register.

obucina updated this revision to Diff 49172.Feb 26 2016, 4:44 AM
obucina edited edge metadata.

Ping... We need review on this, since it is prerequisite for http://reviews.llvm.org/D16889

dsanders accepted this revision.Mar 15 2016, 9:17 AM
dsanders edited edge metadata.

With the indentation and missing predicates fixed it will LGTM

lib/Target/Mips/MipsInstrInfo.td
2062–2063 ↗(On Diff #49172)

Indentation is off by one. There are similar cases below.

2069 ↗(On Diff #49172)

The MipsInstAlias's need predicates too. Without them, the alias is still available for all targets.

Likewise below

2070 ↗(On Diff #49172)

Indentation. The operand should align to the first one on the previous line:

(SDivMacro GPR32Opnd:$rt, GPR32Opnd:$rt,
           GPR32Opnd:$rs)

There are similar cases below.

This revision is now accepted and ready to land.Mar 15 2016, 9:17 AM
obucina updated this revision to Diff 52042.Mar 30 2016, 6:11 AM
obucina edited edge metadata.

We fixed indentation and we added predicates to the MipsInstAlias's.
This led to changes in the macro-div-bad.s, macro-divu-bad.s, macro-ddiv-bad.s and macro-ddivu-bad.s test cases.

obucina marked 3 inline comments as done.Mar 30 2016, 6:12 AM
This revision was automatically updated to reflect the committed changes.