[llvm] r245193 - [PM] Port ScalarEvolution to the new pass manager.

[Tobias Grosser via llvm-commits]

On 08/17/2015 04:08 AM, Chandler Carruth via llvm-commits wrote:
> Author: chandlerc
> Date: Sun Aug 16 21:08:17 2015
> New Revision: 245193
>
> URL: http://llvm.org/viewvc/llvm-project?rev=245193&view=rev
> Log:
> [PM] Port ScalarEvolution to the new pass manager.
>
> This change makes ScalarEvolution a stand-alone object and just produces
> one from a pass as needed. Making this work well requires making the
> object movable, using references instead of overwritten pointers in
> a number of places, and other refactorings.
>
> I've also wired it up to the new pass manager and added a RUN line to
> a test to exercise it under the new pass manager. This includes basic
> printing support much like with other analyses.
>
> But there is a big and somewhat scary change here. Prior to this patch
> ScalarEvolution was never *actually* invalidated!!! Re-running the pass
> just re-wired up the various other analyses and didn't remove any of the
> existing entries in the SCEV caches or clear out anything at all. This
> might seem OK as everything in SCEV that can uses ValueHandles to track
> updates to the values that serve as SCEV keys. However, this still means
> that as we ran SCEV over each function in the module, we kept
> accumulating more and more SCEVs into the cache. At the end, we would
> have a SCEV cache with every value that we ever needed a SCEV for in the
> entire module!!! Yowzers. The releaseMemory routine would dump all of
> this, but that isn't realy called during normal runs of the pipeline as
> far as I can see.
>
> To make matters worse, there *is* actually a key that we don't update
> with value handles -- there is a map keyed off of Loop*s. Because
> LoopInfo *does* release its memory from run to run, it is entirely
> possible to run SCEV over one function, then over another function, and
> then lookup a Loop* from the second function but find an entry inserted
> for the first function! Ouch.
>
> To make matters still worse, there are plenty of updates that *don't*
> trip a value handle. It seems incredibly unlikely that today GVN or
> another pass that invalidates SCEV can update values in *just* such
> a way that a subsequent run of SCEV will incorrectly find lookups in
> a cache, but it is theoretically possible and would be a nightmare to
> debug.
>
> With this refactoring, I've fixed all this by actually destroying and
> recreating the ScalarEvolution object from run to run. Technically, this
> could increase the amount of malloc traffic we see, but then again it is
> also technically correct. ;] I don't actually think we're suffering from
> tons of malloc traffic from SCEV because if we were, the fact that we
> never clear the memory would seem more likely to have come up as an
> actual problem before now. So, I've made the simple fix here. If in fact
> there are serious issues with too much allocation and deallocation,
> I can work on a clever fix that preserves the allocations (while
> clearing the data) between each run, but I'd prefer to do that kind of
> optimization with a test case / benchmark that shows why we need such
> cleverness (and that can test that we actually make it faster). It's
> possible that this will make some things faster by making the SCEV
> caches have higher locality (due to being significantly smaller) so
> until there is a clear benchmark, I think the simple change is best.
>
> Differential Revision: http://reviews.llvm.org/D12063

Thank you Chandler. I have seen a couple of nasty SCEV bugs over time so
carefully cleaning the SCEV cache is very much appreciated.

Best,
Tobias