This is an archive of the discontinued LLVM Phabricator instance.

[LICM&MSSA] Limit store hoisting.
ClosedPublic

Authored by asbirlea on Feb 8 2019, 11:42 AM.

Details

Summary

If there is no clobbering access for a store inside the loop, that store
can only be hoisted if there are no interfearing loads.
A more general verification introduced here: there are no loads that are
not optimized to an access outside the loop.
Addresses PR40586.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Feb 8 2019, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2019, 11:42 AM

Thanks for this!

lib/Transforms/Scalar/LICM.cpp
121 ↗(On Diff #186012)

I don't think that this will play nicely in threaded environments (ThinLTO?)

An alternative might be to stick it into LICM as a member, then pass it to canSinkOrHoistInst as a param, though I don't know this code well enough to say whether that's the best approach

1194 ↗(On Diff #186012)

Why does optimizedness matter here? MemoryUses should always have valid defining accesses; I imagine that a valid-but-not-fully-optimal one would be good enough as long as it points outside of the loop

asbirlea updated this revision to Diff 186052.Feb 8 2019, 2:48 PM
asbirlea marked 2 inline comments as done.

Address comments.

asbirlea marked 2 inline comments as done.Feb 8 2019, 2:49 PM
asbirlea added inline comments.
lib/Transforms/Scalar/LICM.cpp
121 ↗(On Diff #186012)

Ack, passed as param (renamed for clarity of it's meaning).

1194 ↗(On Diff #186012)

Indeed, removed.

LGTM; thanks!

This revision is now accepted and ready to land.Feb 11 2019, 10:49 AM
This revision was automatically updated to reflect the committed changes.
asbirlea marked an inline comment as done.