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

Jakub Jelinek jakub@redhat.com
Tue Dec 10 22:50:00 GMT 2013


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.

	Jakub



More information about the Gcc-patches mailing list