Re: [PATCH 1/3] openacc: Add support for gang local storage allocation in shared memory

[Thomas Schwinge]

Hi!

On 2021-04-19T12:23:56+0100, Julian Brown <jul...@codesourcery.com> wrote:
> On Thu, 15 Apr 2021 19:26:54 +0200
> Thomas Schwinge <tho...@codesourcery.com> wrote:
>> As that may not be obvious to the reader, I'd like to have the
>> 'TREE_ADDRESSABLE' conditionalization be documented in the code.  You
>> had explained that in
>> <http://mid.mail-archive.com/20190612204216.0ec83e4e@squid.athome>: "a
>> non-addressable variable [...]".
>
> Yeah that probably makes sense.

I've pushed "[OpenACC privatization] Explain OpenACC privatization
candidate selection [PR90115]" to master branch in commit
5a0fe1f6c4ad0e50bf4684e723ae2ba17d94c9e4, see attached.


Grüße
 Thomas


-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf

>From 5a0fe1f6c4ad0e50bf4684e723ae2ba17d94c9e4 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Thu, 20 May 2021 15:55:18 +0200
Subject: [PATCH] [OpenACC privatization] Explain OpenACC privatization
 candidate selection [PR90115]

	gcc/
	PR middle-end/90115
	* omp-low.c (oacc_privatization_candidate_p): New function.
	(oacc_privatization_scan_clause_chain)
	(oacc_privatization_scan_decl_chain): Use it.  Also
	'gcc_checking_assert' that we're not seeing duplicates.
---
 gcc/omp-low.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index a86c6c1e82c..577676b2a16 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -10144,6 +10144,36 @@ lower_omp_for_lastprivate (struct omp_for_data *fd, gimple_seq *body_p,
     }
 }
 
+/* OpenACC privatization.
+
+   Or, in other words, *sharing* at the respective OpenACC level of
+   parallelism.
+
+   From a correctness perspective, a non-addressable variable can't be accessed
+   outside the current thread, so it can go in a (faster than shared memory)
+   register -- though that register may need to be broadcast in some
+   circumstances.  A variable can only meaningfully be "shared" across workers
+   or vector lanes if its address is taken, e.g. by a call to an atomic
+   builtin.
+
+   From an optimisation perspective, the answer might be fuzzier: maybe
+   sometimes, using shared memory directly would be faster than
+   broadcasting.  */
+
+static bool
+oacc_privatization_candidate_p (const tree decl)
+{
+  bool res = true;
+
+  if (res && !VAR_P (decl))
+    res = false;
+
+  if (res && !TREE_ADDRESSABLE (decl))
+    res = false;
+
+  return res;
+}
+
 /* Scan CLAUSES for candidates for adjusting OpenACC privatization level in
    CTX.  */
 
@@ -10154,8 +10184,12 @@ oacc_privatization_scan_clause_chain (omp_context *ctx, tree clauses)
     if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_PRIVATE)
       {
 	tree decl = OMP_CLAUSE_DECL (c);
-	if (VAR_P (decl) && TREE_ADDRESSABLE (decl))
-	  ctx->oacc_privatization_candidates.safe_push (decl);
+
+	if (!oacc_privatization_candidate_p (decl))
+	  continue;
+
+	gcc_checking_assert (!ctx->oacc_privatization_candidates.contains (decl));
+	ctx->oacc_privatization_candidates.safe_push (decl);
       }
 }
 
@@ -10166,8 +10200,13 @@ static void
 oacc_privatization_scan_decl_chain (omp_context *ctx, tree decls)
 {
   for (tree decl = decls; decl; decl = DECL_CHAIN (decl))
-    if (VAR_P (decl) && TREE_ADDRESSABLE (decl))
+    {
+      if (!oacc_privatization_candidate_p (decl))
+	continue;
+
+      gcc_checking_assert (!ctx->oacc_privatization_candidates.contains (decl));
       ctx->oacc_privatization_candidates.safe_push (decl);
+    }
 }
 
 /* Callback for walk_gimple_seq.  Find #pragma omp scan statement.  */
-- 
2.30.2