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

[Thomas Schwinge <thomas@codesourcery.com>]

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

Hi!

On 2021-08-06T10:25:22+0100, Julian Brown <julian@codesourcery.com> wrote:
> On Wed, 4 Aug 2021 15:56:49 +0200
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>> a nontrivial amount of data structures/logic/code did get
>> duplicated from the nvptx back end, and modified slightly or
>> not-so-slightly (RTL vs. GIMPLE plus certain implementation
>> "details").
>>
>> We should at least cross reference the two instances, to make sure
>> that any changes to one are also propagated to the other.  (I'll take
>> care.)
>
> OK, thanks,

Pushed "Cross-reference parts adapted in
'gcc/omp-oacc-neuter-broadcast.cc'" to master branch in
commit 62f01243fb27030b8d99c671f27349c2e7465edc, see attached.

>> And then, do you (or anyone else, of course) happen to have any clever
>> idea about how to avoid the duplication, and somehow combine the RTL
>> vs. GIMPLE implementations?  Given that we nowadays may use C++ -- do
>> you foresee it feasible to have an abstract base class capturing
>> basically the data structures, logic, common code, and then
>> RTL-specialized plus GIMPLE-specialized classes inheriting from that?
>
> I suppose one could either use "old-style" inheritance, or maybe do
> it with templates.

Or, as my WIP would show: both of these.  ;-) (To be posted later.)

> There's probably both costs & benefits when it comes
> to maintenance, either way -- having this code shared would mean any
> changes need testing for both nvptx & GCN targets, and risks making it
> harder to follow. OTOH, like you say, changes would only need to be
> made in one place.


> TBH, I'd spend effort on trying to integrate the SESE code (if it'd be
> beneficial) first, before trying to de-duplicate those other bits.

Spending effort on that may make sense, but I'm not able to do that as
part of this task here, because that's new development and related
performance etc. analysis -- which additionally I don't know much about
in the GCN context.


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-Cross-reference-parts-adapted-in-gcc-omp-oacc-neuter.patch --]
[-- Type: text/x-diff, Size: 5133 bytes --]

From 62f01243fb27030b8d99c671f27349c2e7465edc Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 9 Aug 2021 12:21:43 +0200
Subject: [PATCH] Cross-reference parts adapted in
 'gcc/omp-oacc-neuter-broadcast.cc'

	gcc/
	* config/nvptx/nvptx.c: Cross-reference parts adapted in
	'gcc/omp-oacc-neuter-broadcast.cc'.
	* omp-low.c: Likewise.
	* omp-oacc-neuter-broadcast.cc: Cross-reference parts adapted from
	the above files.
---
 gcc/config/nvptx/nvptx.c         | 5 +++++
 gcc/omp-low.c                    | 2 ++
 gcc/omp-oacc-neuter-broadcast.cc | 9 ++++++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 6642bdfa867..4e4909e8c5f 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -3205,6 +3205,7 @@ nvptx_mach_vector_length ()
 
 /* Loop structure of the function.  The entire function is described as
    a NULL loop.  */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:struct parallel_g'.  */
 
 struct parallel
 {
@@ -3282,6 +3283,7 @@ typedef auto_vec<insn_bb_t> insn_bb_vec_t;
    partitioning mode of the function as a whole.  Populate MAP with
    head and tail blocks.  We also clear the BB visited flag, which is
    used when finding partitions.  */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:omp_sese_split_blocks'.  */
 
 static void
 nvptx_split_blocks (bb_insn_map_t *map)
@@ -3383,6 +3385,7 @@ nvptx_discover_pre (basic_block block, int expected)
 }
 
 /* Dump this parallel and all its inner parallels.  */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:omp_sese_dump_pars'.  */
 
 static void
 nvptx_dump_pars (parallel *par, unsigned depth)
