From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9552 invoked by alias); 25 Aug 2007 07:32:21 -0000 Received: (qmail 9455 invoked by uid 22791); 25 Aug 2007 07:32:19 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 25 Aug 2007 07:32:13 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.1/8.13.1) with ESMTP id l7P7VAeT032100; Sat, 25 Aug 2007 03:31:10 -0400 Received: from devserv.devel.redhat.com (devserv.devel.redhat.com [172.16.58.1]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l7P7VA2N000608; Sat, 25 Aug 2007 03:31:10 -0400 Received: from devserv.devel.redhat.com (localhost.localdomain [127.0.0.1]) by devserv.devel.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id l7P7VAL2001912; Sat, 25 Aug 2007 03:31:10 -0400 Received: (from jakub@localhost) by devserv.devel.redhat.com (8.12.11.20060308/8.12.11/Submit) id l7P7VAHf001910; Sat, 25 Aug 2007 03:31:10 -0400 Date: Sat, 25 Aug 2007 09:58:00 -0000 From: Jakub Jelinek To: Sandra Loosemore Cc: GCC Patches , Nigel Stephens , Guy Morrogh , David Ung , Thiemo Seufer , Mark Mitchell , richard@codesourcery.com Subject: Re: [committed] Re: PATCH: fine-tuning for can_store_by_pieces Message-ID: <20070825073109.GK2063@devserv.devel.redhat.com> Reply-To: Jakub Jelinek References: <46CA222D.2050107@codesourcery.com> <87ps1h5mda.fsf@firetop.home> <46CAEBCE.3050807@codesourcery.com> <87r6lx3r9p.fsf@firetop.home> <46CB4B99.5010501@codesourcery.com> <87zm0k39gj.fsf@firetop.home> <46CD9828.3040305@codesourcery.com> <87hcmqe2sp.fsf@firetop.home> <46CF7332.3000706@codesourcery.com> <20070825065954.GJ2063@devserv.devel.redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="UlxN1C6awaFNesUv" Content-Disposition: inline In-Reply-To: <20070825065954.GJ2063@devserv.devel.redhat.com> User-Agent: Mutt/1.4.1i X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2007-08/txt/msg01700.txt.bz2 --UlxN1C6awaFNesUv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 3273 On Sat, Aug 25, 2007 at 02:59:54AM -0400, Jakub Jelinek wrote: > *************** store_expr (tree exp, rtx target, int ca > *** 4507,4513 **** > str_copy_len, builtin_strncpy_read_str, > (void *) TREE_STRING_POINTER (exp), > MEM_ALIGN (target), > ! exp_len > str_copy_len ? 1 : 0); > if (exp_len > str_copy_len) > clear_storage (dest_mem, GEN_INT (exp_len - str_copy_len), > BLOCK_OP_NORMAL); > --- 4520,4527 ---- > str_copy_len, builtin_strncpy_read_str, > (void *) TREE_STRING_POINTER (exp), > MEM_ALIGN (target), > ! exp_len > str_copy_len ? 1 : 0, > ! false); > if (exp_len > str_copy_len) > clear_storage (dest_mem, GEN_INT (exp_len - str_copy_len), > BLOCK_OP_NORMAL); > > This is wrong. You added the memsetp argument to store_by_pieces > as the 6th, before the endp argument, but you are passing > exp_len > str_copy_len ? 1 : 0 as memsetp and false as endp. > This will certainly screw up say > struct A { char a[20]; }; > void foo (void) > { > struct A a = { "abc" }; > bar (&a); > } > because in that case exp_len (20) is bigger than str_copy_len > and so clear_storage is needed for the rest of the array. > If we pass false == 0 as endp, it means the returned endp > will point to the beginning of the array (&a.a[0]), rather > than where store_by_pieces stopped. Surprised this wasn't caught up by make check, I have committed following as obvious. This new testcase at least on x86_64-linux and i686-linux fails after your patch and no longer does after swapping the arguments. Jakub --UlxN1C6awaFNesUv Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=O Content-length: 1815 2007-08-25 Jakub Jelinek * expr.c (store_expr): Fix order of store_by_pieces arguments. * gcc.dg/array-init-2.c: New test. --- gcc/expr.c.jj 2007-08-25 09:06:46.000000000 +0200 +++ gcc/expr.c 2007-08-25 09:24:07.000000000 +0200 @@ -4519,9 +4519,8 @@ store_expr (tree exp, rtx target, int ca dest_mem = store_by_pieces (dest_mem, str_copy_len, builtin_strncpy_read_str, (void *) TREE_STRING_POINTER (exp), - MEM_ALIGN (target), - exp_len > str_copy_len ? 1 : 0, - false); + MEM_ALIGN (target), false, + exp_len > str_copy_len ? 1 : 0); if (exp_len > str_copy_len) clear_storage (dest_mem, GEN_INT (exp_len - str_copy_len), BLOCK_OP_NORMAL); --- gcc/testsuite/gcc.dg/array-init-2.c.jj 2007-08-25 09:23:19.000000000 +0200 +++ gcc/testsuite/gcc.dg/array-init-2.c 2007-08-25 09:17:39.000000000 +0200 @@ -0,0 +1,51 @@ +/* Test array initializion by store_by_pieces. */ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +struct A { char c[10]; }; +extern void abort (void); + +void +__attribute__((noinline)) +check (struct A * a, int b) +{ + const char *p; + switch (b) + { + case 0: + p = "abcdefghi"; + break; + case 1: + p = "j\0\0\0\0\0\0\0\0"; + break; + case 2: + p = "kl\0\0\0\0\0\0\0"; + break; + case 3: + p = "mnop\0\0\0\0\0"; + break; + case 4: + p = "qrstuvwx\0"; + break; + default: + abort (); + } + if (__builtin_memcmp (a->c, p, 10) != 0) + abort (); +} + +int +main (void) +{ + struct A a = { "abcdefghi" }; + check (&a, 0); + struct A b = { "j" }; + check (&b, 1); + struct A c = { "kl" }; + check (&c, 2); + struct A d = { "mnop" }; + check (&d, 3); + struct A e = { "qrstuvwx" }; + check (&e, 4); + return 0; +} --UlxN1C6awaFNesUv--