Re: fold strlen (s) eq/ne 0 to *s eq/ne 0 on GIMPLE

[Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On 1 August 2016 at 17:03, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 1 Aug 2016, Prathamesh Kulkarni wrote:
>
>> Hi Richard,
>> The attached patch tries to fold strlen (s) eq/ne 0 to *s eq/ne 0 on GIMPLE.
>> I am not sure where was the ideal place to put this transform in and ended up
>> adding it to strlen_optimize_stmt().
>> Does that look OK ?
>>
>> I needed to add TODO_update_ssa to strlen pass, otherwise we hit the
>> following assert in execute_todo():
>> if (flag_checking
>>       && cfun
>>       && need_ssa_update_p (cfun))
>>     gcc_assert (flags & TODO_update_ssa_any);
>>
>> Bootstrap+test in progress on x86_64-unknown-linux-gnu.
>
> I believe you should factor small-size part of handle_builtin_memcmp and
> re-use that for the code generation part.
>
> You should also remove the corresponding fold-const.c code I think.
Hi,
In the attached version, I removed code from fold-const.c, and
factored code-gen part
into replace_call_cmp. The previous patch incorrectly applied the transform
when result of strlen() had multiple uses, thus restricting it to
has_single_use.
However I suppose we could do better by checking if each immediate use
of the result is involved in compare against 0 and in that case perform
the transform ?
Bootstrap + test in progress on x86_64-unknown-linux-gnu

Thanks,
Prathamesh
>
> Richard.

[-- Attachment #2: strlen-6.diff --]
[-- Type: text/plain, Size: 6267 bytes --]

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index c5d9a79..309ef38 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -10676,30 +10676,6 @@ fold_binary_loc (location_t loc,
 	    return t1;
 	}
 
-      /* Optimize comparisons of strlen vs zero to a compare of the
-	 first character of the string vs zero.  To wit,
-		strlen(ptr) == 0   =>  *ptr == 0
-		strlen(ptr) != 0   =>  *ptr != 0
-	 Other cases should reduce to one of these two (or a constant)
-	 due to the return value of strlen being unsigned.  */
-      if (TREE_CODE (arg0) == CALL_EXPR
-	  && integer_zerop (arg1))
-	{
-	  tree fndecl = get_callee_fndecl (arg0);
-
-	  if (fndecl
-	      && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
-	      && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRLEN
-	      && call_expr_nargs (arg0) == 1
-	      && TREE_CODE (TREE_TYPE (CALL_EXPR_ARG (arg0, 0))) == POINTER_TYPE)
-	    {
-	      tree iref = build_fold_indirect_ref_loc (loc,
-						   CALL_EXPR_ARG (arg0, 0));
-	      return fold_build2_loc (loc, code, type, iref,
-				  build_int_cst (TREE_TYPE (iref), 0));
-	    }
-	}
-
       /* Fold (X >> C) != 0 into X < 0 if C is one less than the width
 	 of X.  Similarly fold (X >> C) == 0 into X >= 0.  */
       if (TREE_CODE (arg0) == RSHIFT_EXPR
diff --git a/gcc/testsuite/gcc.dg/strlenopt-30.c b/gcc/testsuite/gcc.dg/strlenopt-30.c
new file mode 100644
index 0000000..b041d86
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-30.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-strlen" } */
+
+__attribute__((noinline, no_icf))
+_Bool f1(const char *s)
+{
+  unsigned long len = __builtin_strlen (s);
+  _Bool ret = (len == 0);
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 9d7b4df..4ada2ee 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -45,6 +45,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-chkp.h"
 #include "tree-hash-traits.h"
 #include "builtins.h"
+#include "tree-pretty-print.h"
+#include "tree-cfg.h"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -1937,6 +1939,36 @@ handle_builtin_memset (gimple_stmt_iterator *gsi)
   return false;
 }
 
+static void
+replace_call_by_cmp (gimple_stmt_iterator *gsi, location_t loc, 
+		     tree type, tree arg1, tree arg2,
+		     tree res_type, enum tree_code cmp_code)
+{
+  tree ptrtype = build_pointer_type_for_mode (char_type_node, ptr_mode, true);
+  tree off = build_int_cst (ptrtype, 0);
+  arg1 = build2_loc (loc, MEM_REF, type, arg1, off);
+  tree tem1 = fold_const_aggregate_ref (arg1);
+  if (tem1)
+    arg1 = tem1;
+
+  if (POINTER_TYPE_P (TREE_TYPE (arg2)))
+    {
+      arg2 = build2_loc (loc, MEM_REF, type, arg2, off);
+      tree tem2 = fold_const_aggregate_ref (arg2);
+      if (tem2)
+	arg2 = tem2;
+    }
+  else
+    gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (arg2)));
+
+  tree res = fold_convert_loc (loc, res_type,
+			       fold_build2_loc (loc, cmp_code,
+						boolean_type_node,
+						arg1, arg2));
+
+  gimplify_and_update_call_from_tree (gsi, res);
+}
+
 /* Handle a call to memcmp.  We try to handle small comparisons by
    converting them to load and compare, and replacing the call to memcmp
    with a __builtin_memcmp_eq call where possible.  */
@@ -1994,25 +2026,11 @@ handle_builtin_memcmp (gimple_stmt_iterator *gsi)
 	  && (align >= leni || !SLOW_UNALIGNED_ACCESS (mode, align)))
 	{
 	  location_t loc = gimple_location (stmt2);
-	  tree type, off;
+	  tree type; 
 	  type = build_nonstandard_integer_type (leni, 1);
 	  gcc_assert (GET_MODE_BITSIZE (TYPE_MODE (type)) == leni);
-	  tree ptrtype = build_pointer_type_for_mode (char_type_node,
-						      ptr_mode, true);
-	  off = build_int_cst (ptrtype, 0);
-	  arg1 = build2_loc (loc, MEM_REF, type, arg1, off);
-	  arg2 = build2_loc (loc, MEM_REF, type, arg2, off);
-	  tree tem1 = fold_const_aggregate_ref (arg1);
-	  if (tem1)
-	    arg1 = tem1;
-	  tree tem2 = fold_const_aggregate_ref (arg2);
-	  if (tem2)
-	    arg2 = tem2;
-	  res = fold_convert_loc (loc, TREE_TYPE (res),
-				  fold_build2_loc (loc, NE_EXPR,
-						   boolean_type_node,
-						   arg1, arg2));
-	  gimplify_and_update_call_from_tree (gsi, res);
+	  replace_call_by_cmp (gsi, loc, type, arg1, arg2,
+			       TREE_TYPE (res), NE_EXPR);
 	  return false;
 	}
     }