@@ -3408,6 +3411,7 @@ nvptx_dump_pars (parallel *par, unsigned depth)
 /* If BLOCK contains a fork/join marker, process it to create or
    terminate a loop structure.  Add this block to the current loop,
    and then walk successor blocks.   */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:omp_sese_find_par'.  */
 
 static parallel *
 nvptx_find_par (bb_insn_map_t *map, parallel *par, basic_block block)
@@ -3488,6 +3492,7 @@ nvptx_find_par (bb_insn_map_t *map, parallel *par, basic_block block)
    to head & tail markers, discovered when splitting blocks.  This
    speeds up the discovery.  We rely on the BB visited flag having
    been cleared when splitting blocks.  */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:omp_sese_discover_pars'.  */
 
 static parallel *
 nvptx_discover_pars (bb_insn_map_t *map)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 2f735bcde9c..926087da701 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -615,6 +615,8 @@ omp_copy_decl_1 (tree var, omp_context *ctx)
 
 /* Build COMPONENT_REF and set TREE_THIS_VOLATILE and TREE_READONLY on it
    as appropriate.  */
+/* See also 'gcc/omp-oacc-neuter-broadcast.cc:oacc_build_component_ref'.  */
+
 static tree
 omp_build_component_ref (tree obj, tree field)
 {
diff --git a/gcc/omp-oacc-neuter-broadcast.cc b/gcc/omp-oacc-neuter-broadcast.cc
index 0f6ba885c6c..f8555380451 100644
--- a/gcc/omp-oacc-neuter-broadcast.cc
+++ b/gcc/omp-oacc-neuter-broadcast.cc
@@ -56,6 +56,7 @@
 
 /* Loop structure of the function.  The entire function is described as
    a NULL loop.  */
+/* Adapted from 'gcc/config/nvptx/nvptx.c:struct parallel'.  */
 
 struct parallel_g
 {
@@ -183,6 +184,7 @@ omp_sese_active_worker_call (gcall *call)
    partitioning mode of the function as a whole.  Populate MAP with
    head and tail blocks.  We also clear the BB visited flag, which is
    used when finding partitions.  */
+/* Adapted from 'gcc/config/nvptx/nvptx.c:nvptx_split_blocks'.  */
 
 static void
 omp_sese_split_blocks (bb_stmt_map_t *map)
@@ -341,6 +343,7 @@ mask_name (unsigned mask)
 }
 
 /* Dump this parallel and all its inner parallels.  */
+/* Adapted from 'gcc/config/nvptx/nvptx.c:nvptx_dump_pars'.  */
 
 static void
 omp_sese_dump_pars (parallel_g *par, unsigned depth)
@@ -366,6 +369,7 @@ omp_sese_dump_pars (parallel_g *par, unsigned depth)
 /* If BLOCK contains a fork/join marker, process it to create or
    terminate a loop structure.  Add this block to the current loop,
    and then walk successor blocks.   */
+/* Adapted from 'gcc/config/nvptx/nvptx.c:nvptx_find_par'.  */
 
 static parallel_g *
 omp_sese_find_par (bb_stmt_map_t *map, parallel_g *par, basic_block block)
@@ -471,6 +475,7 @@ walk_successors:
    to head & tail markers, discovered when splitting blocks.  This
    speeds up the discovery.  We rely on the BB visited flag having
    been cleared when splitting blocks.  */
+/* Adapted from 'gcc/config/nvptx/nvptx.c:nvptx_discover_pars'.  */
 
 static parallel_g *
 omp_sese_discover_pars (bb_stmt_map_t *map)
@@ -931,7 +936,9 @@ worker_single_simple (basic_block from, basic_block to,
   update_stmt (acc_bar);
 }
 
-/* This is a copied and renamed omp-low.c:omp_build_component_ref.  */
+/* Build COMPONENT_REF and set TREE_THIS_VOLATILE and TREE_READONLY on it
+   as appropriate.  */
+/* Adapted from 'gcc/omp-low.c:omp_build_component_ref'.  */
 
 static tree
 oacc_build_component_ref (tree obj, tree field)
-- 
2.30.2