On Thu, 19 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> Two years ago, I've added support for up to 2 simple preparation statements
> in value_replacement, but the
> -      && estimate_num_insns (assign, &eni_time_weights)
> +      && estimate_num_insns (bb_seq (middle_bb), &eni_time_weights)
> change, meant that we compute the cost of all those statements rather than
> just the single assign that has been the single supported non-debug
> statement in the bb before, doesn't do what I thought would do, gimple_seq
> is just gimple * and thus it can't be really overloaded depending on whether
> we pass a single gimple * or a whole sequence.  Which means in the last
> two years it doesn't count all the statements, but only the first one.
> With -g that happens to be a DEBUG_STMT, or it could be e.g. the first
> preparation statement which could be much cheaper than the actual assign.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

I wonder if we should make gimple vs gimple_seq type safe via a
struct gimple_seq { gimple *first }; wrapper type ...

Thanks,
Richard.

> 2020-03-19  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR tree-optimization/94211
>       * tree-ssa-phiopt.c (value_replacement): Use estimate_num_insns_seq
>       instead of estimate_num_insns for bb_seq (middle_bb).  Rename
>       emtpy_or_with_defined_p variable to empty_or_with_defined_p, adjust
>       all uses.
> 
>       * gcc.dg/pr94211.c: New test.
> 
> --- gcc/tree-ssa-phiopt.c.jj  2020-03-16 23:49:29.853404202 +0100
> +++ gcc/tree-ssa-phiopt.c     2020-03-18 19:42:22.583225152 +0100
> @@ -1056,7 +1056,7 @@ value_replacement (basic_block cond_bb,
>    gimple *cond;
>    edge true_edge, false_edge;
>    enum tree_code code;
> -  bool emtpy_or_with_defined_p = true;
> +  bool empty_or_with_defined_p = true;
>  
>    /* If the type says honor signed zeros we cannot do this
>       optimization.  */
> @@ -1075,7 +1075,7 @@ value_replacement (basic_block cond_bb,
>       {
>         if (gimple_code (stmt) != GIMPLE_PREDICT
>             && gimple_code (stmt) != GIMPLE_NOP)
> -         emtpy_or_with_defined_p = false;
> +         empty_or_with_defined_p = false;
>         continue;
>       }
>        /* Now try to adjust arg0 or arg1 according to the computation
> @@ -1085,7 +1085,7 @@ value_replacement (basic_block cond_bb,
>            && jump_function_from_stmt (&arg0, stmt))
>           || (lhs == arg1
>               && jump_function_from_stmt (&arg1, stmt)))
> -     emtpy_or_with_defined_p = false;
> +     empty_or_with_defined_p = false;
>      }
>  
>    cond = last_stmt (cond_bb);
> @@ -1137,7 +1137,7 @@ value_replacement (basic_block cond_bb,
>        /* If the middle basic block was empty or is defining the
>        PHI arguments and this is a single phi where the args are different
>        for the edges e0 and e1 then we can remove the middle basic block. */
> -      if (emtpy_or_with_defined_p
> +      if (empty_or_with_defined_p
>         && single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)),
>                                                e0, e1) == phi)
>       {
> @@ -1255,7 +1255,7 @@ value_replacement (basic_block cond_bb,
>        && profile_status_for_fn (cfun) != PROFILE_ABSENT
>        && EDGE_PRED (middle_bb, 0)->probability < profile_probability::even ()
>        /* If assign is cheap, there is no point avoiding it.  */
> -      && estimate_num_insns (bb_seq (middle_bb), &eni_time_weights)
> +      && estimate_num_insns_seq (bb_seq (middle_bb), &eni_time_weights)
>        >= 3 * estimate_num_insns (cond, &eni_time_weights))
>      return 0;
>  
> --- gcc/testsuite/gcc.dg/pr94211.c.jj 2020-03-18 16:30:34.562427467 +0100
> +++ gcc/testsuite/gcc.dg/pr94211.c    2020-03-18 16:30:19.998640206 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/94211 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcompare-debug" } */
> +
> +long
> +foo (long a, long b)
> +{
> +  if (__builtin_expect (b == 1, 1))
> +    return a;
> +  int e = a + 1;
> +  return a / b;
> +}
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Reply via email to