Re: [PATCH] tree-parloops: Fix iteration-private memory decl hoisting [PR83064]

On Fri, Jul 17, 2026 at 6:47 AM <[email protected]> wrote:
>
> From: Dhruv Chawla <[email protected]>
>
> Leave iteration-private decls in place instead of hoisting them, so
> move_sese_region_to_fn gives each thread function its own copy. This fixes the
> testcase mentioned in PR83064, which involves a stack array used as a callee
> return slot getting shared across multiple threads leading to a data race and
> therefore multiple writes to the same location.
>
> This is a conservative way to solve this issue, by assuming that any memory
> decl not referenced outside the loop is private to the iteration. It
> would still be possible to hoist such memory if it was provable that the
> same location would not be written across multiple threads, but that
> analysis would be much more complicated and potentially unlikely to have
> much benefit. Globals are always hoisted.
>
> The gather_escaping_decls function collects those that escape before the loop
> is duplicated by versioning or the exit-first transformation, as that would
> otherwise make a private decl look like it escapes.
>
> This is an alternate attempt to fix PR125717, as this brings back the
> performance on the 800.pot3d_s benchmark to the expected levels. The previous
> attempt was at
> https://gcc.gnu.org/pipermail/gcc-patches/2026-June/720592.html.
>
> Bootstrapped and regtested on aarch64-linux-gnu.
>
> Signed-off-by: Dhruv Chawla <[email protected]>
>
>         PR tree-optimization/83064
>
> gcc/fortran/ChangeLog:
>
>         * trans-stmt.cc (gfc_trans_forall_loop): Emit
>         annot_expr_parallel_kind for do-concurrent loops.
>
> gcc/ChangeLog:
>
>         * tree-parloops.cc (struct elv_data): Add escaping hash_set.
>         (eliminate_local_variables_1): Only hoist escaping local
>         variables.
>         (eliminate_local_variables_stmt): Pass the escaping set through
>         to eliminate_local_variables_1.
>         (record_memory_decl): New function.
>         (gather_escaping_decls): Likewise.
>         (eliminate_local_variables): Pass the escaping set through to
>         eliminate_local_variables_stmt.
>         (gen_parallel_loop): Compute escaping set via
>         gather_escaping_decls and pass to
>         eliminate_local_variables.
>
> gcc/testsuite/ChangeLog:
>
>         * gfortran.dg/do_concurrent_6.f90: Update to handle parallel
>         annotation.
>         * gfortran.dg/do_concurrent_7.f90: Likewise.
>         * gfortran.dg/do_concurrent_16.f90: New test.
> ---
>  gcc/fortran/trans-stmt.cc                     | 12 +--
>  .../gfortran.dg/do_concurrent_16.f90          | 68 +++++++++++++++
>  gcc/testsuite/gfortran.dg/do_concurrent_6.f90 |  2 +-
>  gcc/testsuite/gfortran.dg/do_concurrent_7.f90 |  6 +-
>  gcc/tree-parloops.cc                          | 82 +++++++++++++++++--
>  5 files changed, 154 insertions(+), 16 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_16.f90
>
> diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
> index 526f42febf7..3c9ebdd0274 100644
> --- a/gcc/fortran/trans-stmt.cc
> +++ b/gcc/fortran/trans-stmt.cc
> @@ -4349,13 +4349,13 @@ gfc_trans_forall_loop (forall_info *forall_tmp, tree body,
>        cond = fold_build2_loc (input_location, LE_EXPR, logical_type_node,
>                               count, build_int_cst (TREE_TYPE (count), 0));
>
> -      /* PR 83064 means that we cannot use annot_expr_parallel_kind until
> -       the autoparallelizer can handle this.  */
> +      annot_expr_kind kind = forall_tmp->do_concurrent
> +                              ? annot_expr_parallel_kind
> +                              : annot_expr_ivdep_kind;
>        if (forall_tmp->do_concurrent || iter->annot.ivdep)
> -       cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> -                      build_int_cst (integer_type_node,
> -                                     annot_expr_ivdep_kind),
> -                      integer_zero_node);
> +       cond
> +         = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> +                   build_int_cst (integer_type_node, kind), integer_zero_node);
>
>        if (iter->annot.unroll && cond != error_mark_node)
>         cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> diff --git a/gcc/testsuite/gfortran.dg/do_concurrent_16.f90 b/gcc/testsuite/gfortran.dg/do_concurrent_16.f90
> new file mode 100644
> index 00000000000..a3df15815bb
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/do_concurrent_16.f90
> @@ -0,0 +1,68 @@
> +! { dg-do run }
> +! { dg-require-effective-target pthread }
> +! { dg-additional-options "-O2 -ftree-parallelize-loops=2 -fdump-tree-parloops2-details" }
> +!
> +! PR tree-optimization/83064
> +!
> +! Run two loops, one parallelized via do-concurrent and one serial. Compare
> +! their results to make sure that the loop that is parallelized computes the
> +! correct and expected result.
> +
> +program main
> +    use, intrinsic :: iso_fortran_env
> +    implicit none
> +
> +    integer, parameter :: nsplit = 4
> +    integer(int64), parameter :: ne = 100000
> +    integer(int64) :: stride, low(nsplit), high(nsplit), edof(ne), i
> +    real(real64), dimension(nsplit) :: pi
> +    real(real64) :: par, ser
> +
> +    edof(1::4) = 1
> +    edof(2::4) = 2
> +    edof(3::4) = 3
> +    edof(4::4) = 4
> +
> +    stride = ceiling(real(ne)/nsplit)
> +    do i = 1, nsplit
> +        high(i) = stride*i
> +    end do
> +    do i = 2, nsplit
> +        low(i) = high(i-1) + 1
> +    end do
> +    low(1) = 1
> +    high(nsplit) = ne
> +
> +    ! Parallel loop.
> +    pi = 0
> +    do concurrent (i = 1:nsplit)
> +        pi(i) = sum(compute( low(i), high(i) ))
> +    end do
> +    par = 4*sum(pi)
> +
> +    ! Serial loop as a reference.
> +    pi = 0
> +    do i = 1, nsplit
> +        pi(i) = sum(compute( low(i), high(i) ))
> +    end do
> +    ser = 4*sum(pi)
> +
> +    if (abs(par - ser) > 1.0e-9_real64 * abs(ser)) stop 1
> +
> +contains
> +
> +    pure function compute( low, high ) result( tmp )
> +        integer(int64), intent(in) :: low, high
> +        real(real64), dimension(nsplit) :: tmp
> +        integer(int64) :: j, k
> +
> +        tmp = 0
> +        do j = low, high
> +            k = edof(j)
> +            tmp(k) = tmp(k) + (-1.0_real64)**(j+1) / real( 2*j-1 )
> +        end do
> +    end function
> +
> +end program main
> +
> +! { dg-final { scan-tree-dump "parallelizing \[^\n\r\]*loop" "parloops2" } }
> diff --git a/gcc/testsuite/gfortran.dg/do_concurrent_6.f90 b/gcc/testsuite/gfortran.dg/do_concurrent_6.f90
> index 9585a9f96f4..5320fd108a1 100644
> --- a/gcc/testsuite/gfortran.dg/do_concurrent_6.f90
> +++ b/gcc/testsuite/gfortran.dg/do_concurrent_6.f90
> @@ -10,4 +10,4 @@ program main
>    print *,sum(a)
>  end program main
>
> -! { dg-final { scan-tree-dump-times "ivdep" 1 "original" } }
> +! { dg-final { scan-tree-dump-times "parallel" 1 "original" } }
> diff --git a/gcc/testsuite/gfortran.dg/do_concurrent_7.f90 b/gcc/testsuite/gfortran.dg/do_concurrent_7.f90
> index 604f6712d05..c4c73094960 100644
> --- a/gcc/testsuite/gfortran.dg/do_concurrent_7.f90
> +++ b/gcc/testsuite/gfortran.dg/do_concurrent_7.f90
> @@ -21,6 +21,6 @@ program dc
>    end do
>  end program
>
> -! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* ivdep>, vector" "original" } }
> -! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* ivdep>, no-vector" "original" } }
> -! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* ivdep>, unroll 4>, no-vector" "original" } }
> +! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* parallel>, vector" "original" } }
> +! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* parallel>, no-vector" "original" } }
> +! { dg-final { scan-tree-dump "ANNOTATE_EXPR .* parallel>, unroll 4>, no-vector" "original" } }
> diff --git a/gcc/tree-parloops.cc b/gcc/tree-parloops.cc
> index 1da23f8d3c8..c16ebc7ba76 100644
> --- a/gcc/tree-parloops.cc
> +++ b/gcc/tree-parloops.cc
> @@ -1151,6 +1151,7 @@ struct elv_data
>    struct walk_stmt_info info;
>    edge entry;
>    int_tree_htab_type *decl_address;
> +  hash_set<tree> *escaping;
>    gimple_stmt_iterator *gsi;
>    bool changed;
>    bool reset;
> @@ -1175,6 +1176,9 @@ eliminate_local_variables_1 (tree *tp, int *walk_subtrees, void *data)
>        if (!SSA_VAR_P (t) || DECL_EXTERNAL (t))
>         return NULL_TREE;
>
> +      if (dta->escaping && !is_global_var (t) && !dta->escaping->contains (t))
> +       return NULL_TREE;
> +
>        type = TREE_TYPE (t);
>        addr_type = build_pointer_type (type);
>        addr = take_address_of (t, addr_type, dta->entry, dta->decl_address,
> @@ -1213,6 +1217,10 @@ eliminate_local_variables_1 (tree *tp, int *walk_subtrees, void *data)
>        if (!var || !SSA_VAR_P (var) || DECL_EXTERNAL (var))
>         return NULL_TREE;
>
> +      if (dta->escaping && !is_global_var (var)
> +         && !dta->escaping->contains (var))
> +       return NULL_TREE;
> +
>        addr_type = TREE_TYPE (t);
>        addr = take_address_of (obj, addr_type, dta->entry, dta->decl_address,
>                               dta->gsi);
> @@ -1240,7 +1248,8 @@ eliminate_local_variables_1 (tree *tp, int *walk_subtrees, void *data)
>
>  static void
>  eliminate_local_variables_stmt (edge entry, gimple_stmt_iterator *gsi,
> -                               int_tree_htab_type *decl_address)
> +                               int_tree_htab_type *decl_address,
> +                               hash_set<tree> *escaping)
>  {
>    struct elv_data dta;
>    gimple *stmt = gsi_stmt (*gsi);
> @@ -1248,6 +1257,7 @@ eliminate_local_variables_stmt (edge entry, gimple_stmt_iterator *gsi,
>    memset (&dta.info, '\0', sizeof (dta.info));
>    dta.entry = entry;
>    dta.decl_address = decl_address;
> +  dta.escaping = escaping;
>    dta.changed = false;
>    dta.reset = false;
>
> @@ -1279,6 +1289,53 @@ eliminate_local_variables_stmt (edge entry, gimple_stmt_iterator *gsi,
>      update_stmt (stmt);
>  }
>
> +/* Callback for walk_stmt_load_store_addr_ops to record into DATA any memory
> +   decls referenced by the gimple stmt.  */
> +
> +static bool
> +record_memory_decl (gimple *, tree base, tree, void *data)
> +{
> +  hash_set<tree> *decls = (hash_set<tree> *) data;
> +  base = get_base_address (base);
> +  if (DECL_P (base) && SSA_VAR_P (base) && !is_global_var (base)
> +      && !DECL_EXTERNAL (base))

This looks like you want

    if (VAR_P (base) && auto_var_in_fn_p (base, current_function_decl))

> +    decls->add (base);
> +  return false;
> +}
> +
> +/* Collect into ESCAPING the memory decls referenced outside LOOP.  Must run
> +   before LOOP is duplicated as a duplicated body would otherwise make a private
> +   decl look like it escapes.  */
> +
> +static void
> +gather_escaping_decls (class loop *loop, hash_set<tree> *escaping)
> +{
> +  basic_block *bbs = get_loop_body (loop);
> +  unsigned i;
> +
> +  auto_bitmap in_loop;
> +  for (i = 0; i < loop->num_nodes; i++)
> +    bitmap_set_bit (in_loop, bbs[i]->index);
> +  free (bbs);
> +
> +  basic_block bb;
> +  FOR_EACH_BB_FN (bb, cfun)
> +    if (!bitmap_bit_p (in_loop, bb->index))
> +      {
> +       /* PHI arguments may contain decl addresses.  */
> +       for (gphi_iterator gpi = gsi_start_phis (bb); !gsi_end_p (gpi);
> +            gsi_next (&gpi))
> +         walk_stmt_load_store_addr_ops (gpi.phi (), escaping, NULL, NULL,
> +                                        record_memory_decl);
> +
> +       for (gimple_stmt_iterator gsi = gsi_start_nondebug_bb (bb);
> +            !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
> +         walk_stmt_load_store_addr_ops (gsi_stmt (gsi), escaping,
> +                                        record_memory_decl, record_memory_decl,
> +                                        record_memory_decl);

I'll note this is quadratic in the number of parallelized loops and
function size.

To me this also looks like optimization, not a safety check that's
still missing.
What happens if we hoisted/sunk the decl or if we peeled the loop?  You then
get sharing and thus still miscompilation, no?

> +      }
> +}
> +
>  /* Eliminates the references to local variables from the single entry
>     single exit region between the ENTRY and EXIT edges.
>
> @@ -1288,10 +1345,15 @@ eliminate_local_variables_stmt (edge entry, gimple_stmt_iterator *gsi,
>     necessary).
>
>     2) Dereferencing a local variable -- these are replaced with indirect
> -   references.  */
> +   references.
> +
> +   A memory decl not in ESCAPING is iteration-private and left untouched, so the
> +   outliner gives each thread its own copy.  This is a conservative approach to
> +   avoid hoisting memory that would otherwise end up being overwritten and cause
> +   data races when written by multiple threads.  */
>
>  static void
> -eliminate_local_variables (edge entry, edge exit)
> +eliminate_local_variables (edge entry, edge exit, hash_set<tree> *escaping)
>  {
>    basic_block bb;
>    auto_vec<basic_block, 3> body;
> @@ -1314,7 +1376,8 @@ eliminate_local_variables (edge entry, edge exit)
>                 has_debug_stmt = true;
>             }
>           else
> -           eliminate_local_variables_stmt (entry, &gsi, &decl_address);
> +           eliminate_local_variables_stmt (entry, &gsi, &decl_address,
> +                                           escaping);
>        }
>
>    if (has_debug_stmt)
> @@ -1322,7 +1385,8 @@ eliminate_local_variables (edge entry, edge exit)
>        if (bb != entry_bb && bb != exit_bb)
>         for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>           if (gimple_debug_bind_p (gsi_stmt (gsi)))
> -           eliminate_local_variables_stmt (entry, &gsi, &decl_address);
> +           eliminate_local_variables_stmt (entry, &gsi, &decl_address,
> +                                           escaping);
>  }
>
>  /* Returns true if expression EXPR is not defined between ENTRY and
> @@ -2822,6 +2886,11 @@ gen_parallel_loop (class loop *loop,
>    location_t loc;
>    gimple *cond_stmt;
>
> +  /* Compute this before the loop is duplicated below.  */
> +  hash_set<tree> escaping;
> +  if (loop->can_be_parallel)
> +    gather_escaping_decls (loop, &escaping);
> +
>    /* From
>
>       ---------------------------------------------------------------------
> @@ -2994,7 +3063,8 @@ gen_parallel_loop (class loop *loop,
>       been done for oacc_kernels_p in pass_lower_omp/lower_omp ().  */
>    if (!oacc_kernels_p)
>      {
> -      eliminate_local_variables (entry, exit);
> +      eliminate_local_variables (entry, exit,
> +                                loop->can_be_parallel ? &escaping : NULL);
>        /* In the old loop, move all variables non-local to the loop to a
>          structure and back, and create separate decls for the variables used in
>          loop.  */
> --
> 2.43.0
>