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

[Richard Guenther <richard dot guenther at gmail dot com>]

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
>