On 6/8/21 3:30 AM, Kewen.Lin 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?
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).
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. :)
Yes, that would simplify the interfaces of all the functions that
the info members are passed to as arguments.
Martin
BR,
Kewen
