This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Enable merging of adjacent zero stores for all subtargets.
ClosedPublic

Authored by mcrosier on Nov 8 2016, 6:25 AM.

Details

Summary

This optimization merges adjacent zero stores into a wider store.

e.g.,

strh wzr, [x0]
strh wzr, [x0, #2]
; becomes
str wzr, [x0]

e.g.,

str wzr, [x0]
str wzr, [x0, #4]
; becomes
str xzr, [x0]

Previously, this was only enabled for Kryo and Cortex-A57. I'd like to enable it for all subtargets.

Chad

Diff Detail

Repository
rL LLVM

Event Timeline

mcrosier updated this revision to Diff 77185.Nov 8 2016, 6:25 AM
mcrosier retitled this revision from to [AArch64] Enable merging of adjacent zero stores for all subtargets..
mcrosier updated this object.
mcrosier added subscribers: llvm-commits, gberry.
MatzeB accepted this revision.Nov 10 2016, 2:18 PM
MatzeB edited edge metadata.

LGTM, nitpick below.

BTW: I tried to find some discussion of why we have this load/store combining feature in the AArch64 target (don't we already have that in GVN and other places in the middleend?) but couldn't find any reviews or discussions that would motivate it.

test/CodeGen/AArch64/arm64-narrow-st-merge.ll
1–8 ↗(On Diff #77185)

I don't see the benefits of testing this for every CPU in the AArch64 target. Maybe 1 single RUN: is enough here?

This revision is now accepted and ready to land.Nov 10 2016, 2:18 PM
mcrosier marked an inline comment as done.Nov 11 2016, 6:10 AM
mcrosier added a subscriber: junbuml.

LGTM, nitpick below.

BTW: I tried to find some discussion of why we have this load/store combining feature in the AArch64 target (don't we already have that in GVN and other places in the middleend?) but couldn't find any reviews or discussions that would motivate it.

Thanks for the review, Matthias. I'll follow up on your question with @junbuml as he was the original author of this optimization. I thought we did a similar form of combing at isel lowering, but I may be mistaken.

test/CodeGen/AArch64/arm64-narrow-st-merge.ll
1–8 ↗(On Diff #77185)

Sure.

This revision was automatically updated to reflect the committed changes.
mcrosier marked an inline comment as done.
gberry added inline comments.Nov 11 2016, 10:29 AM
test/CodeGen/AArch64/arm64-narrow-st-merge.ll
9 ↗(On Diff #77185)

Perhaps we should add a run that sets FeatureStrictAlign and check that this merging doesn't happen?

Committed r286617 to show strict align disables the narrow zero store optimization. Thanks, Geoff.