[PATCH] predcom: Adjust some unnecessary update_ssa calls

Martin Sebor msebor@gmail.com
Mon Jun 7 15:20:46 GMT 2021


On 6/7/21 8:46 AM, Richard Biener via Gcc-patches wrote:
> On Wed, Jun 2, 2021 at 11:29 AM Kewen.Lin <linkw@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 agree this would be better than invoking the cleanup lambda
explicitly.

Going a step further would be to encapsulate all the data in a class
(eliminating the static variables) and make
tree_predictive_commoning_loop() its member function (along with
whatever others it calls), and have the dtor take care of
the cleanup.

Of course, if the data types were classes with ctors and dtors
(including, for example, auto_vec rather than vec for chains)
the cleanup would just happen and none of the explicit calls
would be necessary.

Martin

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



More information about the Gcc-patches mailing list