Patchwork share code between tree and gimple folders

login
register
mail settings
Submitter Nathan Froyd
Date Oct. 7, 2010, 5:24 p.m.
Message ID <20101007172434.GE17388@nightcrawler>
Download mbox | patch
Permalink /patch/67084/
State New
Headers show

Comments

Nathan Froyd - Oct. 7, 2010, 5:24 p.m.
The patch below refactors the folding code for {,v}s{,n}printf to share
code between the tree and gimple flavors.  Making this change requires a
new function, rewrite_call_expr_array, that receives its arguments as an
array of trees and is therefore agnostic about tree vs. gimple.  Once we
have such a function, we no longer need gimple_rewrite_call_expr, so it
can be deleted.  Additionally, we need a helper function,
rewrite_call_expr_valist, to share code between rewrite_call_expr_array
and rewrite_call_expr.

As a small optimization, we can also simplify the code from
rewrite_call_expr:

  tree fntype = TREE_TYPE (fndecl);
  tree fn = build1 (ADDR_EXPR, build_pointer_type (fntype), fndecl);
  ...
  return fold (build_call_array_loc (loc, TREE_TYPE (exp), fn, nargs, buffer));

to use a helper function defined recently:

  return build_call_expr_loc_array (loc, fndecl, nargs, buffer);

which avoids consing the result of build_call_array_loc unnecessarily.

Doing this also revealed a small difference between the tree and gimple
versions.  The tree version did:

  return fold (build_call_array_loc (loc, TREE_TYPE (exp), fn, nargs, buffer));

i.e. indicated the return type of the new call to be that of the
original expression, whereas the gimple version did:

  return fold (build_call_array_loc (loc, TREE_TYPE (fntype), fn, nargs, buffer));

i.e. used the return type of the new decl we are now calling.  The new
code follows the gimple version, which I believe is more correct.

Bootstrapped on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

	* builtins.c (rewrite_call_expr): Move code to...
	(rewrite_call_expr_valist): ...here.  Call
	build_call_expr_loc_array.
	(rewrite_call_expr_array): New function.
	(fold_builtin_sprintf_chk_1): New function.
	(fold_builtin_sprintf_chk): Call it.
	(gimple_fold_builtin_sprintf_chk): Likewise.
	(fold_builtin_snprintf_chk_1): New function.
	(fold_builtin_snprintf_chk): Call it.
	(gimple_fold_builtin_snprintf_chk): Likewise.
	(gimple_rewrite_call_expr): Delete.
