[PATCH][RFC] Fix PR59860

Richard Biener rguenther@suse.de
Sun Jan 19 18:05:00 GMT 2014


The following patch fixes PR59860 by removing the only folding
builtins.c does that can end up recursing to GIMPLE call stmt
folding.  It does that via strcat -> strlen + strcpy folding
and then folding the strlen gimple stmt via gimplify which
then can use the SSA web to fold that to a constant and then
the strcpy call to memcpy.  This confuses the virtual operand
updating code - not that it ends up creating wrong virtual SSA
form but by bogously marking virtual operands for renaming
through the operand scanner as the folding on the just gimplified
sequence doesn't see any VOPs yet.

Bootstrapped on x86_64-unknown-linux-gnu, testing reveals that I
have to adjust gcc.c-torture/execute/builtins/strcat.c expectations
and gcc.dg/strlenopt-* likely because of different input.

I still think this patch is better than the second option, avoiding
to call fold_stmt from the gimplifier in this case (either by
a new ctx flag or looking at ctx->into_ssa).  I also think that
"fixing" this by scheduling update-ssa after objsz is wrong.

Any opinions?  Maybe any different preferences for branch / trunk?

Long term all sophisticated call stmt foldings should move from
GENERIC folding in builtins.c to GIMPLE folding in gimple-fold.c
of course (retaining only constant folding in builtins.c for FEs use).
And the gimplifier shouldn't fold itself.

Thanks,
Richard.

2014-01-17  Richard Biener  <rguenther@suse.de>

	PR middle-end/59860
	* builtins.c (fold_builtin_strcat): Remove case better handled
	by tree-ssa-strlen.c

	* gcc.dg/pr59860.c: New testcase.

Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c	(revision 206714)
--- gcc/builtins.c	(working copy)
*************** fold_builtin_strcat (location_t loc ATTR
*** 11749,11789 ****
        if (p && *p == '\0')
  	return dst;
  
-       if (optimize_insn_for_speed_p ())
- 	{
- 	  /* See if we can store by pieces into (dst + strlen(dst)).  */
- 	  tree newdst, call;
- 	  tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
- 	  tree strcpy_fn = builtin_decl_implicit (BUILT_IN_STRCPY);
- 
- 	  if (!strlen_fn || !strcpy_fn)
- 	    return NULL_TREE;
- 
- 	  /* If we don't have a movstr we don't want to emit an strcpy
- 	     call.  We have to do that if the length of the source string
- 	     isn't computable (in that case we can use memcpy probably
- 	     later expanding to a sequence of mov instructions).  If we
- 	     have movstr instructions we can emit strcpy calls.  */
- 	  if (!HAVE_movstr)
- 	    {
- 	      tree len = c_strlen (src, 1);
- 	      if (! len || TREE_SIDE_EFFECTS (len))
- 		return NULL_TREE;
- 	    }
- 
- 	  /* Stabilize the argument list.  */
- 	  dst = builtin_save_expr (dst);
- 
- 	  /* Create strlen (dst).  */
- 	  newdst = build_call_expr_loc (loc, strlen_fn, 1, dst);
- 	  /* Create (dst p+ strlen (dst)).  */
- 
- 	  newdst = fold_build_pointer_plus_loc (loc, dst, newdst);
- 	  newdst = builtin_save_expr (newdst);
- 
- 	  call = build_call_expr_loc (loc, strcpy_fn, 2, newdst, src);
- 	  return build2 (COMPOUND_EXPR, TREE_TYPE (dst), call, dst);
- 	}
        return NULL_TREE;
      }
  }
--- 11749,11754 ----
Index: gcc/testsuite/gcc.dg/pr59860.c
===================================================================
*** gcc/testsuite/gcc.dg/pr59860.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr59860.c	(working copy)
***************
*** 0 ****
--- 1,15 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O" } */
+ 
+ extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__)) __attribute__ ((__artificial__)) char * __attribute__ ((__nothrow__ , __leaf__))
+ strcat (char *__restrict __dest, const char *__restrict __src)
+ {
+   return __builtin___strcat_chk (__dest, __src, __builtin_object_size (__dest, 2 > 1));
+ }
+ static char raw_decode;
+ void foo (char **argv, char *outfilename)
+ {
+   if (**argv == 'r')
+     raw_decode = 1;
+   strcat (outfilename, raw_decode ? ".raw" : ".wav");
+ }



More information about the Gcc-patches mailing list