On Tue, Jun 8, 2021 at 11:31 AM Kewen.Lin <li...@linux.ibm.com> wrote:
>
> on 2021/6/7 下午10:46, Richard Biener wrote:
> > On Wed, Jun 2, 2021 at 11:29 AM Kewen.Lin <li...@linux.ibm.com> wrote:
> >>
> >> Hi,
> >>
> >> As Richi suggested in PR100794, this patch is to remove
> >> some unnecessary update_ssa calls with flag
> >> TODO_update_ssa_only_virtuals, also do some refactoring.
> >>
> >> Bootstrapped/regtested on powerpc64le-linux-gnu P9,
> >> x86_64-redhat-linux and aarch64-linux-gnu, built well
> >> on Power9 ppc64le with --with-build-config=bootstrap-O3,
> >> and passed both P8 and P9 SPEC2017 full build with
> >> {-O3, -Ofast} + {,-funroll-loops}.
> >>
> >> Is it ok for trunk?
> >
> > LGTM, minor comment on the fancy C++:
> >
> > + auto cleanup = [&]() {
> > + release_chains (chains);
> > + free_data_refs (datarefs);
> > + BITMAP_FREE (looparound_phis);
> > + free_affine_expand_cache (&name_expansions);
> > + };
> >
> > + cleanup ();
> > + return 0;
> >
> > so that could have been
> >
> > class cleanup {
> > ~cleanup()
> > {
> > release_chains (chains);
> > free_data_refs (datarefs);
> > BITMAP_FREE (looparound_phis);
> > free_affine_expand_cache (&name_expansions);
> > }
> > } cleanup;
> >
> > ? Or some other means of adding registering a RAII-style cleanup?
> > I mean, we can't wrap it all in
> >
> > try {...}
> > finally {...}
> >
> > because C++ doesn't have finally.
> >
> > OK with this tiny part of the C++ refactoring delayed, but we can also
> > simply
> > discuss best options. At least for looparound_phis a good cleanup would
> > be to pass the bitmap around and use auto_bitmap local to
> > tree_predictive_commoning_loop ...
> >
>
> 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?
>
> One point you might have seen is that to make tree_predictive_commoning_loop
> and its callees as member functions of one class can avoid to pass bitmap
> looparound_phis all around what's in the draft. :)
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.
> BR,
> Kewen
>
