This is an archive of the discontinued LLVM Phabricator instance.

Add isRegSequence property
ClosedPublic

Authored by qcolombet on Jul 30 2014, 5:54 PM.

Details

Reviewers
hfinkel
Summary

Hi,

This patch adds a new property: isRegSequence and the related target hook: TargetIntrInfo::getRegSequenceInputs to specify that a target specific instruction is a (kind of) REG_SEQUENCE.

I am looking forward for your feedbacks!

  • Context **

This patch is a follow-up of the copy rewriting work made in peephole (http://reviews.llvm.org/D4086), which landed in r212100.

This patch is the first of a set of three patches that would teach how to get rid of some target specific copy in the advanced copy optimization of the peephole optimizer. See http://reviews.llvm.org/D4086 for more details.

Thanks to the property introduced in this patch, we would be able to optimize the following sequence:
_simpleVectorDiv: @ @simpleVectorDiv
@ BB#0: @ %entry
vmov d0, r2, r3
vmov d1, r0, r1
vmov r0, s0
vmov r1, s2
udiv r0, r1, r0
vmov r1, s1
vmov r2, s3
udiv r1, r2, r1
vmov.32 d16[0], r0
vmov.32 d16[1], r1
vmov r0, r1, d16
bx lr

into:
udiv r0, r0, r2
udiv r1, r1, r3
vmov.32 d16[0], r0
vmov.32 d16[1], r1
vmov r0, r1, d16
bx lr

But this is for the next patches (see next section).

  • Proposed Patch **

The proposed patch allows to specify that an instruction behaves like a (kind of) REG_SEQUENCE.

In that case, "vmov d0, r2, r3" is equivalent to D0 = REG_SEQUENCE r2, ssub0, r3, ssub1.
The isRegSequence property is used to specify that, and the target hook TargetInstrInfo::getRegSequenceInputs is used to specify how to interpret those instructions if it was a REG_SEQUENCE.

The target hook takes a DefIdx because it is possible that an instruction behaves like a REG_SEQUENCE for a given definition, but not for another.

  • What Is Next? **
  • Make use of this property on target specific instruction .
  • Teach the advanced copy optimizer about those instructions .
  • Do the same for insert_subreg and extract_subreg.

As soon as this patch lands, the next ones would be available through llvm-review.
I am attaching them to show the global direction of this.

Thanks,
-Quentin

Diff Detail

Event Timeline

qcolombet updated this revision to Diff 12050.Jul 30 2014, 5:54 PM
qcolombet retitled this revision from to Add isRegSequence property.
qcolombet updated this object.
qcolombet edited the test plan for this revision. (Show Details)
qcolombet added a reviewer: hfinkel.
qcolombet added a subscriber: Unknown Object (MLST).
hfinkel edited edge metadata.Jul 31 2014, 7:33 AM

I think that the general direction seems reasonable. Some comments...

lib/CodeGen/TargetInstrInfo.cpp
854

I would actually arrange this the other way. Have a target hook called getRegSequenceLikeInputs, which is called by this function. This function need not be virtual, but getRegSequenceLikeInputs would be virtual (and just return false by default). In this way, implementers of the callback don't need to remember to call the base class implementation to get the desired behavior for REG_SEQUENCE itself.

utils/TableGen/InstrInfoEmitter.cpp
507

All of the others are on one line, so this one should be too.

Hi Hal,

Thanks for the feedbacks, I’ll make the changes shortly.

Cheers,
-Quentin

lib/CodeGen/TargetInstrInfo.cpp
854

Good idea, I’ll update accordingly.

utils/TableGen/InstrInfoEmitter.cpp
507

Agree, looks like clang-format disagreed ;).

qcolombet updated this revision to Diff 12072.Jul 31 2014, 10:29 AM
qcolombet edited edge metadata.

Update according Hal's feedbacks.

  • Fix a formatting issue.
  • Introduce a new target hook TargetInstrInfo::getRegSequenceLikeInputs called directly by TargetInstrInfo::getRegSequenceInputs.
hfinkel accepted this revision.Aug 9 2014, 8:04 PM
hfinkel edited edge metadata.

LGTM, thanks!

This revision is now accepted and ready to land.Aug 9 2014, 8:04 PM
qcolombet closed this revision.Aug 11 2014, 3:26 PM

Thanks Hal!

Committed revision 215394.