Page MenuHomePhabricator

SLPVectorizer: Cache results from memory alias checking.
Needs ReviewPublic

Authored by eeckstein on Jan 13 2015, 8:17 AM.

Details

Reviewers
aschwaighofer
Summary

This speeds up the dependency calculations for blocks with many load/store/call instructions.
Beside the improved runtime, there is no functional change.

Diff Detail

Event Timeline

eeckstein updated this revision to Diff 18088.Jan 13 2015, 8:17 AM
eeckstein retitled this revision from to SLPVectorizer: Cache results from memory alias checking..
eeckstein updated this object.
eeckstein edited the test plan for this revision. (Show Details)
eeckstein added reviewers: aschwaighofer, chandlerc.
eeckstein added a subscriber: Unknown Object (MLST).
aschwaighofer edited edge metadata.Jan 13 2015, 8:39 AM

LGTM.

I think the discussion about how and whether we should use a caching mechanism in AliasAnalysis should be separate. Some clients already have their own caching strategy (MemoryDependence has its own, for example) and might not want/need the overhead of a caching implementation ...

lib/Transforms/Vectorize/SLPVectorizer.cpp
928

s/descructed/destructed/

chandlerc edited edge metadata.Jan 13 2015, 1:21 PM

Thanks for reverting and sending this out.

My only comment beyond Arnold's (as he knows this code much better than I do) is that it would be good to document this invariant of BoUpSLP somewhere other than a member comment. Not sure what the best place is.

Committed in rL225977 with a few changes/corrections:

  1. added a missing dropAllReferences() before pushing an instruction to DeletedInstructions
  2. extract the removeFromParent/dropAllReferences/push-to-DeletedInstructions to a new function in BoUpSLP

My only comment beyond Arnold's (as he knows this code much better than I do) is that it would be good to document this invariant of BoUpSLP somewhere other than a member comment. Not sure what the best place is.

  1. added a note in SLPVectorizer::runOnFunction

Thanks for reviewing!