This is an archive of the discontinued LLVM Phabricator instance.

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

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
81

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

213

differ?

240

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.

258

Could you add a comment over that loop?

426

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

443

else llvm_unreachable or assert?

454

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
81

Done.

213

Fixed.

240

Removed.

258

Done.

426

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

443

Done.

454

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.