diff mbox

[4.6] Workaround for stack slot sharing problems with unrolling (PR fortran/49103)

Message ID 20110613203858.GY17079@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek June 13, 2011, 8:38 p.m. UTC 
On Tue, Jun 07, 2011 at 12:24:06PM +0200, Richard Guenther wrote:
> Probably easier and more complete to do
> 
>             if (lhs && TREE_CODE (lhs) != SSA_NAME)
>               {
>                  tree base = 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.  At least reverting
> 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 for
4.6?

2011-06-13  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/49103
	* tree.h (DECL_NONSHAREABLE): Define.
	(struct tree_decl_common): Change decl_common_unused to
	decl_nonshareable_flag.
	* cfgexpand.c (expand_used_vars_for_block, clear_tree_used):
	Ignore vars with DECL_NONSHAREABLE bit set.
	* tree-cfg.c (gimple_duplicate_bb): Set DECL_NONSHAREABLE
	on stores to automatic aggregate vars.

	* gfortran.dg/pr49103.f90: New test.



	Jakub

Comments

Richard Biener June 14, 2011, 10:10 a.m. UTC | #1 
On Mon, Jun 13, 2011 at 10:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Jun 07, 2011 at 12:24:06PM +0200, Richard Guenther wrote:
>> Probably easier and more complete to do
>>
>>             if (lhs && TREE_CODE (lhs) != SSA_NAME)
>>               {
>>                  tree base = 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.  At least reverting
>> 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 for
> 4.6?

Yes, and for trunk.  Micha can revert it there when his patch goes in.

Thanks,
Richard.

> 2011-06-13  Jakub Jelinek  <jakub@redhat.com>
>
>        PR fortran/49103
>        * tree.h (DECL_NONSHAREABLE): Define.
>        (struct tree_decl_common): Change decl_common_unused to
>        decl_nonshareable_flag.
>        * cfgexpand.c (expand_used_vars_for_block, clear_tree_used):
>        Ignore vars with DECL_NONSHAREABLE bit set.
>        * tree-cfg.c (gimple_duplicate_bb): Set DECL_NONSHAREABLE
>        on stores to automatic aggregate vars.
>
>        * gfortran.dg/pr49103.f90: New test.
>
> --- gcc/tree.h.jj       2011-03-14 14:12:15.000000000 +0100
> +++ gcc/tree.h  2011-05-31 14:05:34.000000000 +0200
> @@ -1330,6 +1330,10 @@ extern void omp_clause_range_check_faile
>  #define DECL_READ_P(NODE) \
>   (TREE_CHECK2 (NODE, VAR_DECL, PARM_DECL)->decl_common.decl_read_flag)
>
> +#define DECL_NONSHAREABLE(NODE) \
> +  (TREE_CHECK2 (NODE, VAR_DECL, \
> +               RESULT_DECL)->decl_common.decl_nonshareable_flag)
> +
>  /* In a CALL_EXPR, means that the call is the jump from a thunk to the
>    thunked-to function.  */
>  #define CALL_FROM_THUNK_P(NODE) (CALL_EXPR_CHECK (NODE)->base.protected_flag)
> @@ -2787,8 +2791,9 @@ struct GTY(()) tree_decl_common {
>      being set.  */
>   unsigned decl_read_flag : 1;
>
> -  /* Padding so that 'off_align' can be on a 32-bit boundary.  */
> -  unsigned decl_common_unused : 1;
> +  /* In VAR_DECL or RESULT_DECL set when significant code movement precludes
> +     attempting to share the stack slot with some other variable.  */
> +  unsigned decl_nonshareable_flag : 1;
>
>   /* DECL_OFFSET_ALIGN, used only for FIELD_DECLs.  */
>   unsigned int off_align : 8;
> --- gcc/cfgexpand.c.jj  2011-05-04 10:46:52.000000000 +0200
> +++ gcc/cfgexpand.c     2011-05-31 14:08:36.000000000 +0200
> @@ -1134,7 +1134,9 @@ expand_used_vars_for_block (tree block,
>
>   /* Expand all variables at this level.  */
>   for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
> -    if (TREE_USED (t))
> +    if (TREE_USED (t)
> +        && ((TREE_CODE (t) != VAR_DECL && TREE_CODE (t) != RESULT_DECL)
> +           || !DECL_NONSHAREABLE (t)))
>       expand_one_var (t, toplevel, true);
>
>   this_sv_num = stack_vars_num;
> @@ -1167,6 +1169,8 @@ clear_tree_used (tree block)
>
>   for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
>     /* if (!TREE_STATIC (t) && !DECL_EXTERNAL (t)) */
> +    if ((TREE_CODE (t) != VAR_DECL && TREE_CODE (t) != RESULT_DECL)
> +       || !DECL_NONSHAREABLE (t))
>       TREE_USED (t) = 0;
>
>   for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t))
> --- gcc/tree-cfg.c.jj   2011-03-14 14:12:15.000000000 +0100
> +++ gcc/tree-cfg.c      2011-06-13 19:34:18.000000000 +0200
> @@ -5117,6 +5117,7 @@ gimple_duplicate_bb (basic_block bb)
>     {
>       def_operand_p def_p;
>       ssa_op_iter op_iter;
> +      tree lhs;
>
>       stmt = gsi_stmt (gsi);
>       if (gimple_code (stmt) == GIMPLE_LABEL)
> @@ -5130,6 +5131,24 @@ gimple_duplicate_bb (basic_block bb)
>       maybe_duplicate_eh_stmt (copy, stmt);
>       gimple_duplicate_stmt_histograms (cfun, copy, cfun, stmt);
>
> +      /* When copying around a stmt writing into a local non-user
> +        aggregate, make sure it won't share stack slot with other
> +        vars.  */
> +      lhs = gimple_get_lhs (stmt);
> +      if (lhs && TREE_CODE (lhs) != SSA_NAME)
> +       {
> +         tree base = get_base_address (lhs);
> +         if (base
> +             && (TREE_CODE (base) == VAR_DECL
> +                 || TREE_CODE (base) == RESULT_DECL)
> +             && DECL_IGNORED_P (base)
> +             && !TREE_STATIC (base)
> +             && !DECL_EXTERNAL (base)
> +             && (TREE_CODE (base) != VAR_DECL
> +                 || !DECL_HAS_VALUE_EXPR_P (base)))
> +           DECL_NONSHAREABLE (base) = 1;
> +       }
> +
>       /* Create new names for all the definitions created by COPY and
>         add replacement mappings for each new name.  */
>       FOR_EACH_SSA_DEF_OPERAND (def_p, copy, op_iter, SSA_OP_ALL_DEFS)
> --- gcc/testsuite/gfortran.dg/pr49103.f90.jj    2011-05-31 13:52:43.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/pr49103.f90       2011-05-31 13:57:16.000000000 +0200
> @@ -0,0 +1,19 @@
> +! PR fortran/49103
> +! { dg-do run }
> +  integer :: a(2), b(2), i, j
> +  open (10, status='scratch')
> +  do j = 1, 2
> +    a = (/ 0, 0 /)
> +    b = (/ 1, 1 /)
> +    do i = 1, 2
> +      write (10, *) a
> +      write (10, *) b
> +    end do
> +  end do
> +  rewind (10)
> +  do i = 0, 7
> +    read (10, *) a
> +    if (any (a .ne. mod (i, 2))) call abort
> +  end do
> +  close (10)
> +end
>
>
>        Jakub
>
diff mbox

Patch

--- gcc/tree.h.jj	2011-03-14 14:12:15.000000000 +0100
+++ gcc/tree.h	2011-05-31 14:05:34.000000000 +0200
@@ -1330,6 +1330,10 @@  extern void omp_clause_range_check_faile
 #define DECL_READ_P(NODE) \
   (TREE_CHECK2 (NODE, VAR_DECL, PARM_DECL)->decl_common.decl_read_flag)
 
+#define DECL_NONSHAREABLE(NODE) \
+  (TREE_CHECK2 (NODE, VAR_DECL, \
+		RESULT_DECL)->decl_common.decl_nonshareable_flag)
+
 /* In a CALL_EXPR, means that the call is the jump from a thunk to the
    thunked-to function.  */
 #define CALL_FROM_THUNK_P(NODE) (CALL_EXPR_CHECK (NODE)->base.protected_flag)
@@ -2787,8 +2791,9 @@  struct GTY(()) tree_decl_common {
      being set.  */
   unsigned decl_read_flag : 1;
 
-  /* Padding so that 'off_align' can be on a 32-bit boundary.  */
-  unsigned decl_common_unused : 1;
+  /* In VAR_DECL or RESULT_DECL set when significant code movement precludes
+     attempting to share the stack slot with some other variable.  */
+  unsigned decl_nonshareable_flag : 1;
 
   /* DECL_OFFSET_ALIGN, used only for FIELD_DECLs.  */
   unsigned int off_align : 8;
--- gcc/cfgexpand.c.jj	2011-05-04 10:46:52.000000000 +0200
+++ gcc/cfgexpand.c	2011-05-31 14:08:36.000000000 +0200
@@ -1134,7 +1134,9 @@  expand_used_vars_for_block (tree block, 
 
   /* Expand all variables at this level.  */
   for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
-    if (TREE_USED (t))
+    if (TREE_USED (t)
+        && ((TREE_CODE (t) != VAR_DECL && TREE_CODE (t) != RESULT_DECL)
+	    || !DECL_NONSHAREABLE (t)))
       expand_one_var (t, toplevel, true);
 
   this_sv_num = stack_vars_num;
@@ -1167,6 +1169,8 @@  clear_tree_used (tree block)
 
   for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
     /* if (!TREE_STATIC (t) && !DECL_EXTERNAL (t)) */
+    if ((TREE_CODE (t) != VAR_DECL && TREE_CODE (t) != RESULT_DECL)
+	|| !DECL_NONSHAREABLE (t))
       TREE_USED (t) = 0;
 
   for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t))
--- gcc/tree-cfg.c.jj	2011-03-14 14:12:15.000000000 +0100
+++ gcc/tree-cfg.c	2011-06-13 19:34:18.000000000 +0200
@@ -5117,6 +5117,7 @@  gimple_duplicate_bb (basic_block bb)
     {
       def_operand_p def_p;
       ssa_op_iter op_iter;
+      tree lhs;
 
       stmt = gsi_stmt (gsi);
       if (gimple_code (stmt) == GIMPLE_LABEL)
@@ -5130,6 +5131,24 @@  gimple_duplicate_bb (basic_block bb)
       maybe_duplicate_eh_stmt (copy, stmt);
       gimple_duplicate_stmt_histograms (cfun, copy, cfun, stmt);
 
+      /* When copying around a stmt writing into a local non-user
+	 aggregate, make sure it won't share stack slot with other
+	 vars.  */
+      lhs = gimple_get_lhs (stmt);
+      if (lhs && TREE_CODE (lhs) != SSA_NAME)
+	{
+	  tree base = get_base_address (lhs);
+	  if (base
+	      && (TREE_CODE (base) == VAR_DECL
+		  || TREE_CODE (base) == RESULT_DECL)
+	      && DECL_IGNORED_P (base)
+	      && !TREE_STATIC (base)
+	      && !DECL_EXTERNAL (base)
+	      && (TREE_CODE (base) != VAR_DECL
+		  || !DECL_HAS_VALUE_EXPR_P (base)))
+	    DECL_NONSHAREABLE (base) = 1;
+	}
+
       /* Create new names for all the definitions created by COPY and
 	 add replacement mappings for each new name.  */
       FOR_EACH_SSA_DEF_OPERAND (def_p, copy, op_iter, SSA_OP_ALL_DEFS)
--- gcc/testsuite/gfortran.dg/pr49103.f90.jj	2011-05-31 13:52:43.000000000 +0200
+++ gcc/testsuite/gfortran.dg/pr49103.f90	2011-05-31 13:57:16.000000000 +0200
@@ -0,0 +1,19 @@ 
+! PR fortran/49103
+! { dg-do run }
+  integer :: a(2), b(2), i, j
+  open (10, status='scratch')
+  do j = 1, 2
+    a = (/ 0, 0 /)
+    b = (/ 1, 1 /)
+    do i = 1, 2
+      write (10, *) a
+      write (10, *) b
+    end do
+  end do
+  rewind (10)
+  do i = 0, 7
+    read (10, *) a
+    if (any (a .ne. mod (i, 2))) call abort
+  end do
+  close (10)
+end