This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR 91468] Small fixes in ipa-cp.c and ipa-prop.c


On Tue, Aug 27, 2019 at 3:15 PM Martin Jambor <mjambor@suse.cz> wrote:
>
> Hi,
>
> Feng Xue read through much of ipa-cp.c and ipa-prop.c and reported a few
> redundancies and small errors in PR 91468.  The patch below fixes all of
> them, specifically:
>
> 1) A typo in ipcp_modif_dom_walker::before_dom_children where a wrong
>    tree variable was checked if it is not a VIEW_CONVERT_EXPR.
>
> 2) update_jump_functions_after_inlining currently handles combinations
>    of unary arithmetic functions and ancestor jump functions which make
>    no sense, cannot happen in meaningful code, and the code path could
>    conceivably be triggered only if LTO was abused to avoid
>    type-casting.  In any case the handling should not be there and does
>    not do anything useful (see discussion in bugzilla for more) and so
>    the patch removes it.
>
> 3) compute_complex_assign_jump_func tests a few things twice, because of
>    a rather mechanical cleanup of mine, so these are removed.
>
> 4) merge_agg_lats_step contains a redundant condition too, but this one
>    is an important correctness invariant, so I strengthened the already
>    existing checking assert afterwards to be a normal assert.
>
> Passed bootstrap and testing on x86_64-linux.  OK for trunk?

OK.

Richard.

> Thanks,
>
> Martin
>
>
> 2019-08-26  Martin Jambor  <mjambor@suse.cz>
>
>         PR ipa/91468
>         * ipa-cp.c (merge_agg_lats_step): Removed redundant test, made a
>         checking assert a normal assert to test it really is redundant.
>         * ipa-prop.c (compute_complex_assign_jump_func): Removed
>         redundant test.
>         (update_jump_functions_after_inlining): Removed combining unary
>         arithmetic operations with an ancestor jump function.
>         (ipcp_modif_dom_walker::before_dom_children): Fix wrong use of rhs
>         instead of t.
> ---
>  gcc/ipa-cp.c   |  8 +++-----
>  gcc/ipa-prop.c | 12 ++----------
>  2 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 0046064fea1..33d52fe5537 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -2026,15 +2026,13 @@ merge_agg_lats_step (class ipcp_param_lattices *dest_plats,
>
>    if (**aglat && (**aglat)->offset == offset)
>      {
> -      if ((**aglat)->size != val_size
> -         || ((**aglat)->next
> -             && (**aglat)->next->offset < offset + val_size))
> +      if ((**aglat)->size != val_size)
>         {
>           set_agg_lats_to_bottom (dest_plats);
>           return false;
>         }
> -      gcc_checking_assert (!(**aglat)->next
> -                          || (**aglat)->next->offset >= offset + val_size);
> +      gcc_assert (!(**aglat)->next
> +                 || (**aglat)->next->offset >= offset + val_size);
>        return true;
>      }
>    else
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 1a0e12e6c0c..a23aa2590a0 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1243,9 +1243,7 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi,
>             break;
>           }
>         case GIMPLE_UNARY_RHS:
> -         if (is_gimple_assign (stmt)
> -             && gimple_assign_rhs_class (stmt) == GIMPLE_UNARY_RHS
> -             && ! CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt)))
> +         if (!CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt)))
>             ipa_set_jf_unary_pass_through (jfunc, index,
>                                            gimple_assign_rhs_code (stmt));
>         default:;
> @@ -2725,12 +2723,6 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
>               dst->value.ancestor.agg_preserved &=
>                 src->value.pass_through.agg_preserved;
>             }
> -         else if (src->type == IPA_JF_PASS_THROUGH
> -                  && TREE_CODE_CLASS (src->value.pass_through.operation) == tcc_unary)
> -           {
> -             dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> -             dst->value.ancestor.agg_preserved = false;
> -           }
>           else if (src->type == IPA_JF_ANCESTOR)
>             {
>               dst->value.ancestor.formal_id = src->value.ancestor.formal_id;
> @@ -4933,7 +4925,7 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb)
>         {
>           /* V_C_E can do things like convert an array of integers to one
>              bigger integer and similar things we do not handle below.  */
> -         if (TREE_CODE (rhs) == VIEW_CONVERT_EXPR)
> +         if (TREE_CODE (t) == VIEW_CONVERT_EXPR)
>             {
>               vce = true;
>               break;
> --
> 2.22.0
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]