This is an archive of the discontinued LLVM Phabricator instance.

[LCSSA] Cache LoopExits to avoid wasted work
ClosedPublic

Authored by reames on Sep 13 2016, 7:43 AM.

Details

Summary

When looking at the scribus_1.3 example from https://llvm.org/bugs/show_bug.cgi?id=10584, I noticed that we were spending a large amount of time computing loop exits in LCSSA. This code appears to be written with the assumption that LoopExits are stored in the Loop and thus cheap to query. This is not true, so we should cache the result across the potentially long running loop which tends to visit a small handful of Loops.

On the particular example from 10584, this change drops the time spent in LCSSA computation by about 80%.

Diff Detail

Repository
rL LLVM

Event Timeline

reames updated this revision to Diff 71165.Sep 13 2016, 7:43 AM
reames retitled this revision from to [LCSSA] Cache LoopExits to avoid wasted work.
reames updated this object.
reames added reviewers: chandlerc, wmi, hfinkel, xur, majnemer.
reames added a subscriber: llvm-commits.
reames updated this object.Sep 13 2016, 7:56 AM
chandlerc accepted this revision.Sep 15 2016, 10:46 PM
chandlerc edited edge metadata.

LGTM, Feel free to submit with the tweak below.

lib/Transforms/Utils/LCSSA.cpp
70–73 ↗(On Diff #71165)

Nice!

What about using a SmallDenseMap and TinyPtrVector here with a small size of 4 or so? Seems likely to make the common case a bit lighter weight.

This revision is now accepted and ready to land.Sep 15 2016, 10:46 PM
This revision was automatically updated to reflect the committed changes.
reames added inline comments.Sep 19 2016, 4:43 PM
lib/Transforms/Utils/LCSSA.cpp
70–73 ↗(On Diff #71165)

FYI: I ended up using a SmallDenseMap<Loop*, SmallVector<BasicBlock *,1>>. Trying to use the TinyPtrVector didn't work out well because the getExitBlocks function takes a SmallVectorImpl.

Honestly, it also seems like TinyPtrVector should be an implementation detail of SmallVector<T, 1>.