On Mon, Jan 20, 2014 at 02:37:21PM +0100, Richard Biener wrote:
> Well, strcat itself can do that ... but yes, as I said, if you can
> CSE that strlen call then it's a win. But you can't know this without
It can't, strcat doesn't know the length of the src string, we don't have
any 3 argument strcat alternative API where the compiler tells it the length
of the string that the compiler knows, but the function doesn't.
> > Normally, if folding of a builtin folds it into a call to some other
> > builtin, that other builtin is folded right away, so the common case is
> > optimized immediately, the only problem is if gimple_fold_builtin tries
> > harder to optimize using maximum lengths or exact length (as in this case).
> > And, for this it wouldn't even help if we handled STRCAT/STRCAT_CHK
> > specially too and passed the src length to the folder routine, because
> > gimple_fold_builtin first folds normally and only if that fails, attempts
> > harder.
>
> But we still can re-introduce the folding I removed from builtins.c
> below the avoid_folding_inline_builtin section as in that case
> builtins.c didn't do the folding and the gimple folding has
> strictly more information. No? I don't really like a special
There are two cases. One is where the length of the second string
is really variable or not known to the compiler. Then I agree library
strcat ought to do the exact same job, it is strlen + movstr instruction.
The other case is your testcase, where it is just because the compiler did a
poor job. I guess we can use something like following untested patch
for that. Or supposedly we could only change fold_builtin_strcat and
leave strcat_chk unmodified, after all strcat_chk folding is only handling
the case where the size is -1UL (i.e. unknown) and then folds to strcat.
2014-01-20 Jakub Jelinek <ja...@redhat.com>
* tree.h (fold_builtin_strcat, fold_builtin_strcat_chk): New
prototypes.
* builtins.c (fold_builtin_strcat): No longer static. Add len
argument, if non-NULL, don't call c_strlen. Optimize
directly into __builtin_memcpy instead of __builtin_strcpy.
(fold_builtin_strcat_chk): No longer static. Add len argument,
if non-NULL, call fold_builtin_strcat.
(fold_builtin_2): Adjust fold_builtin_strcat{,_chk} callers.
* gimple-fold.c (gimple_fold_builtin): Handle BUILT_IN_STRCAT
and BUILT_IN_STRCAT_CHK.
--- gcc/tree.h.jj 2014-01-07 17:49:40.000000000 +0100
+++ gcc/tree.h 2014-01-20 15:04:48.273418236 +0100
@@ -5854,12 +5854,14 @@ extern tree fold_call_expr (location_t,
extern tree fold_builtin_fputs (location_t, tree, tree, bool, bool, tree);
extern tree fold_builtin_strcpy (location_t, tree, tree, tree, tree);
extern tree fold_builtin_strncpy (location_t, tree, tree, tree, tree, tree);
+extern tree fold_builtin_strcat (location_t, tree, tree, tree);
extern tree fold_builtin_memory_chk (location_t, tree, tree, tree, tree, tree,
tree, bool,
enum built_in_function);
extern tree fold_builtin_stxcpy_chk (location_t, tree, tree, tree, tree, tree,
bool,
enum built_in_function);
extern tree fold_builtin_stxncpy_chk (location_t, tree, tree, tree, tree,
tree, bool,
enum built_in_function);
+extern tree fold_builtin_strcat_chk (location_t, tree, tree, tree, tree, tree);
extern tree fold_builtin_snprintf_chk (location_t, tree, tree, enum
built_in_function);
extern bool fold_builtin_next_arg (tree, bool);
extern enum built_in_function builtin_mathfn_code (const_tree);
--- gcc/builtins.c.jj 2014-01-20 12:41:48.000000000 +0100
+++ gcc/builtins.c 2014-01-20 15:19:23.585947825 +0100
@@ -180,7 +180,6 @@ static tree fold_builtin_varargs (locati
static tree fold_builtin_strpbrk (location_t, tree, tree, tree);
static tree fold_builtin_strstr (location_t, tree, tree, tree);
static tree fold_builtin_strrchr (location_t, tree, tree, tree);
-static tree fold_builtin_strcat (location_t, tree, tree);
static tree fold_builtin_strncat (location_t, tree, tree, tree);
static tree fold_builtin_strspn (location_t, tree, tree);
static tree fold_builtin_strcspn (location_t, tree, tree);
@@ -194,7 +193,6 @@ static void maybe_emit_chk_warning (tree
static void maybe_emit_sprintf_chk_warning (tree, enum built_in_function);
static void maybe_emit_free_warning (tree);
static tree fold_builtin_object_size (tree, tree);
-static tree fold_builtin_strcat_chk (location_t, tree, tree, tree, tree);
static tree fold_builtin_strncat_chk (location_t, tree, tree, tree, tree,
tree);
static tree fold_builtin_sprintf_chk (location_t, tree, enum
built_in_function);
static tree fold_builtin_printf (location_t, tree, tree, tree, bool, enum
built_in_function);
@@ -10770,7 +10768,7 @@ fold_builtin_2 (location_t loc, tree fnd
return fold_builtin_strstr (loc, arg0, arg1, type);
case BUILT_IN_STRCAT:
- return fold_builtin_strcat (loc, arg0, arg1);
+ return fold_builtin_strcat (loc, arg0, arg1, NULL_TREE);
case BUILT_IN_STRSPN:
return fold_builtin_strspn (loc, arg0, arg1);
@@ -10963,7 +10961,8 @@ fold_builtin_3 (location_t loc, tree fnd
ignore, fcode);
case BUILT_IN_STRCAT_CHK:
- return fold_builtin_strcat_chk (loc, fndecl, arg0, arg1, arg2);
+ return fold_builtin_strcat_chk (loc, fndecl, arg0, arg1,
+ NULL_TREE, arg2);
case BUILT_IN_PRINTF_CHK:
case BUILT_IN_VPRINTF_CHK:
@@ -11813,8 +11812,9 @@ fold_builtin_strpbrk (location_t loc, tr
COMPOUND_EXPR in the chain will contain the tree for the simplified
form of the builtin function call. */
-static tree
-fold_builtin_strcat (location_t loc ATTRIBUTE_UNUSED, tree dst, tree src)
+tree
+fold_builtin_strcat (location_t loc ATTRIBUTE_UNUSED, tree dst, tree src,
+ tree len)
{
if (!validate_arg (dst, POINTER_TYPE)
|| !validate_arg (src, POINTER_TYPE))
@@ -11832,14 +11832,15 @@ fold_builtin_strcat (location_t loc ATTR
/* 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);
+ tree memcpy_fn = builtin_decl_implicit (BUILT_IN_MEMCPY);
- if (!strlen_fn || !strcpy_fn)
+ if (!strlen_fn || !memcpy_fn)
return NULL_TREE;
/* If the length of the source string isn't computable don't
- split strcat into strlen and strcpy. */
- tree len = c_strlen (src, 1);
+ split strcat into strlen and memcpy. */
+ if (! len)
+ len = c_strlen (src, 1);
if (! len || TREE_SIDE_EFFECTS (len))
return NULL_TREE;
@@ -11853,7 +11854,11 @@ fold_builtin_strcat (location_t loc ATTR
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);
+ len = fold_convert_loc (loc, size_type_node, len);
+ len = size_binop_loc (loc, PLUS_EXPR, len,
+ build_int_cst (size_type_node, 1));
+
+ call = build_call_expr_loc (loc, memcpy_fn, 3, newdst, src, len);
return build2 (COMPOUND_EXPR, TREE_TYPE (dst), call, dst);
}
return NULL_TREE;
@@ -13005,9 +13010,9 @@ fold_builtin_stxncpy_chk (location_t loc
/* Fold a call to the __strcat_chk builtin FNDECL. DEST, SRC, and SIZE
are the arguments to the call. */
-static tree
+tree
fold_builtin_strcat_chk (location_t loc, tree fndecl, tree dest,
- tree src, tree size)
+ tree src, tree len, tree size)
{
tree fn;
const char *p;
@@ -13030,6 +13035,12 @@ fold_builtin_strcat_chk (location_t loc,
if (!fn)
return NULL_TREE;
+ if (len)
+ {
+ tree ret = fold_builtin_strcat (loc, dest, src, len);
+ if (ret)
+ return ret;
+ }
return build_call_expr_loc (loc, fn, 2, dest, src);
}
--- gcc/gimple-fold.c.jj 2013-11-05 13:06:02.000000000 +0100
+++ gcc/gimple-fold.c 2014-01-20 15:12:42.297991142 +0100
@@ -866,6 +866,8 @@ gimple_fold_builtin (gimple stmt)
break;
case BUILT_IN_STRCPY:
case BUILT_IN_STRNCPY:
+ case BUILT_IN_STRCAT:
+ case BUILT_IN_STRCAT_CHK:
arg_idx = 1;
type = 0;
break;
@@ -941,6 +943,22 @@ gimple_fold_builtin (gimple stmt)
val[1]);
break;
+ case BUILT_IN_STRCAT:
+ if (val[1] && is_gimple_val (val[1]) && nargs == 2)
+ result = fold_builtin_strcat (loc, gimple_call_arg (stmt, 0),
+ gimple_call_arg (stmt, 1),
+ val[1]);
+ break;
+
+ case BUILT_IN_STRCAT_CHK:
+ if (val[1] && is_gimple_val (val[1]) && nargs == 3)
+ result = fold_builtin_strcat_chk (loc, callee,
+ gimple_call_arg (stmt, 0),
+ gimple_call_arg (stmt, 1),
+ val[1],
+ gimple_call_arg (stmt, 2));
+ break;
+
case BUILT_IN_FPUTS:
if (nargs == 2)
result = fold_builtin_fputs (loc, gimple_call_arg (stmt, 0),
Jakub
