Re: [PATCH PR41488]Recognize more induction variables by simplifying PEELED chrec in scalar evolution

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Wed, Dec 11, 2013 at 6:50 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Dec 10, 2013 at 10:46:32AM +0800, bin.cheng wrote:
>> This is the new version patch computing the difference in tree affine then
>> comparing against integer_zero_node.
>> Bootstrap and test on current upstream.  I will commit it if there is no
>> other objection.
>
> This breaks bootstrap on x86_64-linux for me too, though in a different
> pass.
> Anyway, the bugs I'm seeing here:
> 1) other passes that use a cache for tree-affine computations
>    don't initialize it by pointer_map_create (), tree-affine.c does that
>    for you
> 2) the cache shouldn't be destroyed by pointer_map_destroy, but
>    instead by free_affine_expand_cache that will actually make sure
>    it doesn't leak memory, etc.
> 3) the actual issue why this breaks bootstrap is that you've added
>    the pointer_map_destroy (should be free_affine_expand_cache per 2) )
>    to scev_finalize, but scev_initialize is performed at the start
>    of the loop passes and scev_finalize several passes later, and
>    in between passes ggc_collect is called.  As the cache isn't registered
>    with GC, if you are unlucky and ggc_collect actually collects
>    in between scev_initialize and scev_finalize and garbage collects
>    some trees only mentioned in the affine cache peeled_chrec_map,
>    things can break completely.
>
> So, IMHO please revert this patch for now and try to find some
> better place for the free_affine_expand_cache (&peeled_chrec_map);
> call so that the cache is never non-NULL when ggc_collect may be called.
> Alternatively you'd need to either tell GC about it (but the structures
> are heap allocated, not GC), or say instruct through some GTY magic
> that during collection free_affine_expand_cache (&peeled_chrec_map);
> should be called.

Thanks for helping on the issue, I shouldn't use the affine facility
before fully understanding it.

I know little about GC, so when ggc_collect may be called (between two
passes)? If so, I have to call free_affine_expand_cache just after the
calling to tree_to_affine_combination_expand in SCEV because it's an
analyzer and as you pointed out, the analyzing result is used by
different optimizers.  The pointer map would be maintained between
different passes if it's not instantly freed.

Thanks,
bin
>
>         Jakub



-- 
Best Regards.