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,
Richard.

> BR,
> Kewen
> -----
> gcc/ChangeLog:
>
>         * tree-predcom.c (execute_pred_commoning): Remove update_ssa call.
>         (tree_predictive_commoning_loop): Factor some cleanup stuffs into
>         lambda function cleanup, remove scev_reset call, and adjust return
>         value.
>         (tree_predictive_commoning): Adjust for different changed values,
>         only set flag TODO_update_ssa_only_virtuals if changed.
>         (pass_data pass_data_predcom): Remove TODO_update_ssa_only_virtuals
>         from todo_flags_finish.
>

Reply via email to