Re: [PATCH] Refactor tree-loop-distribution for thread safety

[Andrew Pinski <pinskia at gmail dot com>]

On Tue, Nov 12, 2019 at 1:16 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Sat, Nov 9, 2019 at 3:26 PM Giuliano Belinassi
> <giuliano.belinassi@usp.br> wrote:
> >
> > Hi all,
> >
> > This patch refactors tree-loop-distribution.c for thread safety without
> > use of C11 __thread feature. All global variables were moved to a struct
> > which is initialized at ::execute time.
>
> Thanks for working on this.  I've been thinking on how to make this
> nicer which naturally leads to the use of C++ classes and member
> functions which get 'this' for free.  This means all functions that
> make use of 'priv' in your patch would need to become member
> functions of the class and pass_loop_distribution::execute would
> wrap it like
>
> unsigned int
> pass_loop_distribution::execute (function *fun)
> {
>   return priv_pass_vars().execute (fun);
> }
>
> please find a better name for 'priv_pass_vars' since you can't
> reuse that name for other passes due to C++ ODR rules.
> I would suggest 'loop_distribution'.

Unless you use an anonymous namespace or your own namespace.
This is what I did when I was developing a pass, I used both and even
had an using class statement in that file to reduce the ammount of
typing in some cases.

Thanks,
Andrew


>
> Can you try if going this route works well?
>
> Thanks,
> Richard.
>
> > I can install this patch myself in trunk if it's OK.
> >
> > gcc/ChangeLog
> > 2019-11-09  Giuliano Belinassi  <giuliano.belinassi@usp.br>
> >
> >         * cfgloop.c (get_loop_body_in_custom_order): New.
> >         * cfgloop.h (get_loop_body_in_custom_order): New prototype.
> >         * tree-loop-distribution.c (struct priv_pass_vars): New.
> >         (bb_top_order_cmp_r): New.
> >         (create_rdg_vertices): Update prototype.
> >         (stmts_from_loop): Same as above.
> >         (update_for_merge): Same as above.
> >         (partition_merge_into): Same as above.
> >         (get_data_dependence): Same as above.
> >         (data_dep_in_cycle_p): Same as above.
> >         (update_type_for_merge): Same as above.
> >         (build_rdg_partition_for-vertex): Same as above.
> >         (classify_builtin_ldst): Same as above.
> >         (classify_partition): Same as above.
> >         (share_memory_accesses): Same as above.
> >         (rdg_build_partitions): Same as above.
> >         (pg_add_dependence_edges): Same as above.
> >         (build_partition_graph): Same as above.
> >         (merge_dep_scc_partitions): Same as above.
> >         (break_alias_scc_partitions): Same as above.
> >         (finalize_partitions): Same as above.
> >         (distribute_loop): Same as above.
> >         (bb_top_order_init): New function.
> >         (bb_top_order_destroy): New function.
> >         (pass_loop_distribution::execute): Initialize struct priv.
> >
> > Thank you,
> > Giuliano.