This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][4.1/4.2] Fix PR27260, fallout of fix for PR27095
On Wed, Apr 26, 2006 at 11:10:19PM +0930, Alan Modra wrote:
> On Wed, Apr 26, 2006 at 09:15:58AM +0930, Alan Modra wrote:
> > 4.0 and up are much simplified due to ssa.
> > After much looking at code, I don't think we'll ever see a SAVE_EXPR in
> > the arg trees. expand_call seems to agree with my analysis, with
> > fix_unsafe_tree no longer needed.
>
> :ADDPATCH middle-end:
>
> Since we don't use the args twice here for anything, the
> builtin_save_expr's aren't needed and we can simplify the code quite a
> bit, in the process avoiding PR27260. Bootstrapped and regression
> tested powerpc-linux and powerpc64-linux.
>
> PR middle-end/27260
> * builtins.c (expand_builtin_memset): Don't use SAVE_EXPR on args
> or expand libcall here. Instead, just throw away rtl on failure.
Is that safe? What if some of the temporaries expanded during RTL expansion
get the temporary saved into some DECL_RTL (e.g. if there are any SAVE_EXPR
embedded in dest, val or len)? I know that because of tree-ssa
that certainly isn't very likely, arguments shouldn't have side-effects
etc., still throwing away expanded RTL sounds risky to me.
Just for completeness, here is a patch I wrote today for this instead before
seeing your patch, at least the testcase from it should be IMHO usable.
2006-04-26 Jakub Jelinek <jakub@redhat.com>
PR middle-end/27260
* builtins.c (expand_builtin_memset): Don't call builtin_save_expr.
In do_libcall fallback, create val, dest and len arguments
using make_tree on the created rtxs.
* gcc.c-torture/execute/20060426-1.c: New test.
--- gcc/builtins.c.jj 2006-04-19 17:16:09.000000000 +0200
+++ gcc/builtins.c 2006-04-26 14:48:09.000000000 +0200
@@ -3394,7 +3394,7 @@ expand_builtin_memset (tree arglist, rtx
enum built_in_function fcode;
char c;
unsigned int dest_align;
- rtx dest_mem, dest_addr, len_rtx;
+ rtx dest_mem, dest_addr, len_rtx, val_rtx = NULL;
dest_align = get_pointer_alignment (dest, BIGGEST_ALIGNMENT);
@@ -3411,18 +3411,12 @@ expand_builtin_memset (tree arglist, rtx
return expand_expr (dest, target, mode, EXPAND_NORMAL);
}
- /* Stabilize the arguments in case we fail. */
- dest = builtin_save_expr (dest);
- val = builtin_save_expr (val);
- len = builtin_save_expr (len);
-
len_rtx = expand_normal (len);
dest_mem = get_memory_rtx (dest, len);
if (TREE_CODE (val) != INTEGER_CST)
{
tree cval;
- rtx val_rtx;
cval = fold_build1 (CONVERT_EXPR, unsigned_char_type_node, val);
val_rtx = expand_normal (cval);
@@ -3487,10 +3481,30 @@ expand_builtin_memset (tree arglist, rtx
fndecl = get_callee_fndecl (orig_exp);
fcode = DECL_FUNCTION_CODE (fndecl);
gcc_assert (fcode == BUILT_IN_MEMSET || fcode == BUILT_IN_BZERO);
- arglist = build_tree_list (NULL_TREE, len);
+ arglist = build_tree_list (NULL_TREE,
+ make_tree (TREE_TYPE (len), len_rtx));
if (fcode == BUILT_IN_MEMSET)
- arglist = tree_cons (NULL_TREE, val, arglist);
- arglist = tree_cons (NULL_TREE, dest, arglist);
+ {
+ if (val_rtx)
+ {
+ tree arg;
+
+ /* Avoid useless zero-extension, memset argument is int
+ rather than unsigned char. */
+ if (GET_CODE (val_rtx) == SUBREG
+ && GET_MODE (val_rtx) == TYPE_MODE (unsigned_char_type_node)
+ && GET_MODE (SUBREG_REG (val_rtx))
+ == TYPE_MODE (integer_type_node))
+ arg = make_tree (integer_type_node, SUBREG_REG (val_rtx));
+ else
+ arg = make_tree (unsigned_char_type_node, val_rtx);
+ val = fold_convert (TREE_TYPE (val), arg);
+ }
+ arglist = tree_cons (NULL_TREE, val, arglist);
+ }
+ arglist = tree_cons (NULL_TREE,
+ make_tree (ptr_type_node, XEXP (dest_mem, 0)),
+ arglist);
fn = build_function_call_expr (fndecl, arglist);
if (TREE_CODE (fn) == CALL_EXPR)
CALL_EXPR_TAILCALL (fn) = CALL_EXPR_TAILCALL (orig_exp);
--- gcc/testsuite/gcc.c-torture/execute/20060426-1.c.jj 2006-04-26 12:11:53.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/20060426-1.c 2006-04-26 12:11:35.000000000 +0200
@@ -0,0 +1,33 @@
+/* PR middle-end/27260 */
+
+extern void abort (void);
+extern void *memset (void *, int, __SIZE_TYPE__);
+
+char buf[65];
+
+void
+foo (int x)
+{
+ memset (buf, x != 2 ? 1 : 0, 64);
+}
+
+int
+main (void)
+{
+ int i;
+ buf[64] = 2;
+ for (i = 0; i < 64; i++)
+ if (buf[i] != 0)
+ abort ();
+ foo (0);
+ for (i = 0; i < 64; i++)
+ if (buf[i] != 1)
+ abort ();
+ foo (2);
+ for (i = 0; i < 64; i++)
+ if (buf[i] != 0)
+ abort ();
+ if (buf[64] != 2)
+ abort ();
+ return 0;
+}
Jakub