On Tue, Jun 22, 2021 at 4:35 AM Kewen.Lin <li...@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.