Richard Henderson - Oct. 7, 2010, 6:08 p.m.
On 10/07/2010 10:24 AM, Nathan Froyd wrote:
> +rewrite_call_expr_array (location_t loc, int oldnargs, tree *args,
> +			 int skip, tree fndecl, int n, ...)
> +{
> +  va_list ap;
> +  tree t;
> +
> +  va_start (ap, n);
> +  t = rewrite_call_expr_valist (loc, oldnargs, args, skip, fndecl, n ap);
                                                                       ^^
Missing comma?  Did this really build or did you post the wrong patch?

 +rewrite_call_expr (location_t loc, tree exp, int skip, tree fndecl, int n, ...)
> +{
> +  va_list ap;
> +  tree t;
> +
> +  va_start (ap, n);
> +  t = rewrite_call_expr_valist (loc, call_expr_nargs (exp),
> +				CALL_EXPR_ARGP (exp), skip, ap);

Missing arguments?

I'm going to wait for you to re-post the patch.


r~

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 4c3b1ae..74c8edc 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -10869,18 +10869,16 @@  fold_builtin_call_array (location_t loc, tree type,
   return build_call_array_loc (loc, type, fn, n, argarray);
 }
 
-/* Construct a new CALL_EXPR using the tail of the argument list of EXP
-   along with N new arguments specified as the "..." parameters.  SKIP
-   is the number of arguments in EXP to be omitted.  This function is used
-   to do varargs-to-varargs transformations.  */
+/* Construct a new CALL_EXPR to FNDECL using the tail of the argument
+   list ARGS along with N new arguments in NEWARGS.  SKIP is the number
+   of arguments in ARGS to be omitted.  OLDNARGS is the number of
+   elements in ARGS.  */
 
 static tree
-rewrite_call_expr (location_t loc, tree exp, int skip, tree fndecl, int n, ...)
+rewrite_call_expr_valist (location_t loc, int oldnargs, tree *args,
+			  int skip, tree fndecl, int n, va_list newargs)
 {
-  int oldnargs = call_expr_nargs (exp);
   int nargs = oldnargs - skip + n;
-  tree fntype = TREE_TYPE (fndecl);
-  tree fn = build1 (ADDR_EXPR, build_pointer_type (fntype), fndecl);
   tree *buffer;
 
   if (n > 0)
@@ -10889,17 +10887,53 @@  rewrite_call_expr (location_t loc, tree exp, int skip, tree fndecl, int n, ...)
       va_list ap;
 
       buffer = XALLOCAVEC (tree, nargs);
-      va_start (ap, n);
       for (i = 0; i < n; i++)
-	buffer[i] = va_arg (ap, tree);
-      va_end (ap);
+	buffer[i] = va_arg (newargs, tree);
       for (j = skip; j < oldnargs; j++, i++)
-	buffer[i] = CALL_EXPR_ARG (exp, j);
+	buffer[i] = args[j];
     }
   else
-    buffer = CALL_EXPR_ARGP (exp) + skip;
+    buffer = args + skip;
 
-  return fold (build_call_array_loc (loc, TREE_TYPE (exp), fn, nargs, buffer));
+  return build_call_expr_loc_array (loc, fndecl, nargs, buffer);
+}
+
+/* Construct a new CALL_EXPR to FNDECL using the tail of the argument
+   list ARGS along with N new arguments specified as the "..."
+   parameters.  SKIP is the number of arguments in ARGS to be omitted.
+   OLDNARGS is the number of elements in ARGS.  */
+
+static tree
+rewrite_call_expr_array (location_t loc, int oldnargs, tree *args,
+			 int skip, tree fndecl, int n, ...)
+{
+  va_list ap;
+  tree t;
+
+  va_start (ap, n);
+  t = rewrite_call_expr_valist (loc, oldnargs, args, skip, fndecl, n ap);
+  va_end (ap);
+
+  return t;
+}
+
+/* Construct a new CALL_EXPR using the tail of the argument list of EXP
+   along with N new arguments specified as the "..." parameters.  SKIP
+   is the number of arguments in EXP to be omitted.  This function is used
+   to do varargs-to-varargs transformations.  */
+
+static tree
+rewrite_call_expr (location_t loc, tree exp, int skip, tree fndecl, int n, ...)
+{
+  va_list ap;
+  tree t;
+
+  va_start (ap, n);
+  t = rewrite_call_expr_valist (loc, call_expr_nargs (exp),
+				CALL_EXPR_ARGP (exp), skip, ap);
+  va_end (ap);
+
+  return t;
 }
 
 /* Validate a single argument ARG against a tree code CODE representing
@@ -12460,31 +12494,31 @@  fold_builtin_strncat_chk (location_t loc, tree fndecl,
   return build_call_expr_loc (loc, fn, 3, dest, src, len);
 }
 
-/* Fold a call EXP to __{,v}sprintf_chk.  Return NULL_TREE if
-   a normal call should be emitted rather than expanding the function
-   inline.  FCODE is either BUILT_IN_SPRINTF_CHK or BUILT_IN_VSPRINTF_CHK.  */
+/* Fold a call EXP to __{,v}sprintf_chk having NARGS passed as ARGS.
+   Return NULL_TREE if a normal call should be emitted rather than
+   expanding the function inline.  FCODE is either BUILT_IN_SPRINTF_CHK
+   or BUILT_IN_VSPRINTF_CHK.  */
 
 static tree
-fold_builtin_sprintf_chk (location_t loc, tree exp,
-			  enum built_in_function fcode)
+fold_builtin_sprintf_chk_1 (location_t loc, int nargs, tree *args,
+			    enum built_in_function fcode)
 {
   tree dest, size, len, fn, fmt, flag;
   const char *fmt_str;
-  int nargs = call_expr_nargs (exp);
 
   /* Verify the required arguments in the original call.  */
   if (nargs < 4)
     return NULL_TREE;
-  dest = CALL_EXPR_ARG (exp, 0);
+  dest = args[0];
   if (!validate_arg (dest, POINTER_TYPE))
     return NULL_TREE;
-  flag = CALL_EXPR_ARG (exp, 1);
+  flag = args[1];
   if (!validate_arg (flag, INTEGER_TYPE))
     return NULL_TREE;
-  size = CALL_EXPR_ARG (exp, 2);
+  size = args[2];
   if (!validate_arg (size, INTEGER_TYPE))
     return NULL_TREE;
-  fmt = CALL_EXPR_ARG (exp, 3);
+  fmt = args[3];
   if (!validate_arg (fmt, POINTER_TYPE))
     return NULL_TREE;
 
@@ -12515,7 +12549,7 @@  fold_builtin_sprintf_chk (location_t loc, tree exp,
 
 	  if (nargs == 5)
 	    {
-	      arg = CALL_EXPR_ARG (exp, 4);
+	      arg = args[4];
 	      if (validate_arg (arg, POINTER_TYPE))
 		{
 		  len = c_strlen (arg, 1);
@@ -12549,38 +12583,50 @@  fold_builtin_sprintf_chk (location_t loc, tree exp,
   if (!fn)
     return NULL_TREE;
 
-  return rewrite_call_expr (loc, exp, 4, fn, 2, dest, fmt);
+  return rewrite_call_expr_array (loc, nargs, args, 4, fn, 2, dest, fmt);
 }
 
-/* Fold a call EXP to {,v}snprintf.  Return NULL_TREE if
+/* Fold a call EXP to __{,v}sprintf_chk.  Return NULL_TREE if
    a normal call should be emitted rather than expanding the function
-   inline.  FCODE is either BUILT_IN_SNPRINTF_CHK or
+   inline.  FCODE is either BUILT_IN_SPRINTF_CHK or BUILT_IN_VSPRINTF_CHK.  */
+
+static tree
+fold_builtin_sprintf_chk (location_t loc, tree exp,
+			  enum built_in_function fcode)
+{
+  return fold_builtin_sprintf_chk_1 (loc, call_expr_nargs (exp),
+				     CALL_EXPR_ARGP (exp), fcode);
+}
+
+/* Fold a call EXP to {,v}snprintf having NARGS passed as ARGS.  Return
+   NULL_TREE if a normal call should be emitted rather than expanding
+   the function inline.  FCODE is either BUILT_IN_SNPRINTF_CHK or
    BUILT_IN_VSNPRINTF_CHK.  If MAXLEN is not NULL, it is maximum length
    passed as second argument.  */
 
-tree
-fold_builtin_snprintf_chk (location_t loc, tree exp, tree maxlen,
-			   enum built_in_function fcode)
+static tree
+fold_builtin_snprintf_chk_1 (location_t loc, int nargs, tree *args,
+			     tree maxlen, enum built_in_function fcode)
 {
   tree dest, size, len, fn, fmt, flag;
   const char *fmt_str;
 
   /* Verify the required arguments in the original call.  */
-  if (call_expr_nargs (exp) < 5)
+  if (nargs < 5)
     return NULL_TREE;
-  dest = CALL_EXPR_ARG (exp, 0);
+  dest = args[0];
   if (!validate_arg (dest, POINTER_TYPE))
     return NULL_TREE;
-  len = CALL_EXPR_ARG (exp, 1);
+  len = args[1];
   if (!validate_arg (len, INTEGER_TYPE))
     return NULL_TREE;
-  flag = CALL_EXPR_ARG (exp, 2);
+  flag = args[2];
   if (!validate_arg (flag, INTEGER_TYPE))
     return NULL_TREE;
-  size = CALL_EXPR_ARG (exp, 3);
+  size = args[3];
   if (!validate_arg (size, INTEGER_TYPE))
     return NULL_TREE;
-  fmt = CALL_EXPR_ARG (exp, 4);
+  fmt = args[4];
   if (!validate_arg (fmt, POINTER_TYPE))
     return NULL_TREE;
 
@@ -12626,7 +12672,21 @@  fold_builtin_snprintf_chk (location_t loc, tree exp, tree maxlen,
   if (!fn)
     return NULL_TREE;
 
-  return rewrite_call_expr (loc, exp, 5, fn, 3, dest, len, fmt);
+  return rewrite_call_expr_array (loc, nargs, args, 5, fn, 3, dest, len, fmt);
+}
+
+/* Fold a call EXP to {,v}snprintf.  Return NULL_TREE if
+   a normal call should be emitted rather than expanding the function
+   inline.  FCODE is either BUILT_IN_SNPRINTF_CHK or
+   BUILT_IN_VSNPRINTF_CHK.  If MAXLEN is not NULL, it is maximum length
+   passed as second argument.  */
+
+tree
+fold_builtin_snprintf_chk (location_t loc, tree exp, tree maxlen,
+			   enum built_in_function fcode)
+{
+  return fold_builtin_snprintf_chk_1 (loc, call_expr_nargs (exp),
+				      CALL_EXPR_ARGP (exp), maxlen, fcode);
 }
 
 /* Fold a call to the {,v}printf{,_unlocked} and __{,v}printf_chk builtins.
@@ -13482,43 +13542,6 @@  do_mpc_arg2 (tree arg0, tree arg1, tree type, int do_nonfinite,
   return result;
 }
 
-/* FIXME tuples.
-   The functions below provide an alternate interface for folding
-   builtin function calls presented as GIMPLE_CALL statements rather
-   than as CALL_EXPRs.  The folded result is still expressed as a
-   tree.  There is too much code duplication in the handling of
-   varargs functions, and a more intrusive re-factoring would permit
-   better sharing of code between the tree and statement-based
-   versions of these functions.  */
-
-/* Construct a new CALL_EXPR using the tail of the argument list of STMT
-   along with N new arguments specified as the "..." parameters.  SKIP
-   is the number of arguments in STMT to be omitted.  This function is used
-   to do varargs-to-varargs transformations.  */
-
-static tree
-gimple_rewrite_call_expr (gimple stmt, int skip, tree fndecl, int n, ...)
-{
-  int oldnargs = gimple_call_num_args (stmt);
-  int nargs = oldnargs - skip + n;
-  tree fntype = TREE_TYPE (fndecl);
-  tree fn = build1 (ADDR_EXPR, build_pointer_type (fntype), fndecl);
-  tree *buffer;
-  int i, j;
-  va_list ap;
-  location_t loc = gimple_location (stmt);
-
-  buffer = XALLOCAVEC (tree, nargs);
-  va_start (ap, n);
-  for (i = 0; i < n; i++)
-    buffer[i] = va_arg (ap, tree);
-  va_end (ap);
-  for (j = skip; j < oldnargs; j++, i++)
-    buffer[i] = gimple_call_arg (stmt, j);
-
-  return fold (build_call_array_loc (loc, TREE_TYPE (fntype), fn, nargs, buffer));
-}
-
 /* Fold a call STMT to __{,v}sprintf_chk.  Return NULL_TREE if
    a normal call should be emitted rather than expanding the function
    inline.  FCODE is either BUILT_IN_SPRINTF_CHK or BUILT_IN_VSPRINTF_CHK.  */
@@ -13526,88 +13549,12 @@  gimple_rewrite_call_expr (gimple stmt, int skip, tree fndecl, int n, ...)
 static tree
 gimple_fold_builtin_sprintf_chk (gimple stmt, enum built_in_function fcode)
 {
-  tree dest, size, len, fn, fmt, flag;
-  const char *fmt_str;
   int nargs = gimple_call_num_args (stmt);
 
-  /* Verify the required arguments in the original call.  */
-  if (nargs < 4)
-    return NULL_TREE;
-  dest = gimple_call_arg (stmt, 0);
-  if (!validate_arg (dest, POINTER_TYPE))
-    return NULL_TREE;
-  flag = gimple_call_arg (stmt, 1);
-  if (!validate_arg (flag, INTEGER_TYPE))
-    return NULL_TREE;
-  size = gimple_call_arg (stmt, 2);
-  if (!validate_arg (size, INTEGER_TYPE))
-    return NULL_TREE;
-  fmt = gimple_call_arg (stmt, 3);
-  if (!validate_arg (fmt, POINTER_TYPE))
-    return NULL_TREE;
-
-  if (! host_integerp (size, 1))
-    return NULL_TREE;
-
-  len = NULL_TREE;
-
-  if (!init_target_chars ())
-    return NULL_TREE;
-
-  /* Check whether the format is a literal string constant.  */
-  fmt_str = c_getstr (fmt);
-  if (fmt_str != NULL)
-    {
-      /* If the format doesn't contain % args or %%, we know the size.  */
-      if (strchr (fmt_str, target_percent) == 0)
-	{
-	  if (fcode != BUILT_IN_SPRINTF_CHK || nargs == 4)
-	    len = build_int_cstu (size_type_node, strlen (fmt_str));
-	}
-      /* If the format is "%s" and first ... argument is a string literal,
-	 we know the size too.  */
-      else if (fcode == BUILT_IN_SPRINTF_CHK
-	       && strcmp (fmt_str, target_percent_s) == 0)
-	{
-	  tree arg;
-
-	  if (nargs == 5)
-	    {
-	      arg = gimple_call_arg (stmt, 4);
-	      if (validate_arg (arg, POINTER_TYPE))
-		{
-		  len = c_strlen (arg, 1);
-		  if (! len || ! host_integerp (len, 1))
-		    len = NULL_TREE;
-		}
-	    }
-	}
-    }
-
-  if (! integer_all_onesp (size))
-    {
-      if (! len || ! tree_int_cst_lt (len, size))
-	return NULL_TREE;
-    }
-
-  /* Only convert __{,v}sprintf_chk to {,v}sprintf if flag is 0
-     or if format doesn't contain % chars or is "%s".  */
-  if (! integer_zerop (flag))
-    {
-      if (fmt_str == NULL)
-	return NULL_TREE;
-      if (strchr (fmt_str, target_percent) != NULL
-	  && strcmp (fmt_str, target_percent_s))
-	return NULL_TREE;
-    }
-
-  /* If __builtin_{,v}sprintf_chk is used, assume {,v}sprintf is available.  */
-  fn = built_in_decls[fcode == BUILT_IN_VSPRINTF_CHK
-		      ? BUILT_IN_VSPRINTF : BUILT_IN_SPRINTF];
-  if (!fn)
-    return NULL_TREE;
-
-  return gimple_rewrite_call_expr (stmt, 4, fn, 2, dest, fmt);
+  return fold_builtin_sprintf_chk_1 (gimple_location (stmt), nargs,
+				     (nargs > 0
+				      ? gimple_call_arg_ptr (stmt, 0)
+				      : &error_mark_node), fcode);
 }
 
 /* Fold a call STMT to {,v}snprintf.  Return NULL_TREE if
@@ -13620,71 +13567,12 @@  tree
 gimple_fold_builtin_snprintf_chk (gimple stmt, tree maxlen,
                                   enum built_in_function fcode)
 {
-  tree dest, size, len, fn, fmt, flag;
-  const char *fmt_str;
-
-  /* Verify the required arguments in the original call.  */
-  if (gimple_call_num_args (stmt) < 5)
-    return NULL_TREE;
-  dest = gimple_call_arg (stmt, 0);
-  if (!validate_arg (dest, POINTER_TYPE))
-    return NULL_TREE;
-  len = gimple_call_arg (stmt, 1);
-  if (!validate_arg (len, INTEGER_TYPE))
-    return NULL_TREE;
-  flag = gimple_call_arg (stmt, 2);
-  if (!validate_arg (flag, INTEGER_TYPE))
-    return NULL_TREE;
-  size = gimple_call_arg (stmt, 3);
-  if (!validate_arg (size, INTEGER_TYPE))
-    return NULL_TREE;
-  fmt = gimple_call_arg (stmt, 4);
-  if (!validate_arg (fmt, POINTER_TYPE))
-    return NULL_TREE;
-
-  if (! host_integerp (size, 1))
-    return NULL_TREE;
-
-  if (! integer_all_onesp (size))
-    {
-      if (! host_integerp (len, 1))
-	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! host_integerp (maxlen, 1))
-	    return NULL_TREE;
-	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return NULL_TREE;
-    }
-
-  if (!init_target_chars ())
-    return NULL_TREE;
-
-  /* Only convert __{,v}snprintf_chk to {,v}snprintf if flag is 0
-     or if format doesn't contain % chars or is "%s".  */
-  if (! integer_zerop (flag))
-    {
-      fmt_str = c_getstr (fmt);
-      if (fmt_str == NULL)
-	return NULL_TREE;
-      if (strchr (fmt_str, target_percent) != NULL
-	  && strcmp (fmt_str, target_percent_s))
-	return NULL_TREE;
-    }
-
-  /* If __builtin_{,v}snprintf_chk is used, assume {,v}snprintf is
-     available.  */
-  fn = built_in_decls[fcode == BUILT_IN_VSNPRINTF_CHK
-		      ? BUILT_IN_VSNPRINTF : BUILT_IN_SNPRINTF];
-  if (!fn)
-    return NULL_TREE;
+  int nargs = gimple_call_num_args (stmt);
 
-  return gimple_rewrite_call_expr (stmt, 5, fn, 3, dest, len, fmt);
+  return fold_builtin_snprintf_chk_1 (gimple_location (stmt), nargs,
+				      (nargs > 0
+				       ? gimple_call_arg_ptr (stmt, 0)
+				       : &error_mark_node), maxlen, fcode);
 }
 
 /* Builtins with folding operations that operate on "..." arguments