Page MenuHomePhabricator

LEA code size optimization pass (Part 2): Remove redundant LEA instructions
ClosedPublic

Authored by aturetsk on Sep 30 2015, 8:50 AM.

Details

Summary

Make x86 OptimizeLEAs pass remove LEA instruction if there is another LEA (in the same basic block) which calculates address differing only be a displacement. Works only for -Oz.

Diff Detail

Repository
rL LLVM

Event Timeline

aturetsk updated this revision to Diff 36113.Sep 30 2015, 8:50 AM
aturetsk retitled this revision from to LEA code size optimization pass (Part 2): Remove redundant LEA instructions.
aturetsk updated this object.
aturetsk added a reviewer: nadav.
aturetsk added a subscriber: llvm-commits.
aturetsk updated this object.Sep 30 2015, 9:04 AM
nadav edited edge metadata.Nov 2 2015, 7:28 AM
nadav added a subscriber: nadav.

LGTM.

aturetsk updated this revision to Diff 40930.Nov 23 2015, 7:29 AM
aturetsk edited edge metadata.

Made some changes according to the comments to the first part of the patch

aturetsk updated this revision to Diff 41311.Nov 27 2015, 8:04 AM

Minor test changes to be up-to-date with the first part of the patch.

aturetsk updated this revision to Diff 41763.Dec 3 2015, 8:59 AM

Improved comments.

aturetsk updated this revision to Diff 41853.Dec 4 2015, 2:22 AM

Sync up with the changes of the first part of the patch.

aturetsk updated this revision to Diff 42835.Dec 15 2015, 4:28 AM

Updated to the latest trunk.

qcolombet edited edge metadata.Dec 15 2015, 11:44 AM

Hi Andrey,

Thanks for working on this.

Out of curiosity, what performance impact do you see?
I.e., compile time and code size improvements.

Couple more comments inlined.

Thanks,
-Quentin

lib/Target/X86/X86OptimizeLEAs.cpp
76 ↗(On Diff #42835)

Please comment what AddrDispShift is and how it should be used by the caller.

201 ↗(On Diff #42835)

differ?

228 ↗(On Diff #42835)

This assert is useless.
This is already enforced by isLEA, isn’t it?

Moreover, in the previous check (getOperand(0).getReg()), the compiler would have asserted if the operand was not a register.

246 ↗(On Diff #42835)

Could you add a comment over that loop?

398 ↗(On Diff #42835)

The formatting looks funny to me, have you used clang-format?

415 ↗(On Diff #42835)

else llvm_unreachable or assert?

426 ↗(On Diff #42835)

Add a message in the assert.

aturetsk updated this revision to Diff 43516.Dec 23 2015, 2:33 AM
aturetsk edited edge metadata.

Fixed remarks from inline comments.

Hi Quentin,
Thanks for the review.
This part reduces code size on Spec2000 for additional 0.1%, making it total 0.3% at -Oz.
The performance impact I measured was negative, yet very close to zero. I didn't want to possibly hurt performance at any rate at -Os, so I enabled it only for -Oz and I intend to keep it so until I do some analysis.
I don't see any significant compile time impact on Spec 2000 or the test from https://llvm.org/bugs/show_bug.cgi?id=25843. The complexity of removeRedundantLEAs is something like O(n^2), so the compile time should not blow up for big basic blocks.

lib/Target/X86/X86OptimizeLEAs.cpp
76 ↗(On Diff #42835)

Done.

201 ↗(On Diff #42835)

Fixed.

228 ↗(On Diff #42835)

Removed.

246 ↗(On Diff #42835)

Done.

398 ↗(On Diff #42835)

Absolutely, that's clang-format's doing.

415 ↗(On Diff #42835)

Done.

426 ↗(On Diff #42835)

Done.

qcolombet accepted this revision.Jan 11 2016, 1:59 PM
qcolombet edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Jan 11 2016, 1:59 PM
This revision was automatically updated to reflect the committed changes.