From patchwork Fri Apr 1 15:54:29 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 89300 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 81EA7B6F8F for ; Sat, 2 Apr 2011 02:55:03 +1100 (EST) Received: (qmail 31643 invoked by alias); 1 Apr 2011 15:55:01 -0000 Received: (qmail 31622 invoked by uid 22791); 1 Apr 2011 15:54:59 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 01 Apr 2011 15:54:54 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p31FsVf6021674 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 1 Apr 2011 11:54:31 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p31FsUTJ025964 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 1 Apr 2011 11:54:31 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (localhost.localdomain [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id p31FsUwH005664; Fri, 1 Apr 2011 17:54:30 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id p31FsU1h005662; Fri, 1 Apr 2011 17:54:30 +0200 Date: Fri, 1 Apr 2011 17:54:29 +0200 From: Jakub Jelinek To: Eric Botcazou Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix expansion issues on type changing MEM_REFs on LHS (PR middle-end/48335) Message-ID: <20110401155429.GB18914@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek References: <20110330160546.GP18914@tyan-ft48-01.lab.bos.redhat.com> <201104011252.04323.ebotcazou@adacore.com> <20110401112943.GW18914@tyan-ft48-01.lab.bos.redhat.com> <201104011735.54196.ebotcazou@adacore.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <201104011735.54196.ebotcazou@adacore.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Fri, Apr 01, 2011 at 05:35:53PM +0200, Eric Botcazou wrote: > > result is set there from store_field, and result is only used in > > if (result) > > preserve_temp_slots (result); > > afterwards. store_field might want to preserve_temp_slots perhaps, so > > clearing result afterwards might be wrong. > > You're very likely right, but then this should apply to the case just above. See the patch I've posted in the mean time. > --- expmed.c (revision 171818) > +++ expmed.c (working copy) > @@ -419,7 +419,9 @@ store_bit_field_1 (rtx str_rtx, unsigned > && (!MEM_P (op0) > ? ((GET_MODE_SIZE (fieldmode) >= UNITS_PER_WORD > || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode)) > - && byte_offset % GET_MODE_SIZE (fieldmode) == 0) > + && ((GET_MODE (op0) == fieldmode && byte_offset == 0) > + || validate_subreg (fieldmode, GET_MODE (op0), op0, > + byte_offset))) > : (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0)) > || (offset * BITS_PER_UNIT % bitsize == 0 > && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0)))) > > should work. You're right. I'm now bootstrapping/regtesting following adjusted patch, ok if it passes? 2011-04-01 Jakub Jelinek PR middle-end/48335 * expr.c (expand_assignment): Handle all possibilities if TO_RTX is CONCAT. * expmed.c (store_bit_field_1): Avoid trying to create invalid SUBREGs. (store_split_bit_field): If SUBREG_REG (op0) or op0 itself has smaller mode than word, return it for offset 0 and const0_rtx for out-of-bounds stores. If word is const0_rtx, skip it. * gcc.c-torture/compile/pr48335-1.c: New test. * gcc.dg/pr48335-1.c: New test. * gcc.dg/pr48335-2.c: New test. * gcc.dg/pr48335-3.c: New test. * gcc.dg/pr48335-4.c: New test. * gcc.dg/pr48335-5.c: New test. * gcc.dg/pr48335-6.c: New test. * gcc.dg/pr48335-7.c: New test. * gcc.dg/pr48335-8.c: New test. * gcc.target/i386/pr48335-1.c: New test. Jakub --- gcc/expr.c.jj 2011-04-01 13:35:51.113644464 +0200 +++ gcc/expr.c 2011-04-01 13:41:36.585402545 +0200 @@ -4280,16 +4280,47 @@ expand_assignment (tree to, tree from, b /* Handle expand_expr of a complex value returning a CONCAT. */ else if (GET_CODE (to_rtx) == CONCAT) { - if (COMPLEX_MODE_P (TYPE_MODE (TREE_TYPE (from)))) + unsigned short mode_bitsize = GET_MODE_BITSIZE (GET_MODE (to_rtx)); + if (COMPLEX_MODE_P (TYPE_MODE (TREE_TYPE (from))) + && bitpos == 0 + && bitsize == mode_bitsize) + result = store_expr (from, to_rtx, false, nontemporal); + else if (bitsize == mode_bitsize / 2 + && (bitpos == 0 || bitpos == mode_bitsize / 2)) + result = store_expr (from, XEXP (to_rtx, bitpos != 0), false, + nontemporal); + else if (bitpos + bitsize <= mode_bitsize / 2) + result = store_field (XEXP (to_rtx, 0), bitsize, bitpos, + mode1, from, TREE_TYPE (tem), + get_alias_set (to), nontemporal); + else if (bitpos >= mode_bitsize / 2) + result = store_field (XEXP (to_rtx, 1), bitsize, + bitpos - mode_bitsize / 2, mode1, from, + TREE_TYPE (tem), get_alias_set (to), + nontemporal); + else if (bitpos == 0 && bitsize == mode_bitsize) { - gcc_assert (bitpos == 0); - result = store_expr (from, to_rtx, false, nontemporal); + rtx from_rtx; + result = expand_normal (from); + from_rtx = simplify_gen_subreg (GET_MODE (to_rtx), result, + TYPE_MODE (TREE_TYPE (from)), 0); + emit_move_insn (XEXP (to_rtx, 0), + read_complex_part (from_rtx, false)); + emit_move_insn (XEXP (to_rtx, 1), + read_complex_part (from_rtx, true)); } else { - gcc_assert (bitpos == 0 || bitpos == GET_MODE_BITSIZE (mode1)); - result = store_expr (from, XEXP (to_rtx, bitpos != 0), false, - nontemporal); + rtx temp = assign_stack_temp (GET_MODE (to_rtx), + GET_MODE_SIZE (GET_MODE (to_rtx)), + 0); + write_complex_part (temp, XEXP (to_rtx, 0), false); + write_complex_part (temp, XEXP (to_rtx, 1), true); + result = store_field (temp, bitsize, bitpos, mode1, from, + TREE_TYPE (tem), get_alias_set (to), + nontemporal); + emit_move_insn (XEXP (to_rtx, 0), read_complex_part (temp, false)); + emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp, true)); } } else --- gcc/expmed.c.jj 2011-04-01 13:35:51.153501638 +0200 +++ gcc/expmed.c 2011-04-01 17:48:32.766401928 +0200 @@ -418,8 +418,10 @@ store_bit_field_1 (rtx str_rtx, unsigned && bitsize == GET_MODE_BITSIZE (fieldmode) && (!MEM_P (op0) ? ((GET_MODE_SIZE (fieldmode) >= UNITS_PER_WORD - || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode)) - && byte_offset % GET_MODE_SIZE (fieldmode) == 0) + || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode)) + && ((GET_MODE (op0) == fieldmode && byte_offset == 0) + || validate_subreg (fieldmode, GET_MODE (op0), op0, + byte_offset))) : (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0)) || (offset * BITS_PER_UNIT % bitsize == 0 && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0)))) @@ -479,6 +481,7 @@ store_bit_field_1 (rtx str_rtx, unsigned struct expand_operand ops[2]; enum insn_code icode = optab_handler (movstrict_optab, fieldmode); rtx arg0 = op0; + unsigned HOST_WIDE_INT subreg_off; if (GET_CODE (arg0) == SUBREG) { @@ -491,15 +494,18 @@ store_bit_field_1 (rtx str_rtx, unsigned arg0 = SUBREG_REG (arg0); } - arg0 = gen_rtx_SUBREG (fieldmode, arg0, - (bitnum % BITS_PER_WORD) / BITS_PER_UNIT - + (offset * UNITS_PER_WORD)); - - create_fixed_operand (&ops[0], arg0); - /* Shrink the source operand to FIELDMODE. */ - create_convert_operand_to (&ops[1], value, fieldmode, false); - if (maybe_expand_insn (icode, 2, ops)) - return true; + subreg_off = (bitnum % BITS_PER_WORD) / BITS_PER_UNIT + + (offset * UNITS_PER_WORD); + if (validate_subreg (fieldmode, GET_MODE (arg0), arg0, subreg_off)) + { + arg0 = gen_rtx_SUBREG (fieldmode, arg0, subreg_off); + + create_fixed_operand (&ops[0], arg0); + /* Shrink the source operand to FIELDMODE. */ + create_convert_operand_to (&ops[1], value, fieldmode, false); + if (maybe_expand_insn (icode, 2, ops)) + return true; + } } /* Handle fields bigger than a word. */ @@ -1045,22 +1051,32 @@ store_split_bit_field (rtx op0, unsigned if (GET_CODE (op0) == SUBREG) { int word_offset = (SUBREG_BYTE (op0) / UNITS_PER_WORD) + offset; - word = operand_subword_force (SUBREG_REG (op0), word_offset, - GET_MODE (SUBREG_REG (op0))); + enum machine_mode sub_mode = GET_MODE (SUBREG_REG (op0)); + if (sub_mode != BLKmode && GET_MODE_SIZE (sub_mode) < UNITS_PER_WORD) + word = word_offset ? const0_rtx : op0; + else + word = operand_subword_force (SUBREG_REG (op0), word_offset, + GET_MODE (SUBREG_REG (op0))); offset = 0; } else if (REG_P (op0)) { - word = operand_subword_force (op0, offset, GET_MODE (op0)); + enum machine_mode op0_mode = GET_MODE (op0); + if (op0_mode != BLKmode && GET_MODE_SIZE (op0_mode) < UNITS_PER_WORD) + word = offset ? const0_rtx : op0; + else + word = operand_subword_force (op0, offset, GET_MODE (op0)); offset = 0; } else word = op0; /* OFFSET is in UNITs, and UNIT is in bits. - store_fixed_bit_field wants offset in bytes. */ - store_fixed_bit_field (word, offset * unit / BITS_PER_UNIT, thissize, - thispos, part); + store_fixed_bit_field wants offset in bytes. If WORD is const0_rtx, + it is just an out-of-bounds access. Ignore it. */ + if (word != const0_rtx) + store_fixed_bit_field (word, offset * unit / BITS_PER_UNIT, thissize, + thispos, part); bitsdone += thissize; } } --- gcc/testsuite/gcc.c-torture/compile/pr48335-1.c.jj 2011-04-01 13:36:42.066554469 +0200 +++ gcc/testsuite/gcc.c-torture/compile/pr48335-1.c 2011-04-01 13:36:42.066554469 +0200 @@ -0,0 +1,41 @@ +/* PR middle-end/48335 */ + +struct S { float d; }; + +void bar (struct S); + +void +f0 (int x) +{ + struct S s = {.d = 0.0f }; + ((char *) &s.d)[0] = x; + s.d *= 7.0; + bar (s); +} + +void +f1 (int x) +{ + struct S s = {.d = 0.0f }; + ((char *) &s.d)[1] = x; + s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = {.d = 0.0f }; + ((char *) &s.d)[2] = x; + s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = {.d = 0.0f }; + ((char *) &s.d)[3] = x; + s.d *= 7.0; + bar (s); +} --- gcc/testsuite/gcc.dg/pr48335-1.c.jj 2011-04-01 13:36:42.067670347 +0200 +++ gcc/testsuite/gcc.dg/pr48335-1.c 2011-04-01 13:36:42.067670347 +0200 @@ -0,0 +1,48 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef long long T __attribute__((may_alias)); + +struct S +{ + _Complex float d __attribute__((aligned (8))); +}; + +void bar (struct S); + +void +f1 (T x) +{ + struct S s; + *(T *) &s.d = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = { .d = 0.0f }; + *(char *) &s.d = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = { .d = 0.0f }; + ((char *) &s.d)[2] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f4 (int x, int y) +{ + struct S s = { .d = 0.0f }; + ((char *) &s.d)[y] = x; + __real__ s.d *= 7.0; + bar (s); +} --- gcc/testsuite/gcc.dg/pr48335-2.c.jj 2011-04-01 13:36:42.068398915 +0200 +++ gcc/testsuite/gcc.dg/pr48335-2.c 2011-04-01 13:36:42.068398915 +0200 @@ -0,0 +1,58 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef long long T __attribute__((may_alias, aligned (1))); +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + _Complex float d __attribute__((aligned (8))); +}; + +void bar (struct S); + +void +f1 (T x) +{ + struct S s; + *(T *) ((char *) &s.d + 1) = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) &s.d + 1))[0] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) &s.d + 1))[1] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f4 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) &s.d + 1))[2] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f5 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) &s.d + 1))[3] = x; + __real__ s.d *= 7.0; + bar (s); +} --- gcc/testsuite/gcc.dg/pr48335-3.c.jj 2011-04-01 13:36:42.068398915 +0200 +++ gcc/testsuite/gcc.dg/pr48335-3.c 2011-04-01 13:36:42.068398915 +0200 @@ -0,0 +1,48 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + double d; +}; + +void bar (struct S); + +void +f1 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[0] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[1] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[2] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f4 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[3] = x; + __real__ s.d *= 7.0; + bar (s); +} --- gcc/testsuite/gcc.dg/pr48335-4.c.jj 2011-04-01 13:36:42.069670582 +0200 +++ gcc/testsuite/gcc.dg/pr48335-4.c 2011-04-01 13:36:42.069670582 +0200 @@ -0,0 +1,39 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + double d; +}; + +void bar (struct S); + +void +f1 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[-1] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[-2] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = { .d = 0.0 }; + ((U *)((char *) &s.d + 1))[5] = x; + __real__ s.d *= 7.0; + bar (s); +} --- gcc/testsuite/gcc.dg/pr48335-5.c.jj 2011-04-01 13:36:42.069670582 +0200 +++ gcc/testsuite/gcc.dg/pr48335-5.c 2011-04-01 13:36:42.069670582 +0200 @@ -0,0 +1,38 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef long long T __attribute__((may_alias)); + +struct S +{ + _Complex float d __attribute__((aligned (8))); +}; + +int +f1 (struct S x) +{ + struct S s = x; + return *(T *) &s.d; +} + +int +f2 (struct S x) +{ + struct S s = x; + return *(char *) &s.d; +} + +int +f3 (struct S x) +{ + struct S s = x; + return ((char *) &s.d)[2]; +} + +int +f4 (struct S x, int y) +{ + struct S s = x; + return ((char *) &s.d)[y]; +} --- gcc/testsuite/gcc.dg/pr48335-6.c.jj 2011-04-01 13:36:42.070670448 +0200 +++ gcc/testsuite/gcc.dg/pr48335-6.c 2011-04-01 13:36:42.070670448 +0200 @@ -0,0 +1,46 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef long long T __attribute__((may_alias, aligned (1))); +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + _Complex float d __attribute__((aligned (8))); +}; + +T +f1 (struct S x) +{ + struct S s = x; + return *(T *) ((char *) &s.d + 1); +} + +int +f2 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[0]; +} + +int +f3 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[1]; +} + +int +f4 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[2]; +} + +int +f5 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[3]; +} --- gcc/testsuite/gcc.dg/pr48335-7.c.jj 2011-04-01 13:36:42.070670448 +0200 +++ gcc/testsuite/gcc.dg/pr48335-7.c 2011-04-01 13:36:42.070670448 +0200 @@ -0,0 +1,38 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + double d; +}; + +int +f1 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[0]; +} + +int +f2 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[1]; +} + +int +f3 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[2]; +} + +int +f4 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[3]; +} --- gcc/testsuite/gcc.dg/pr48335-8.c.jj 2011-04-01 13:36:42.071670340 +0200 +++ gcc/testsuite/gcc.dg/pr48335-8.c 2011-04-01 13:36:42.071670340 +0200 @@ -0,0 +1,31 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra" } */ + +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + double d; +}; + +int +f1 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[-1]; +} + +int +f2 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[-2]; +} + +int +f3 (struct S x) +{ + struct S s = x; + return ((U *)((char *) &s.d + 1))[5]; +} --- gcc/testsuite/gcc.target/i386/pr48335-1.c.jj 2011-04-01 13:36:42.072683389 +0200 +++ gcc/testsuite/gcc.target/i386/pr48335-1.c 2011-04-01 13:36:42.072683389 +0200 @@ -0,0 +1,32 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-sra -msse2" } */ + +#include + +typedef __float128 T __attribute__((may_alias)); + +struct S +{ + _Complex double d __attribute__((aligned (16))); +}; + +void bar (struct S); + +void +f1 (T x) +{ + struct S s; + *(T *) &s.d = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (__m128d x) +{ + struct S s; + _mm_store_pd ((double *) &s.d, x); + __real__ s.d *= 7.0; + bar (s); +}