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

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

[Prathamesh Kulkarni]

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

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.

Thanks,
Prathamesh

[-- Attachment #2: patch-1.txt --]
[-- Type: text/plain, Size: 3519 bytes --]

2016-08-01  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	* tree-ssa-strlen.c (strlen_optimize_stmt): Fold strlen (s) eq/ne 0 to
	*s eq/ne 0.
	Change todo_flags_finish for pass_data_strlen from 0 to TODO_update_ssa.

testsuite/
	* gcc.dg/strlenopt-30.c: New test-case.

diff --git a/gcc/testsuite/gcc.dg/strlenopt-30.c b/gcc/testsuite/gcc.dg/strlenopt-30.c
new file mode 100644
index 0000000..da9732f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-30.c
@@ -0,0 +1,20 @@
+/* { 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;
+}
+
+/* Check CONVERT_EXPR's get properly handled.  */
+__attribute__((noinline, no_icf))
+_Bool f2(const char *s)
+{
+  unsigned len = __builtin_strlen (s);
+  return len == 0; 
+}
+
+/* { 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..54f8109 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -45,6 +45,7 @@ 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"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -2302,6 +2303,43 @@ 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 (is_a<gassign *> (def_stmt)
+		      && (gimple_assign_rhs_code (def_stmt) == CONVERT_EXPR
+			  || gimple_assign_rhs_code (def_stmt) == NOP_EXPR)
+		      && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME)
+		    def_stmt = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (def_stmt));
+
+		  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 arg = gimple_call_arg (call_stmt, 0);
+			  tree op = build2 (MEM_REF, char_type_node, arg, build_zero_cst (TREE_TYPE (arg)));
+			  tree temp = make_temp_ssa_name (TREE_TYPE (op), NULL, "strlen");
+			  gimple *memref_stmt = gimple_build_assign (temp, op);
+			  gimple_stmt_iterator call_gsi = gsi_for_stmt (call_stmt);
+			  gsi_insert_before (&call_gsi, memref_stmt, GSI_SAME_STMT);
+			  gassign *g = gimple_build_assign (gimple_call_lhs (call_stmt), CONVERT_EXPR, temp);
+			  gsi_replace (&call_gsi, g, true);
+			}
+		    }
+		}
+	    }
+	}
       else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
 	{
 	  tree type = TREE_TYPE (lhs);
@@ -2505,7 +2543,7 @@ const pass_data pass_data_strlen =
   0, /* properties_provided */
   0, /* properties_destroyed */
   0, /* todo_flags_start */
-  0, /* todo_flags_finish */
+  TODO_update_ssa, /* todo_flags_finish */
 };
 
 class pass_strlen : public gimple_opt_pass

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

[Andrew Pinski]

On Mon, Aug 1, 2016 at 12:15 AM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> 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 suspect it might be better in match.pd.

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

Also you only need to update the virtual SSAs rather than a full SSA update.

Thanks,
Andrew

>
> Bootstrap+test in progress on x86_64-unknown-linux-gnu.
>
> Thanks,
> Prathamesh

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

[Andrew Pinski]

On Mon, Aug 1, 2016 at 12:22 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, Aug 1, 2016 at 12:15 AM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> 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 suspect it might be better in match.pd.

The main reason is it is already in fold-const.c:
      /* 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));
            }
        }

So you are basically moving that to match.pd instead of adding extra code.


Thanks,
Andrew Pinski

>
>>
>> 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);
>
> Also you only need to update the virtual SSAs rather than a full SSA update.
>
> Thanks,
> Andrew
>
>>
>> Bootstrap+test in progress on x86_64-unknown-linux-gnu.
>>
>> Thanks,
>> Prathamesh

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

[Richard Biener]

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.

Richard.

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

[Richard Biener]

On Mon, 1 Aug 2016, Andrew Pinski wrote:

> On Mon, Aug 1, 2016 at 12:22 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> > On Mon, Aug 1, 2016 at 12:15 AM, Prathamesh Kulkarni
> > <prathamesh.kulkarni@linaro.org> 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 suspect it might be better in match.pd.
> 
> The main reason is it is already in fold-const.c:
>       /* 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));
>             }
>         }
> 
> So you are basically moving that to match.pd instead of adding extra code.

The issue is that we currently cannot update virtual operands in most
cases so

(simplify
 (ne (BUILT_IN_STRLEN @0) integer_zerop)
 (ne (MEM_REF:char_type_node @0 { build_int_cst (ptr_type_node, 0); } ) { 
build_zero_cst (char_type_node); } ))

will work but ICE on

int foo (const char *s)
{
  int a = 0;
  return __builtin_strlen (s) != a;
}

when optimizing.  I didn't yet arrive at a good way to solve this issue.
The most straight-forward way would maybe be sth explicit, like

(simplify
 (ne (BUILT_IN_STRLEN:vop@2 @0) integer_zerop)
 (ne (MEM_REF:char_type_node:vop@2 @0 { ...

but inventing a good syntax here is difficult.  Basically you have to
connect memory references in the matched pattern with those in the
replacement.

There is also the possibility to at least capture an existing VUSE during 
matching
and make sure to set that on the replacement.  That would work
automagically but doing this more generically to also handle
stores is difficult.

Note that it all works if the replacement has the only memory
reference in the last stmt.

Richard.

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

[Prathamesh Kulkarni]

[-- 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

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

[Jakub Jelinek]

On Thu, Aug 11, 2016 at 04:06:10PM +0530, Prathamesh Kulkarni wrote:
> +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);

I know it is pre-existing, but do you really need to create everything as
trees first and then gimplify?  Can't you emit GIMPLE right away?

> @@ -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

And if you do, I'd hope you can avoid this.  The strlen call has
a vuse, just use the same virtual operand in vuse on the mem read.

	Jakub