This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Do not emit record-form rotates when record-form andi/andis suffices
ClosedPublic

Authored by nemanjai on Mar 26 2018, 8:29 AM.

Details

Summary

This is a follow-up to the previous patch that eliminated some of the rotates. With this addition, we will also emit the record-form andis.
This patch increases the number of record-form rotates we eliminate by more than 70%.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Mar 26 2018, 8:29 AM
nemanjai updated this revision to Diff 139907.Mar 27 2018, 6:06 AM

Added the missing check for the record-form "and immediate shifted" as compares they feed are often redundant.

inouehrs added inline comments.
lib/Target/PowerPC/PPCInstrInfo.cpp
1912 ↗(On Diff #139907)

MBInLoHWord/MEInLoHWord are more descriptive. I first think H means Hi before reading comment.
Also they can be bool, but it is ok if you prefer int for bool.

nemanjai updated this revision to Diff 148844.May 28 2018, 9:29 PM

Change variable names as requested as it cleans up the code.

nemanjai marked an inline comment as done.May 28 2018, 9:30 PM
nemanjai added inline comments.
lib/Target/PowerPC/PPCInstrInfo.cpp
1912 ↗(On Diff #139907)

Thanks, that's a good idea.

nemanjai marked an inline comment as done.

Ping. I'd like to get this committed. Can someone please review?

This patch increases the number of record-form rotates we emit by more than 70%.

Do you mean reducing the record-form rotates by 70%? If so, it is great!

lib/Target/PowerPC/PPCInstrInfo.cpp
1803 ↗(On Diff #148844)

This change seems independent from other part of this patch. So this can be a separate patch.
Also it is nice to add an unit test for this part.

nemanjai updated this revision to Diff 162847.Aug 28 2018, 6:33 AM

Remove the orthogonal change that allows us to optimize compares fed by record-form ANDIS.

nemanjai edited the summary of this revision. (Show Details)Aug 28 2018, 6:33 AM

This patch increases the number of record-form rotates we emit by more than 70%.

Do you mean reducing the record-form rotates by 70%? If so, it is great!

Oops! Yes this patch definitely causes a reduction in the number of record-form rotates :)
I updated the description (it was meant to say eliminate rather than emit).

nemanjai marked an inline comment as done.Aug 28 2018, 6:36 AM
nemanjai added inline comments.
lib/Target/PowerPC/PPCInstrInfo.cpp
1803 ↗(On Diff #148844)

Both great points (this part being orthogonal as well as needing its own test case). Separate patch in https://reviews.llvm.org/D51353

nemanjai marked an inline comment as done.Aug 31 2018, 4:41 AM

@inouehrs I think I've addressed your comments adequately. Do you have any further comments or can you accept this revision?

This revision is now accepted and ready to land.Aug 31 2018, 5:01 AM
This revision was automatically updated to reflect the committed changes.