OpenMP: 'allocate' directive - fixes for 'alignof' and [[omp::decl]]
Checks
Commit Message
In C, [[omp::decl]] wasn't supported and when discussing 'alignof' with Jakub,
he pointed out that there are corner cases (see commit log for one), which imply
that it is better that 'omp allocate align(…) doesn't affect 'alignof'.
The patch now removes the alignment handling from the FE (C and Fortran, but
only required for C); for static variables it is now handled in
varpool_node::finalize_decl (and, as before, for stack variables in
gimplify_bind_expr). [Stack variables are omp_alloc'ed, for static
variables only the alignment is honored, albeit the backends could do
something special. While they don't, I could imagine that the GPU
backends might eventually handle them in a special way.]
Comments suggestions?
Tobias
PS: C++ only has a very basic support (fails with sorry during parsing). However,
a patch to add proper support will by shortly be submitted by my colleague. It is
based on a patch that was posted end of last year, but fixes to Jakub's review
comments have/had still to be done.
The attached patch is kind of a fallout of discussions that were started during
the C++ revision work.
Comments
On Tue, Dec 03, 2024 at 10:34:34AM +0100, Tobias Burnus wrote:
> --- a/gcc/testsuite/c-c++-common/gomp/allocate-18.c
> +++ b/gcc/testsuite/c-c++-common/gomp/allocate-18.c
> @@ -17,14 +17,14 @@ typedef enum omp_allocator_handle_t
>
> void test0 ()
> {
> - int A1[5];
> + int A1[5], B1[1];
I'd use B1[5], just in case some target has the alignment dependent on
the size of the array.
> --- a/gcc/testsuite/c-c++-common/gomp/allocate-19.c
> +++ b/gcc/testsuite/c-c++-common/gomp/allocate-19.c
> @@ -19,14 +19,14 @@ typedef enum omp_allocator_handle_t
> __omp_allocator_handle_t_max__ = __UINTPTR_MAX__
> } omp_allocator_handle_t;
>
> -static int A1[5] = {1,2,3,4,5};
> +static int A1[5] = {1,2,3,4,5}, B1[1];
And here too.
Otherwise LGTM.
Jakub
Hi Tobias,
ok for the Fortran part.
- Andre
On Tue, 3 Dec 2024 10:34:34 +0100
Tobias Burnus <tburnus@baylibre.com> wrote:
> In C, [[omp::decl]] wasn't supported and when discussing 'alignof' with Jakub,
> he pointed out that there are corner cases (see commit log for one), which
> imply that it is better that 'omp allocate align(…) doesn't affect 'alignof'.
> The patch now removes the alignment handling from the FE (C and Fortran, but
> only required for C); for static variables it is now handled in
> varpool_node::finalize_decl (and, as before, for stack variables in
> gimplify_bind_expr). [Stack variables are omp_alloc'ed, for static
> variables only the alignment is honored, albeit the backends could do
> something special. While they don't, I could imagine that the GPU
> backends might eventually handle them in a special way.]
>
> Comments suggestions?
>
> Tobias
>
> PS: C++ only has a very basic support (fails with sorry during parsing).
> However, a patch to add proper support will by shortly be submitted by my
> colleague. It is based on a patch that was posted end of last year, but fixes
> to Jakub's review comments have/had still to be done.
> The attached patch is kind of a fallout of discussions that were started
> during the C++ revision work.
OpenMP: 'allocate' directive - fixes for 'alignof' and [[omp::decl]]
Fixed a check to permit [[omp::decl(allocate,...)]] parsing in C.
Additionaly, we discussed that 'allocate align' should not affect
'alignof' to avoid issues like with:
int a;
_Alignas(_Alignof(a)) int b;
#pragma omp allocate(a) align(128)
_Alignas(_Alignof(a)) int c;
Thus, the alignment is no longer set in the C and Fortran front ends,
but for static variables now in varpool_node::finalize_decl.
(For stack variables, the alignment is handled in gimplify_bind_expr.)
NOTE: 'omp allocate' is not yet supported in C++.
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_allocate): Only check scope if
not in_omp_decl_attribute. Remove setting the alignment.
gcc/ChangeLog:
* cgraphunit.cc (varpool_node::finalize_decl): Set alignment
based on OpenMP's 'omp allocate' attribute/directive.
gcc/fortran/ChangeLog:
* trans-decl.cc (gfc_finish_var_decl): Remove setting the alignment.
libgomp/ChangeLog:
* libgomp.texi (Memory allocation): Mention (non-)effect of 'align'
on _Alignof.
* testsuite/libgomp.c/allocate-7.c: New test.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-18.c: Check that alignof is unaffected
by 'omp allocate'.
* c-c++-common/gomp/allocate-19.c: Likewise.
gcc/c/c-parser.cc | 5 +--
gcc/cgraphunit.cc | 12 ++++++
gcc/fortran/trans-decl.cc | 3 --
gcc/testsuite/c-c++-common/gomp/allocate-18.c | 6 +--
gcc/testsuite/c-c++-common/gomp/allocate-19.c | 10 ++---
libgomp/libgomp.texi | 6 +++
libgomp/testsuite/libgomp.c/allocate-7.c | 54 +++++++++++++++++++++++++++
7 files changed, 81 insertions(+), 15 deletions(-)
@@ -22158,7 +22158,7 @@ c_parser_omp_allocate (c_parser *parser)
"%<allocate%> directive", var);
continue;
}
- if (!c_check_in_current_scope (var))
+ if (!parser->in_omp_decl_attribute && !c_check_in_current_scope (var))
{
error_at (OMP_CLAUSE_LOCATION (nl),
"%<allocate%> directive must be in the same scope as %qD",
@@ -22199,9 +22199,6 @@ c_parser_omp_allocate (c_parser *parser)
= {EXPR_LOC_OR_LOC (allocator, OMP_CLAUSE_LOCATION (nl)), var};
walk_tree (&allocator, c_check_omp_allocate_allocator_r, &data, NULL);
}
- if (alignment)
- SET_DECL_ALIGN (var, BITS_PER_UNIT * MAX (tree_to_uhwi (alignment),
- DECL_ALIGN_UNIT (var)));
DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"),
build_tree_list (allocator, alignment),
DECL_ATTRIBUTES (var));
@@ -983,6 +983,18 @@ varpool_node::finalize_decl (tree decl)
&& !DECL_ARTIFICIAL (node->decl)))
node->force_output = true;
+ if (flag_openmp)
+ {
+ tree attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl));
+ if (attr)
+ {
+ tree align = TREE_VALUE (TREE_VALUE (attr));
+ if (align)
+ SET_DECL_ALIGN (decl, MAX (tree_to_uhwi (align) * BITS_PER_UNIT,
+ DECL_ALIGN (decl)));
+ }
+ }
+
if (symtab->state == CONSTRUCTION
&& (node->needed_p () || node->referred_to_p ()))
enqueue_node (node);
@@ -830,9 +830,6 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym)
tree alloc = gfc_conv_constant_to_tree (n->u2.allocator);
tree align = (n->u.align ? gfc_conv_constant_to_tree (n->u.align)
: NULL_TREE);
- if (align != NULL_TREE)
- SET_DECL_ALIGN (decl, MAX (tree_to_uhwi (align),
- DECL_ALIGN_UNIT (decl)) * BITS_PER_UNIT);
DECL_ATTRIBUTES (decl)
= tree_cons (get_identifier ("omp allocate"),
build_tree_list (alloc, align), DECL_ATTRIBUTES (decl));
@@ -17,14 +17,14 @@ typedef enum omp_allocator_handle_t
void test0 ()
{
- int A1[5];
+ int A1[5], B1[1];
#pragma omp allocate(A1) align(128) allocator(omp_default_mem_alloc)
/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#ifndef __cplusplus
- _Static_assert (_Alignof(A1) == 128, "wrong alignment");
+ _Static_assert (_Alignof(A1) == _Alignof(B1), "wrong alignment");
#elif __cplusplus >= 201103L
- static_assert (alignof(A1) == 128, "wrong alignment"); /* { dg-bogus "static assertion failed: wrong alignment" "" { xfail { c++ && { ! c++98_only } } } } */
+ static_assert (alignof(A1) == alignof(B1), "wrong alignment");
#endif
}
@@ -19,14 +19,14 @@ typedef enum omp_allocator_handle_t
__omp_allocator_handle_t_max__ = __UINTPTR_MAX__
} omp_allocator_handle_t;
-static int A1[5] = {1,2,3,4,5};
+static int A1[5] = {1,2,3,4,5}, B1[1];
#pragma omp allocate(A1) align(128) allocator(omp_default_mem_alloc)
/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#ifndef __cplusplus
-_Static_assert (_Alignof(A1) == 128, "wrong alignment");
+_Static_assert (_Alignof(A1) == _Alignof(B1), "wrong alignment");
#elif __cplusplus >= 201103L
-static_assert (alignof(A1) == 128, "wrong alignment"); /* { dg-bogus "static assertion failed: wrong alignment" "" { xfail { c++ && { ! c++98_only } } } } */
+static_assert (alignof(A1) == alignof(B1), "wrong alignment");
#endif
@@ -49,9 +49,9 @@ get ()
/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#ifndef __cplusplus
- _Static_assert (_Alignof(q) == 1024, "wrong alignment");
+ _Static_assert (_Alignof(q) == _Alignof(int), "wrong alignment");
#elif __cplusplus >= 201103L
- static_assert (alignof(q) == 1024, "wrong alignment"); /* { dg-bogus "static assertion failed: wrong alignment" "" { xfail { c++ && { ! c++98_only } } } } */
+ static_assert (alignof(q) == alignof(int), "wrong alignment");
#endif
q += 1;
@@ -6731,6 +6731,12 @@ The description below applies to:
compiled with @option{-fopenmp-allocators}. Additionally, all files that
might explicitly or implicitly deallocate memory allocated that way must
also be compiled with that option.
+@item The used alignment is the maximum of the value the @code{align} clause
+ and the alignment of the type after honoring, if present, the
+ @code{aligned} (@code{GNU::aligned}) attribute and C's @code{_Alignas}
+ and C++'s @code{alignas}. However, the @code{align} clause of the
+ @code{allocate} directive has no effect on the value of C's
+ @code{_Alignof} and C++'s @code{alignof}.
@end itemize
For the available predefined allocators and, as applicable, their associated
new file mode 100644
@@ -0,0 +1,54 @@
+/* TODO: move to ../libgomp.c-c++-common once C++ is implemented. */
+/* NOTE: { target c } is unsupported with with the C compiler. */
+
+/* { dg-do run } */
+
+#include <omp.h>
+
+int AAA [[omp::decl(allocate,allocator(omp_low_lat_mem_alloc),align(4096))]];
+
+#ifndef __cplusplus
+ _Static_assert (_Alignof(AAA) == _Alignof(int), "wrong alignment");
+#elif __cplusplus >= 201103L
+ static_assert (alignof(AAA) == _Alignof(int), "wrong alignment");
+#endif
+
+
+void test0 ()
+{
+ int A1[5], B1;
+ #pragma omp allocate(A1, B1) align(512) allocator(omp_default_mem_alloc)
+
+#ifndef __cplusplus
+ _Static_assert (_Alignof(A1) == _Alignof(int[5]), "wrong alignment");
+ _Static_assert (_Alignof(B1) == _Alignof(int), "wrong alignment");
+#elif __cplusplus >= 201103L
+ static_assert (alignof(A1) == alignof(int[5]), "wrong alignment");
+ static_assert (alignof(B1) == alignof(int), "wrong alignment");
+#endif
+
+ if (((__UINTPTR_TYPE__) &A1 % 512) != 0)
+ __builtin_abort ();
+ if (((__UINTPTR_TYPE__) &B1 % 512) != 0)
+ __builtin_abort ();
+}
+
+int main()
+{
+ static int BBB [[omp::decl(allocate,allocator(omp_low_lat_mem_alloc),align(4096))]];
+
+#ifndef __cplusplus
+ _Static_assert (_Alignof(AAA) == _Alignof(int), "wrong alignment");
+#elif __cplusplus >= 201103L
+ static_assert (alignof(AAA) == alignof(int), "wrong alignment");
+#endif
+
+ if (((__UINTPTR_TYPE__) &AAA % 4096) != 0)
+ __builtin_abort ();
+ if (((__UINTPTR_TYPE__) &BBB % 4096) != 0)
+ __builtin_abort ();
+
+ test0 ();
+
+ return 0;
+}