Re: [PATCH 1/4] openacc: Middle-end worker-partitioning support

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

Hi!

On 2021-03-02T04:20:11-0800, Julian Brown <julian@codesourcery.com> wrote:
> --- /dev/null
> +++ b/gcc/oacc-neuter-bcast.c

Allocated here:

> +/* Sets of SSA_NAMES or VAR_DECLs to propagate.  */
> +typedef hash_set<tree> propagation_set;
> +
> +static void
> +find_ssa_names_to_propagate ([...],
> +                          vec<propagation_set *> *prop_set)
> +{

> +                   if (!(*prop_set)[def_bb->index])
> +                     (*prop_set)[def_bb->index] = new propagation_set;

> +                   if (!(*prop_set)[def_bb->index])
> +                     (*prop_set)[def_bb->index] = new propagation_set;

..., and here:

> +static void
> +find_local_vars_to_propagate ([...],
> +                           vec<propagation_set *> *prop_set)
> +{

> +                   if (!(*prop_set)[block->index])
> +                     (*prop_set)[block->index] = new propagation_set;

..., and deallocated here:

> +static void
> +neuter_worker_single ([...],
> +                   vec<propagation_set *> *prop_set,
> +                   [...])
> +{

> +       propagation_set *ws_prop = (*prop_set)[block->index];
> +
> +       if (ws_prop)
> +         {
> +           [...]
> +           delete ws_prop;
> +           (*prop_set)[block->index] = 0;
> +         }

..., and defined here:

> +void
> +oacc_do_neutering (void)
> +{

> +  vec<propagation_set *> prop_set;
> +  prop_set.create (last_basic_block_for_fn (cfun));
> +
> +  for (unsigned i = 0; i < last_basic_block_for_fn (cfun); i++)
> +    prop_set.quick_push (0);

I recently learned about 'safe_grow_cleared', which allows for
simplifying this loop.

> +  find_ssa_names_to_propagate ([...], &prop_set);

> +  find_local_vars_to_propagate ([...], &prop_set);

> +  neuter_worker_single ([...], &prop_set, [...]);
> +
> +  prop_set.release ();

It seems appropriate to add a check that 'neuter_worker_single' has
indeed handled/'delete'd all these.

Pushed "Clarify memory management for 'prop_set' in
'gcc/omp-oacc-neuter-broadcast.cc'" to master branch in
commit 7b9d99e615212c24cecae4202d8def9aa5e71809, see attached.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-memory-management-for-prop_set-in-gcc-omp-oa.patch --]
[-- Type: text/x-diff, Size: 1640 bytes --]

From 7b9d99e615212c24cecae4202d8def9aa5e71809 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 11 Aug 2021 22:31:55 +0200
Subject: [PATCH] Clarify memory management for 'prop_set' in
 'gcc/omp-oacc-neuter-broadcast.cc'

Clean-up for recent commit e2a58ed6dc5293602d0d168475109caa81ad0f0d
"openacc: Middle-end worker-partitioning support".

	gcc/
	* omp-oacc-neuter-broadcast.cc
	(execute_omp_oacc_neuter_broadcast): Clarify memory management for
	'prop_set'.
---
 gcc/omp-oacc-neuter-broadcast.cc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/omp-oacc-neuter-broadcast.cc b/gcc/omp-oacc-neuter-broadcast.cc
index 9bde0aca10f..d30867085c3 100644
--- a/gcc/omp-oacc-neuter-broadcast.cc
+++ b/gcc/omp-oacc-neuter-broadcast.cc
@@ -1398,11 +1398,8 @@ execute_omp_oacc_neuter_broadcast ()
   FOR_ALL_BB_FN (bb, cfun)
     bb->aux = NULL;
 
-  vec<propagation_set *> prop_set;
-  prop_set.create (last_basic_block_for_fn (cfun));
-
-  for (int i = 0; i < last_basic_block_for_fn (cfun); i++)
-    prop_set.quick_push (0);
+  vec<propagation_set *> prop_set (vNULL);
+  prop_set.safe_grow_cleared (last_basic_block_for_fn (cfun), true);
 
   find_ssa_names_to_propagate (par, mask, worker_single, vector_single,
 			       &prop_set);
@@ -1461,6 +1458,9 @@ execute_omp_oacc_neuter_broadcast ()
     delete it.second;
   record_field_map.empty ();
 
+  /* These are supposed to have been 'delete'd by 'neuter_worker_single'.  */
+  for (auto it : prop_set)
+    gcc_checking_assert (!it);
   prop_set.release ();
 
   /* This doesn't seem to make a difference.  */
-- 
2.30.2