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

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?

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(-)

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
>