@@ -2302,6 +2320,41 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
 	  else if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
 	    handle_pointer_plus (gsi);
 	}
+      /* strlen (s) eq/ne 0 -> *s eq/ne 0.  */
+      else if (TREE_CODE (lhs) == SSA_NAME && INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+	{
+	  tree rhs2 = gimple_assign_rhs2 (stmt);
+	  tree_code code = gimple_assign_rhs_code (stmt);
+
+	  if ((code == EQ_EXPR || code == NE_EXPR) && integer_zerop (rhs2))
+	    {
+	      tree rhs1 = gimple_assign_rhs1 (stmt);
+	      if (TREE_CODE (rhs1) == SSA_NAME)
+		{
+		  gimple *def_stmt = SSA_NAME_DEF_STMT (rhs1);
+		  if (gcall *call_stmt = dyn_cast<gcall *> (def_stmt))
+		    {
+		      tree callee = gimple_call_fndecl (call_stmt);
+		      if (valid_builtin_call (call_stmt)
+			  && DECL_FUNCTION_CODE (callee) == BUILT_IN_STRLEN)
+			{
+			  tree call_arg = gimple_call_arg (call_stmt, 0);
+			  tree call_lhs = gimple_call_lhs (call_stmt);
+
+			  if (has_single_use (call_lhs))
+			    {
+			      gimple_stmt_iterator call_gsi = gsi_for_stmt (call_stmt);
+			      replace_call_by_cmp (&call_gsi, gimple_location (call_stmt),
+						   TREE_TYPE (rhs2), call_arg, rhs2, TREE_TYPE (call_lhs),
+						   code);
+			      gimple_assign_set_rhs_with_ops (gsi, CONVERT_EXPR, call_lhs);
+			      update_stmt (stmt);
+			    }
+			}
+		    }
+		}
+	    }
+	}
       else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
 	{
 	  tree type = TREE_TYPE (lhs);
@@ -2505,7 +2558,7 @@ const pass_data pass_data_strlen =
   0, /* properties_provided */
   0, /* properties_destroyed */
   0, /* todo_flags_start */
-  0, /* todo_flags_finish */
+  TODO_update_ssa_only_virtuals, /* todo_flags_finish */
 };
 
 class pass_strlen : public gimple_opt_pass