This is an archive of the discontinued LLVM Phabricator instance.

Promote aggregate store to memset when possible
ClosedPublic

Authored by deadalnix on Jan 6 2016, 5:59 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

deadalnix updated this revision to Diff 44112.Jan 6 2016, 5:59 AM
deadalnix retitled this revision from to Promote aggregate store to memset when possible.
deadalnix updated this object.
deadalnix added a subscriber: llvm-commits.
junbuml added a subscriber: junbuml.Jan 6 2016, 6:47 AM
mehdi_amini added inline comments.Jan 6 2016, 10:57 AM
lib/Transforms/Scalar/MemCpyOptimizer.cpp
537 ↗(On Diff #44112)

Unrelated change? Please commit separately if you want it.

639 ↗(On Diff #44112)

It is not intuitive (to me at least) that it is always a good thing, especially since tryMergingIntoMemset just above has a notion of profitability.
If I understand correctly, you're doing it unconditionally for aggregate because the rest of the optimizer is not dealing nicely with them, right?
If this is correct, please add a comment here explaining this.

deadalnix added inline comments.Jan 6 2016, 11:24 AM
lib/Transforms/Scalar/MemCpyOptimizer.cpp
639 ↗(On Diff #44112)

Yes, also one element aggregate would have been unpacked away by instcombine at this stage, so only somewhat large aggregate will be memsetted here.

It is done after tryMergingIntoMemset because it is indeed less profitable so we want the most profitable optimization tried first.

deadalnix updated this revision to Diff 44139.Jan 6 2016, 11:33 AM

Add a comment to explain why the transformation.

mehdi_amini accepted this revision.Jan 6 2016, 11:49 AM
mehdi_amini edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Jan 6 2016, 11:49 AM
This revision was automatically updated to reflect the committed changes.