On Wed, Dec 11, 2013 at 4:17 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Tue, Dec 10, 2013 at 11:59:16PM -0700, Jeff Law wrote:
>> On 12/10/13 23:35, Bin.Cheng wrote:
>> >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.
>> The garbage collector only runs between passes. If you have roots
>> in static storage, then you'll need to decorate them so the garbage
>> collector scans them.
>>
>> There's a fairly extensive discussion of the garbage collector in
>> the GCC internals manual.
>
> It isn't that easy, pointer_map isn't a data structure recognized by the
> garbage collector at all (plus, as I've said before, the code is inserting
> XNEW allocated structures into the pointer_map, which isn't GC friendly
> either, but making them GC allocated might increase garbage unnecessarily,
> as all the structs are short lived).
>
> I've looked through scev_{initialize,finalize} callers, and almost all
> passes initialize it and finalize within the same pass, it is only the loop
> optimizations:
> NEXT_PASS (pass_tree_loop_init);
> NEXT_PASS (pass_lim);
> NEXT_PASS (pass_copy_prop);
> NEXT_PASS (pass_dce_loop);
> NEXT_PASS (pass_tree_unswitch);
> NEXT_PASS (pass_scev_cprop);
> NEXT_PASS (pass_record_bounds);
> NEXT_PASS (pass_check_data_deps);
> NEXT_PASS (pass_loop_distribution);
> NEXT_PASS (pass_copy_prop);
> NEXT_PASS (pass_graphite);
> PUSH_INSERT_PASSES_WITHIN (pass_graphite)
> NEXT_PASS (pass_graphite_transforms);
> NEXT_PASS (pass_lim);
> NEXT_PASS (pass_copy_prop);
> NEXT_PASS (pass_dce_loop);
> POP_INSERT_PASSES ()
> NEXT_PASS (pass_iv_canon);
> NEXT_PASS (pass_parallelize_loops);
> NEXT_PASS (pass_if_conversion);
> /* pass_vectorize must immediately follow pass_if_conversion.
> Please do not add any other passes in between. */
> NEXT_PASS (pass_vectorize);
> PUSH_INSERT_PASSES_WITHIN (pass_vectorize)
> NEXT_PASS (pass_dce_loop);
> POP_INSERT_PASSES ()
> NEXT_PASS (pass_predcom);
> NEXT_PASS (pass_complete_unroll);
> NEXT_PASS (pass_slp_vectorize);
> NEXT_PASS (pass_loop_prefetch);
> NEXT_PASS (pass_iv_optimize);
> NEXT_PASS (pass_lim);
> NEXT_PASS (pass_tree_loop_done);
> where scev is live in between the passes, and unfortunately there
> is no call that each pass calls that you could easily add the free_affine.*
> call to (except for execute_function_todo but you'd need to invent another
> TODO_* flag for it and the question is how you'd ensure it will be handled
> for the non-loop specific passes that are just run in between these
> passes (say pass_copy_prop, would we need pass_copy_prop_loop?).
>
> Perhaps you could replace in tree-affine.c and all it's users
> {,struct} pointer_map_t * with
> struct GTY((user)) affine_expand_cache {
> pointer_map_t *cache;
> };
> and write custom gt_ggc_mx (affine_expand_cache *) function that would
> would either pointer_map_traverse the cache and for each cache entry
> call gt_ggc_mx on the embedded trees in the structures, or drop the cache
> (dunno if the cache is really just a cache and it doesn't matter if it is
> dropped any time, or if it can affect code generation). There are also PCH
> related functions that need to be defined, though I guess you could just
> assert there that the cache is NULL and not walk anything.
Thank both of you for being patient on this patch.
I went through the documentation quickly and realized that I have to
modify pointer-map structure to make it recognized by GC (maybe more
work suggested by Jakub). It seems I shouldn't include that task in
this patch at this stage 3, I am thinking just call free_affine*
function in the place it is created for SCEV. Of course, that makes
it more expensive.
I just found that the patch also fixes PR58296 on IVOPT and I do want
to include it in 4.9 if we are ok with it. So what's your opinion?
Thanks,
bin
>
> Jakub
--
Best Regards.
