From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29988 invoked by alias); 14 Jun 2011 10:10:26 -0000 Received: (qmail 29975 invoked by uid 22791); 14 Jun 2011 10:10:25 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST,TW_TM X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Jun 2011 10:10:07 +0000 Received: by wwf26 with SMTP id 26so5123815wwf.8 for ; Tue, 14 Jun 2011 03:10:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.197.83 with SMTP id ej19mr6055761wbb.105.1308046205444; Tue, 14 Jun 2011 03:10:05 -0700 (PDT) Received: by 10.227.28.69 with HTTP; Tue, 14 Jun 2011 03:10:05 -0700 (PDT) In-Reply-To: <20110613203858.GY17079@tyan-ft48-01.lab.bos.redhat.com> References: <20110607063150.GY17079@tyan-ft48-01.lab.bos.redhat.com> <20110613203858.GY17079@tyan-ft48-01.lab.bos.redhat.com> Date: Tue, 14 Jun 2011 10:16:00 -0000 Message-ID: Subject: Re: [4.6 PATCH] Workaround for stack slot sharing problems with unrolling (PR fortran/49103) From: Richard Guenther To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: 2011-06/txt/msg01027.txt.bz2 On Mon, Jun 13, 2011 at 10:38 PM, Jakub Jelinek wrote: > On Tue, Jun 07, 2011 at 12:24:06PM +0200, Richard Guenther wrote: >> Probably easier and more complete to do >> >> =A0 =A0 =A0 =A0 =A0 =A0 if (lhs && TREE_CODE (lhs) !=3D SSA_NAME) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tree base =3D get_base_address (lhs); > > Done in the patch below, bootstrapped/regtested again on x86_64-linux and > i686-linux on the 4.6 branch. > >> I don't like the patch too much, but it looks reasonable. =A0At least re= verting >> your patch doesn't really fix anything. >> >> Any opinions from others? > > Michael said the same, anyone else has any opinion or can I check it in f= or > 4.6? Yes, and for trunk. Micha can revert it there when his patch goes in. Thanks, Richard. > 2011-06-13 =A0Jakub Jelinek =A0 > > =A0 =A0 =A0 =A0PR fortran/49103 > =A0 =A0 =A0 =A0* tree.h (DECL_NONSHAREABLE): Define. > =A0 =A0 =A0 =A0(struct tree_decl_common): Change decl_common_unused to > =A0 =A0 =A0 =A0decl_nonshareable_flag. > =A0 =A0 =A0 =A0* cfgexpand.c (expand_used_vars_for_block, clear_tree_used= ): > =A0 =A0 =A0 =A0Ignore vars with DECL_NONSHAREABLE bit set. > =A0 =A0 =A0 =A0* tree-cfg.c (gimple_duplicate_bb): Set DECL_NONSHAREABLE > =A0 =A0 =A0 =A0on stores to automatic aggregate vars. > > =A0 =A0 =A0 =A0* gfortran.dg/pr49103.f90: New test. > > --- gcc/tree.h.jj =A0 =A0 =A0 2011-03-14 14:12:15.000000000 +0100 > +++ gcc/tree.h =A02011-05-31 14:05:34.000000000 +0200 > @@ -1330,6 +1330,10 @@ extern void omp_clause_range_check_faile > =A0#define DECL_READ_P(NODE) \ > =A0 (TREE_CHECK2 (NODE, VAR_DECL, PARM_DECL)->decl_common.decl_read_flag) > > +#define DECL_NONSHAREABLE(NODE) \ > + =A0(TREE_CHECK2 (NODE, VAR_DECL, \ > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 RESULT_DECL)->decl_common.decl_nonshareable= _flag) > + > =A0/* In a CALL_EXPR, means that the call is the jump from a thunk to the > =A0 =A0thunked-to function. =A0*/ > =A0#define CALL_FROM_THUNK_P(NODE) (CALL_EXPR_CHECK (NODE)->base.protecte= d_flag) > @@ -2787,8 +2791,9 @@ struct GTY(()) tree_decl_common { > =A0 =A0 =A0being set. =A0*/ > =A0 unsigned decl_read_flag : 1; > > - =A0/* Padding so that 'off_align' can be on a 32-bit boundary. =A0*/ > - =A0unsigned decl_common_unused : 1; > + =A0/* In VAR_DECL or RESULT_DECL set when significant code movement pre= cludes > + =A0 =A0 attempting to share the stack slot with some other variable. = =A0*/ > + =A0unsigned decl_nonshareable_flag : 1; > > =A0 /* DECL_OFFSET_ALIGN, used only for FIELD_DECLs. =A0*/ > =A0 unsigned int off_align : 8; > --- gcc/cfgexpand.c.jj =A02011-05-04 10:46:52.000000000 +0200 > +++ gcc/cfgexpand.c =A0 =A0 2011-05-31 14:08:36.000000000 +0200 > @@ -1134,7 +1134,9 @@ expand_used_vars_for_block (tree block, > > =A0 /* Expand all variables at this level. =A0*/ > =A0 for (t =3D BLOCK_VARS (block); t ; t =3D DECL_CHAIN (t)) > - =A0 =A0if (TREE_USED (t)) > + =A0 =A0if (TREE_USED (t) > + =A0 =A0 =A0 =A0&& ((TREE_CODE (t) !=3D VAR_DECL && TREE_CODE (t) !=3D R= ESULT_DECL) > + =A0 =A0 =A0 =A0 =A0 || !DECL_NONSHAREABLE (t))) > =A0 =A0 =A0 expand_one_var (t, toplevel, true); > > =A0 this_sv_num =3D stack_vars_num; > @@ -1167,6 +1169,8 @@ clear_tree_used (tree block) > > =A0 for (t =3D BLOCK_VARS (block); t ; t =3D DECL_CHAIN (t)) > =A0 =A0 /* if (!TREE_STATIC (t) && !DECL_EXTERNAL (t)) */ > + =A0 =A0if ((TREE_CODE (t) !=3D VAR_DECL && TREE_CODE (t) !=3D RESULT_DE= CL) > + =A0 =A0 =A0 || !DECL_NONSHAREABLE (t)) > =A0 =A0 =A0 TREE_USED (t) =3D 0; > > =A0 for (t =3D BLOCK_SUBBLOCKS (block); t ; t =3D BLOCK_CHAIN (t)) > --- gcc/tree-cfg.c.jj =A0 2011-03-14 14:12:15.000000000 +0100 > +++ gcc/tree-cfg.c =A0 =A0 =A02011-06-13 19:34:18.000000000 +0200 > @@ -5117,6 +5117,7 @@ gimple_duplicate_bb (basic_block bb) > =A0 =A0 { > =A0 =A0 =A0 def_operand_p def_p; > =A0 =A0 =A0 ssa_op_iter op_iter; > + =A0 =A0 =A0tree lhs; > > =A0 =A0 =A0 stmt =3D gsi_stmt (gsi); > =A0 =A0 =A0 if (gimple_code (stmt) =3D=3D GIMPLE_LABEL) > @@ -5130,6 +5131,24 @@ gimple_duplicate_bb (basic_block bb) > =A0 =A0 =A0 maybe_duplicate_eh_stmt (copy, stmt); > =A0 =A0 =A0 gimple_duplicate_stmt_histograms (cfun, copy, cfun, stmt); > > + =A0 =A0 =A0/* When copying around a stmt writing into a local non-user > + =A0 =A0 =A0 =A0aggregate, make sure it won't share stack slot with other > + =A0 =A0 =A0 =A0vars. =A0*/ > + =A0 =A0 =A0lhs =3D gimple_get_lhs (stmt); > + =A0 =A0 =A0if (lhs && TREE_CODE (lhs) !=3D SSA_NAME) > + =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 tree base =3D get_base_address (lhs); > + =A0 =A0 =A0 =A0 if (base > + =A0 =A0 =A0 =A0 =A0 =A0 && (TREE_CODE (base) =3D=3D VAR_DECL > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 || TREE_CODE (base) =3D=3D RESULT_DECL) > + =A0 =A0 =A0 =A0 =A0 =A0 && DECL_IGNORED_P (base) > + =A0 =A0 =A0 =A0 =A0 =A0 && !TREE_STATIC (base) > + =A0 =A0 =A0 =A0 =A0 =A0 && !DECL_EXTERNAL (base) > + =A0 =A0 =A0 =A0 =A0 =A0 && (TREE_CODE (base) !=3D VAR_DECL > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 || !DECL_HAS_VALUE_EXPR_P (base))) > + =A0 =A0 =A0 =A0 =A0 DECL_NONSHAREABLE (base) =3D 1; > + =A0 =A0 =A0 } > + > =A0 =A0 =A0 /* Create new names for all the definitions created by COPY a= nd > =A0 =A0 =A0 =A0 add replacement mappings for each new name. =A0*/ > =A0 =A0 =A0 FOR_EACH_SSA_DEF_OPERAND (def_p, copy, op_iter, SSA_OP_ALL_DE= FS) > --- gcc/testsuite/gfortran.dg/pr49103.f90.jj =A0 =A02011-05-31 13:52:43.0= 00000000 +0200 > +++ gcc/testsuite/gfortran.dg/pr49103.f90 =A0 =A0 =A0 2011-05-31 13:57:16= .000000000 +0200 > @@ -0,0 +1,19 @@ > +! PR fortran/49103 > +! { dg-do run } > + =A0integer :: a(2), b(2), i, j > + =A0open (10, status=3D'scratch') > + =A0do j =3D 1, 2 > + =A0 =A0a =3D (/ 0, 0 /) > + =A0 =A0b =3D (/ 1, 1 /) > + =A0 =A0do i =3D 1, 2 > + =A0 =A0 =A0write (10, *) a > + =A0 =A0 =A0write (10, *) b > + =A0 =A0end do > + =A0end do > + =A0rewind (10) > + =A0do i =3D 0, 7 > + =A0 =A0read (10, *) a > + =A0 =A0if (any (a .ne. mod (i, 2))) call abort > + =A0end do > + =A0close (10) > +end > > > =A0 =A0 =A0 =A0Jakub >