predcom: Refactor more by encapsulating global states

Richard Biener richard.guenther@gmail.com
Wed Jun 23 07:22:58 GMT 2021


On Tue, Jun 22, 2021 at 4:35 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> Hi Richi and Martin,
>
> >>
> >> Thanks Richi!  One draft (not ready for review) is attached for the further
> >> discussion.  It follows the idea of RAII-style cleanup.  I noticed that
> >> Martin suggested stepping forward to make tree_predictive_commoning_loop
> >> and its callees into one class (Thanks Martin), since there are not many
> >> this kind of C++-style work functions, I want to double confirm which option
> >> do you guys prefer?
> >>
> >
> > Such general cleanup is of course desired - Giuliano started some of it within
> > GSoC two years ago in the attempt to thread the compilation process.  The
> > cleanup then helps to get rid of global state which of course interferes here
> > (and avoids unnecessary use of TLS vars).
> >
> > So yes, encapsulating global state into a class and making accessors
> > member functions is something that is desired (but a lot of mechanical
> > work).
> >
> > Thanks
> > Richard.
> >
> > I meant that not necessarily as something to include in this patch
> > but as a suggestion for a future improvement.  If you'd like to
> > tackle it at any point that would be great of course   In any
> > event, thanks for double-checking!
> >
> > The attached patch looks good to me as well (more for the sake of
> > style than anything else, declaring the class copy ctor and copy assignment = delete would > make it clear it's not meant to be
> > copied, although in this case it's unlikely to make a practical
> > difference).
> >
> > Martin.
>
>
> Thanks for your explanation!  Sorry for the late response.
> As the way to encapsulate global state into a class and making accessors
> member functions looks more complete, I gave up the RAII draft and
> switched onto this way.
>
> This patch is to encapsulate global states into a class and
> making their accessors as member functions, remove some
> consequent useless clean up code, and do some clean up with
> RAII.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu P9,
> x86_64-redhat-linux and aarch64-linux-gnu, also
> bootstrapped on ppc64le P9 with bootstrap-O3 config.
>
> Is it ok for trunk?

OK, thanks for the work - Martins suggestions are good but indeed can be
handled as followup.

Richard.

> BR,
> Kewen
> -----
> gcc/ChangeLog:
>
>         * tree-predcom.c (class pcom_worker): New class.
>         (release_chain): Renamed to...
>         (pcom_worker::release_chain): ...this.
>         (release_chains): Renamed to...
>         (pcom_worker::release_chains): ...this.
>         (aff_combination_dr_offset): Renamed to...
>         (pcom_worker::aff_combination_dr_offset): ...this.
>         (determine_offset): Renamed to...
>         (pcom_worker::determine_offset): ...this.
>         (class comp_ptrs): New class.
>         (split_data_refs_to_components): Renamed to...
>         (pcom_worker::split_data_refs_to_components): ...this,
>         and update with class comp_ptrs.
>         (suitable_component_p): Renamed to...
>         (pcom_worker::suitable_component_p): ...this.
>         (filter_suitable_components): Renamed to...
>         (pcom_worker::filter_suitable_components): ...this.
>         (valid_initializer_p): Renamed to...
>         (pcom_worker::valid_initializer_p): ...this.
>         (find_looparound_phi): Renamed to...
>         (pcom_worker::find_looparound_phi): ...this.
>         (add_looparound_copies): Renamed to...
>         (pcom_worker::add_looparound_copies): ...this.
>         (determine_roots_comp): Renamed to...
>         (pcom_worker::determine_roots_comp): ...this.
>         (determine_roots): Renamed to...
>         (pcom_worker::determine_roots): ...this.
>         (single_nonlooparound_use): Renamed to...
>         (pcom_worker::single_nonlooparound_use): ...this.
>         (remove_stmt): Renamed to...
>         (pcom_worker::remove_stmt): ...this.
>         (execute_pred_commoning_chain): Renamed to...
>         (pcom_worker::execute_pred_commoning_chain): ...this.
>         (execute_pred_commoning): Renamed to...
>         (pcom_worker::execute_pred_commoning): ...this.
>         (struct epcc_data): New member worker.
>         (execute_pred_commoning_cbck): Call execute_pred_commoning
>         with pcom_worker pointer.
>         (find_use_stmt): Renamed to...
>         (pcom_worker::find_use_stmt): ...this.
>         (find_associative_operation_root): Renamed to...
>         (pcom_worker::find_associative_operation_root): ...this.
>         (find_common_use_stmt): Renamed to...
>         (pcom_worker::find_common_use_stmt): ...this.
>         (combinable_refs_p): Renamed to...
>         (pcom_worker::combinable_refs_p): ...this.
>         (reassociate_to_the_same_stmt): Renamed to...
>         (pcom_worker::reassociate_to_the_same_stmt): ...this.
>         (stmt_combining_refs): Renamed to...
>         (pcom_worker::stmt_combining_refs): ...this.
>         (combine_chains): Renamed to...
>         (pcom_worker::combine_chains): ...this.
>         (try_combine_chains): Renamed to...
>         (pcom_worker::try_combine_chains): ...this.
>         (prepare_initializers_chain): Renamed to...
>         (pcom_worker::prepare_initializers_chain): ...this.
>         (prepare_initializers): Renamed to...
>         (pcom_worker::prepare_initializers): ...this.
>         (prepare_finalizers_chain): Renamed to...
>         (pcom_worker::prepare_finalizers_chain): ...this.
>         (prepare_finalizers): Renamed to...
>         (pcom_worker::prepare_finalizers): ...this.
>         (tree_predictive_commoning_loop): Renamed to...
>         (pcom_worker::tree_predictive_commoning_loop): ...this, adjust
>         some calls and remove some cleanup code.
>         (tree_predictive_commoning): Adjusted to use pcom_worker instance.
>         (static variable looparound_phis): Remove.
>         (static variable name_expansions): Remove.


More information about the Gcc-patches mailing list