On Tue, Dec 13, 2016 at 05:41:09PM +0530, Prathamesh Kulkarni wrote:
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -2222,6 +2222,90 @@ handle_char_store (gimple_stmt_iterator *gsi)
>    return true;
>  }
>  
> +/* Try to fold strstr (s, t) eq/ne s to memcmp (s, t, strlen (t)) eq/ne 0.  
> */
> +
> +static void
> +fold_strstr_to_memcmp (enum tree_code code, tree rhs1, tree rhs2, gimple 
> *stmt)

You can drop code argument here, see below.  And I'd say it is better to
do the
  if (TREE_CODE (rhs1) != SSA_NAME || TREE_CODE (rhs2) != SSA_NAME)
    return;
here than repeat it in all the callers.

> +               if (gimple_assign_rhs_code (stmt) == COND_EXPR)
> +                 {
> +                   tree cond = gimple_assign_rhs1 (stmt);
> +                   TREE_SET_CODE (cond, code);

TREE_CODE (cond) is already code, so no need to set it again.

> +               gcond *cond = as_a<gcond *> (stmt);
> +               gimple_cond_set_lhs (cond, memcmp_lhs);
> +               gimple_cond_set_rhs (cond, zero);
> +               gimple_cond_set_code (cond, code);

And gimple_cond_code (cond) == code here too.

> +               update_stmt (cond);
> +             }

You can perhaps move the update_stmt (stmt); to a single spot after
all the 3 cases are handled.

> +         if (cond_code == EQ_EXPR || cond_code == NE_EXPR)
> +           {
> +             tree rhs1 = TREE_OPERAND (cond, 0);
> +             tree rhs2 = TREE_OPERAND (cond, 1);

While it is necessary to check cond_code here and in the other spots
similarly, because otherwise you don't know if it has 2 arguments etc.,
you can avoid the SSA_NAME tests here.

> +             if (TREE_CODE (rhs1) == SSA_NAME
> +                 && TREE_CODE (rhs2) == SSA_NAME)
> +               fold_strstr_to_memcmp (cond_code, rhs1, rhs2, stmt);
> +           }
> +       }
> +     else if (code == EQ_EXPR || code == NE_EXPR)
> +       {
> +         tree rhs1 = gimple_assign_rhs1 (stmt);
> +         tree rhs2 = gimple_assign_rhs2 (stmt);
> +
> +         if (TREE_CODE (rhs1) == SSA_NAME
> +             && TREE_CODE (rhs2) == SSA_NAME)

And here.
> +           fold_strstr_to_memcmp (code, rhs1, rhs2, stmt);
> +       }
> +      }
> +    else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
>       {
>         tree type = TREE_TYPE (lhs);
>         if (TREE_CODE (type) == ARRAY_TYPE)
> @@ -2316,6 +2427,17 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
>           }
>       }
>      }
> +  else if (gcond *cond = dyn_cast<gcond *> (stmt))
> +    {
> +      enum tree_code code = gimple_cond_code (cond);
> +      tree lhs = gimple_cond_lhs (stmt);
> +      tree rhs = gimple_cond_rhs (stmt);
> +
> +      if ((code == EQ_EXPR || code == NE_EXPR)
> +       && TREE_CODE (lhs) == SSA_NAME
> +       && TREE_CODE (rhs) == SSA_NAME)

And here.
> +     fold_strstr_to_memcmp (code, lhs, rhs, stmt);
> +    }
>  
>    if (gimple_vdef (stmt))
>      maybe_invalidate (stmt);

Otherwise LGTM, but it would be nice to cover also the COND_EXPR case by a
testcase (can be done incrementally).

        Jakub

Reply via email to