On Fri, Dec 09, 2016 at 05:36:41PM +0530, Prathamesh Kulkarni wrote:
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -2302,7 +2302,81 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
>         else if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>           handle_pointer_plus (gsi);
>       }
> -      else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
> +
> +      /* Fold strstr (s, t) == s to memcmp (s, t, strlen (t)) == 0.
> +      if strlen (t) is known and var holding return value of strstr
> +      has single use.  */
> +
> +      else if (TREE_CODE (lhs) == SSA_NAME && INTEGRAL_TYPE_P (TREE_TYPE 
> (lhs)))
> +     {
> +       enum tree_code code = gimple_assign_rhs_code (stmt);
> +       if (code == EQ_EXPR || code == NE_EXPR)

This way you handle _8 = _5 == _7;, but not if (_5 == _7) bar ();.  Shouldn't 
you
also handle GIMPLE_COND similarly (of course, the rhs1 and rhs2 grabbing
and code grabbing is different for GIMPLE_COND.  But the rest should be
the same, except again that you don't want to replace the GIMPLE_COND, but
adjust it.  Maybe also COND_EXPR in gimple_assign (_9 = _5 == _7 ? _10 : _11;).

> +         {
> +           tree rhs1 = gimple_assign_rhs1 (stmt);
> +           tree rhs2 = gimple_assign_rhs2 (stmt);
> +           if (TREE_CODE (rhs1) == SSA_NAME
> +               && TREE_CODE (rhs2) == SSA_NAME)
> +             {
> +               gcall *call_stmt = dyn_cast<gcall *> (SSA_NAME_DEF_STMT 
> (rhs1));
> +               if (!call_stmt)
> +                 {
> +                   call_stmt = dyn_cast<gcall *> (SSA_NAME_DEF_STMT (rhs2));
> +                   tree tmp = rhs1;
> +                   rhs1 = rhs2;
> +                   rhs2 = tmp;

We use std::swap (rhs1, rhs2); in this case these days.

> +                 }
> +
> +               tree call_lhs;
> +               if (call_stmt
> +                   && gimple_call_builtin_p (call_stmt, BUILT_IN_STRSTR)
> +                   && (call_lhs = gimple_call_lhs (call_stmt))
> +                   && has_single_use (call_lhs))

This might not optimize if you have:
  _5 = foo ();
  _7 = __builtin_strstr (_5, "abcd");
  _8 = _5 == _7;

Or even you could have:
  _5 = __builtin_strstr (...);
  _7 = __builtin_strstr (_5, "abcd");
  _8 = _5 == _7;

So I wonder if you shouldn't do:
                  gimple *call_stmt = NULL;
                  for (int pass = 0; pass < 2; pass++)
                    {
                      gimple *g = SSA_NAME_DEF_STMT (rhs1);
                      if (gimple_call_builtin_p (g, BUILT_IN_STRSTR)
                          && gimple_call_lhs (g) == rhs1
                          && has_single_use (rhs1)
                          && gimple_call_arg (g, 0) == rhs2)
                        {
                          call_stmt = g;
                          break;
                        }
                      std::swap (rhs1, rhs2);
                    }
                  if (call_stmt)
...

I think you don't need operand_equal_p, because SSA_NAMEs should just
be the same pointer if they are the same thing.
The above way you handle both orderings.  Perhaps also it is big enough to
be done in a separate function, which you call with the code/rhs1/rhs2 and
stmt for the EQ/NE_EXPR is_gimple_assign as well as for COND_EXPR and
GIMPLE_COND.

        Jakub

Reply via email to