[PATCH] [og12] amdgcn: Use FLAT addressing for all functions with pointer arguments
The GCN backend uses a heuristic to determine whether to use FLAT or GLOBAL addressing in a particular (offload) function: namely, if a function takes a pointer-to-scalar parameter, it is assumed that the pointer may refer to "flat scratch" space, and thus FLAT addressing must be used instead of GLOBAL. I came up with this heuristic initially whilst working on support for moving OpenACC gang-private variables into local-data share (scratch) memory. The assumption that only scalar variables would be transformed in that way turned out to be wrong. For example, prior to the next patch in the series, Fortran compiler-generated temporary structures were treated as gang private and moved to LDS space, typically overflowing the region allocated for such variables. That will no longer happen after that patch is applied, but there may be other cases of structs moving to LDS space now or in the future that this patch may be needed for. Tested with offloading to AMD GCN. I will apply shortly (to og12). 2022-10-14 Julian Brown gcc/ * config/gcn/gcn.cc (gcn_detect_incoming_pointer_arg): Any pointer argument forces FLAT addressing mode, not just pointer-to-non-aggregate. --- gcc/ChangeLog.omp | 6 ++ gcc/config/gcn/gcn.cc | 15 +-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/gcc/ChangeLog.omp b/gcc/ChangeLog.omp index d296eb137e8..ceed4da9799 100644 --- a/gcc/ChangeLog.omp +++ b/gcc/ChangeLog.omp @@ -1,3 +1,9 @@ +2022-10-14 Julian Brown + + * config/gcn/gcn.cc (gcn_detect_incoming_pointer_arg): Any pointer + argument forces FLAT addressing mode, not just + pointer-to-non-aggregate. + 2022-10-12 Andrew Stubbs * config/gcn/gcn.cc (gcn_expand_builtin_1): Change gcn_full_exec_reg diff --git a/gcc/config/gcn/gcn.cc b/gcc/config/gcn/gcn.cc index 1f8d8e19971..b01131c0dc2 100644 --- a/gcc/config/gcn/gcn.cc +++ b/gcc/config/gcn/gcn.cc @@ -2819,10 +2819,14 @@ gcn_arg_partial_bytes (cumulative_args_t cum_v, const function_arg_info &arg) return (NUM_PARM_REGS - cum_num) * regsize; } -/* A normal function which takes a pointer argument (to a scalar) may be - passed a pointer to LDS space (via a high-bits-set aperture), and that only - works with FLAT addressing, not GLOBAL. Force FLAT addressing if the - function has an incoming pointer-to-scalar parameter. */ +/* A normal function which takes a pointer argument may be passed a pointer to + LDS space (via a high-bits-set aperture), and that only works with FLAT + addressing, not GLOBAL. Force FLAT addressing if the function has an + incoming pointer parameter. NOTE: This is a heuristic that works in the + offloading case, but in general, a function might read global pointer + variables, etc. that may refer to LDS space or other special memory areas + not supported by GLOBAL instructions, and then this argument check would not + suffice. */ static void gcn_detect_incoming_pointer_arg (tree fndecl) @@ -2832,8 +2836,7 @@ gcn_detect_incoming_pointer_arg (tree fndecl) for (tree arg = TYPE_ARG_TYPES (TREE_TYPE (fndecl)); arg; arg = TREE_CHAIN (arg)) -if (POINTER_TYPE_P (TREE_VALUE (arg)) - && !AGGREGATE_TYPE_P (TREE_TYPE (TREE_VALUE (arg +if (POINTER_TYPE_P (TREE_VALUE (arg))) cfun->machine->use_flat_addressing = true; } -- 2.29.2
[PATCH] [og12] OpenACC: Don't gang-privatize artificial variables
This patch prevents compiler-generated artificial variables from being
treated as privatization candidates for OpenACC.
The rationale is that e.g. "gang-private" variables actually must be
shared by each worker and vector spawned within a particular gang, but
that sharing is not necessary for any compiler-generated variable (at
least at present, but no such need is anticipated either). Variables on
the stack (and machine registers) are already private per-"thread"
(gang, worker and/or vector), and that's fine for artificial variables.
Several tests need their scan output patterns adjusted to compensate.
Tested with offloading to AMD GCN. I will apply shortly (to og12).
2022-10-14 Julian Brown
gcc/
* omp-low.cc (oacc_privatization_candidate_p): Artificial vars are not
privatization candidates.
libgomp/
* testsuite/libgomp.oacc-fortran/declare-1.f90: Adjust scan output.
* testsuite/libgomp.oacc-fortran/host_data-5.F90: Likewise.
* testsuite/libgomp.oacc-fortran/if-1.f90: Likewise.
* testsuite/libgomp.oacc-fortran/print-1.f90: Likewise.
* testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Likewise.
---
gcc/ChangeLog.omp | 5
gcc/omp-low.cc| 22 +++
libgomp/ChangeLog.omp | 8 ++
.../libgomp.oacc-fortran/declare-1.f90| 12 +++-
.../libgomp.oacc-fortran/host_data-5.F90 | 28 +++
.../testsuite/libgomp.oacc-fortran/if-1.f90 | 12
.../libgomp.oacc-fortran/print-1.f90 | 13 +
.../libgomp.oacc-fortran/privatized-ref-2.f90 | 12 ++--
8 files changed, 72 insertions(+), 40 deletions(-)
diff --git a/gcc/ChangeLog.omp b/gcc/ChangeLog.omp
index ceed4da9799..c34d0ec7c77 100644
--- a/gcc/ChangeLog.omp
+++ b/gcc/ChangeLog.omp
@@ -1,3 +1,8 @@
+2022-10-14 Julian Brown
+
+ * omp-low.cc (oacc_privatization_candidate_p): Artificial vars are not
+ privatization candidates.
+
2022-10-14 Julian Brown
* config/gcn/gcn.cc (gcn_detect_incoming_pointer_arg): Any pointer
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index d726eea2480..f171181e2c4 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -11400,6 +11400,28 @@ oacc_privatization_candidate_p (const location_t loc,
const tree c,
}
}
+ /* If an artificial variable has been added to a bind, e.g.
+ a compiler-generated temporary structure used by the Fortran front-end, do
+ not consider it as a privatization candidate. Note that variables on
+ the stack are private per-thread by default: making them "gang-private"
+ for OpenACC actually means to share a single instance of a variable
+ amongst all workers and threads spawned within each gang.
+ At present, no compiler-generated artificial variables require such
+ sharing semantics, so this is safe. */
+
+ if (res && DECL_ARTIFICIAL (decl))
+{
+ res = false;
+
+ if (dump_enabled_p ())
+ {
+ oacc_privatization_begin_diagnose_var (l_dump_flags, loc, c, decl);
+ dump_printf (l_dump_flags,
+ "isn%'t candidate for adjusting OpenACC privatization "
+ "level: %s\n", "artificial");
+ }
+}
+
if (res)
{
if (dump_enabled_p ())
diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 7353fff2554..cb3541be378 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,11 @@
+2022-10-14 Julian Brown
+
+ * testsuite/libgomp.oacc-fortran/declare-1.f90: Adjust scan output.
+ * testsuite/libgomp.oacc-fortran/host_data-5.F90: Likewise.
+ * testsuite/libgomp.oacc-fortran/if-1.f90: Likewise.
+ * testsuite/libgomp.oacc-fortran/print-1.f90: Likewise.
+ * testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Likewise.
+
2022-10-05 Tobias Burnus
Backport from mainline:
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
index 51776a1d260..959e8941d5b 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90
@@ -25,6 +25,9 @@ module vars
end module vars
subroutine subr5 (a, b, c, d)
+ ! { dg-note {variable 'a\.[0-9]+' declared in block isn't candidate for
adjusting OpenACC privatization level: not addressable} "" { target *-*-* } .-1
}
+ ! { dg-note {variable 'c\.[0-9]+' declared in block isn't candidate for
adjusting OpenACC privatization level: not addressable} "" { target *-*-* } .-2
}
+ ! { dg-note {variable 'd\.[0-9]+' declared in block isn't candidate for
adjusting OpenACC privatization level: not addressable} "" { target *-*-* } .-3
}
implicit none
integer, parameter :: N = 8
integer :: i
@@ -51,6 +54,8 @@ subroutine subr5 (a, b, c, d)
end subroutine
subroutine subr4 (a, b)
+ ! { dg-note {variable 'a\.[0-9]+' decla
[committed] gfortran.dg/c-interop/deferred-character-2.f90: Fix dg-do
Just spotted this. It did only compile instead of also run and was the
only occurrence I could find for 'dg-.*execute'.
Committed as https://gcc.gnu.org/r13-3306
Tobias
-
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
commit 3760dd553eed21ac5614cf0d0841ca984b4361e2
Author: Tobias Burnus
Date: Fri Oct 14 18:34:49 2022 +0200
gfortran.dg/c-interop/deferred-character-2.f90: Fix dg-do
gcc/testsuite/
* gfortran.dg/c-interop/deferred-character-2.f90: Use 'dg-do run'.
diff --git a/gcc/testsuite/gfortran.dg/c-interop/deferred-character-2.f90 b/gcc/testsuite/gfortran.dg/c-interop/deferred-character-2.f90
index 356097af241..4dab32662c6 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/deferred-character-2.f90
+++ b/gcc/testsuite/gfortran.dg/c-interop/deferred-character-2.f90
@@ -1,5 +1,5 @@
! PR 92482
-! { dg-do execute}
+! { dg-do run }
!
! TS 29113
! 8.7 Interoperability of procedures and procedure interfaces
Re: [PATCH] Fortran: fix check of polymorphic elements in data transfers [PR100971]
Le 09/10/2022 à 20:57, Harald Anlauf via Fortran a écrit : Dear all, the check of data transfer elements needs to verify that for polymorphic objects there is a user defined DTIO procedure. This check worked fine for scalars, but skipped arrays, leading to an ICE later. The obvious fix is to allow this check to inspect arrays. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes, thanks.
[Patch] Fortran: Fixes for kind=4 characters strings [PR107266]
Long introduction - but the patch is rather simple: Don't use kind=1
as type where kind=4 should be used.
Long introduction + background, feel free to skip.
This popped up for libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90
which uses kind=4 characters – if Sandra's "Fortran: delinearize
multi-dimensional
array accesses" patch is applied.
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562230.html
Used for OG11:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584716.html
On the OG12 alias devel/omp/gcc-12 vendor branch, it is used:
https://gcc.gnu.org/g:39a8c371fda6136cf77c74895a00b136409e0ba3
* * *
For mainline, I did not observe a wrong-code issue at runtime, still:
void frobc (character(kind=4)[1:*_a] * & restrict a, ...
...
static void frobc (character(kind=1) * & restrict, ...
feels odd, i.e. having the definition as kind=4 and the declaration as kind=1.
With the patch, it becomes:
static void frobc (character(kind=4) * & restrict, character(kind=4) * &, ...
* * *
For the following, questionable code (→ PR107266), it is even worse:
character(kind=4) function f(x) bind(C)
character(kind=4), value :: x
end
this gives the following, which has the wrong ABI:
character(kind=1) f (character(kind=1) x)
{
(void) 0;
}
With the patch, it becomes:
character(kind=4) f (character(kind=4) x)
* * *
I think that all only exercises the trans-type.cc patch;
the trans-expr.cc code gets called – as an assert shows,
but I fail to get a dump where this goes wrong.
However, for struct-elem-map-1.f90 with mainline or with
OG12 and the patch:
#pragma omp target map(tofrom:var.uni2[40 / 20] [len: 20])
while on OG12 without the attached patch:
#pragma omp target map(tofrom:var.uni2[40 / 5] [len: 5])
where the problem is that TYPE_SIZE_UNIT is wrong. Whether
this only affects OG12 due to the delinearizer patch or
some code on mainline as well, I don't know.
Still, I think it should be fixed ...
OK for mainline?
Tobias
-
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
Fortran: Fixes for kind=4 characters strings [PR107266]
PR fortran/107266
gcc/fortran/
* trans-expr.cc (gfc_conv_string_parameter): Use passed
type to honor character kind.
* trans-types.cc (gfc_sym_type): Honor character kind.
* trans-decl.cc (gfc_conv_cfi_to_gfc): Fix handling kind=4
character strings.
gcc/testsuite/
* gfortran.dg/char4_decl.f90: New test.
* gfortran.dg/char4_decl-2.f90: New test.
gcc/fortran/trans-decl.cc | 10 ++---
gcc/fortran/trans-expr.cc | 12 +++---
gcc/fortran/trans-types.cc | 2 +-
gcc/testsuite/gfortran.dg/char4_decl-2.f90 | 59 ++
gcc/testsuite/gfortran.dg/char4_decl.f90 | 52 ++
5 files changed, 123 insertions(+), 12 deletions(-)
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 5d16d640322..4b570c3551a 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -7378,13 +7378,13 @@ done:
/* Set string length for len=:, only. */
if (sym->ts.type == BT_CHARACTER && !sym->ts.u.cl->length)
{
- tmp = sym->ts.u.cl->backend_decl;
+ tmp2 = gfc_get_cfi_desc_elem_len (cfi);
+ tmp = fold_convert (TREE_TYPE (tmp2), sym->ts.u.cl->backend_decl);
if (sym->ts.kind != 1)
tmp = fold_build2_loc (input_location, MULT_EXPR,
- gfc_array_index_type,
- sym->ts.u.cl->backend_decl, tmp);
- tmp2 = gfc_get_cfi_desc_elem_len (cfi);
- gfc_add_modify (&block, tmp2, fold_convert (TREE_TYPE (tmp2), tmp));
+ TREE_TYPE (tmp2), tmp,
+ build_int_cst (TREE_TYPE (tmp2), sym->ts.kind));
+ gfc_add_modify (&block, tmp2, tmp);
}
if (!sym->attr.dimension)
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 1551a2e4df4..e7b9211f17e 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -10374,15 +10374,15 @@ gfc_conv_string_parameter (gfc_se * se)
|| TREE_CODE (TREE_TYPE (se->expr)) == INTEGER_TYPE)
&& TYPE_STRING_FLAG (TREE_TYPE (se->expr)))
{
+ type = TREE_TYPE (se->expr);
if (TREE_CODE (se->expr) != INDIRECT_REF)
- {
- type = TREE_TYPE (se->expr);
- se->expr = gfc_build_addr_expr (build_pointer_type (type), se->expr);
- }
+ se->expr = gfc_build_addr_expr (build_pointer_type (type), se->expr);
else
{
- type = gfc_get_character_type_len (gfc_default_character_kind,
- se->string_length);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ type = TREE_TYPE (type);
+ type = gfc_get_character_type_len_for_eltype (type,
+ se->string_length);
type = build_pointer_type (type);
se->expr =
