Re: [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD
On 1/10/2024 7:52 PM, Richard Earnshaw wrote: On 05/01/2024 01:43, Lipeng Zhu wrote: This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is not defined in dec_waiting_unlocked function. As io.h does not include async.h, the WRLOCK and RWUNLOCK macros are undefined. libgfortran/ChangeLog: * io/io.h (dec_waiting_unlocked): Use __gthread_rwlock_wrlock/__gthread_rwlock_unlock or __gthread_mutex_lock/__gthread_mutex_unlock functions to replace WRLOCK and RWUNLOCK macros. Signed-off-by: Lipeng Zhu Has this been committed yet? R. Hi Richard, The patch is waiting for community's review. Hi Tobias, Any concern about this patch? Best Regards, Lipeng Zhu --- libgfortran/io/io.h | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index 15daa0995b1..c7f0f7d7d9e 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else - WRLOCK (&unit_rwlock); +#ifdef __GTHREAD_RWLOCK_INIT + __gthread_rwlock_wrlock (&unit_rwlock); + u->waiting--; + __gthread_rwlock_unlock (&unit_rwlock); +#else + __gthread_mutex_lock (&unit_rwlock); u->waiting--; - RWUNLOCK (&unit_rwlock); + __gthread_mutex_unlock (&unit_rwlock); +#endif #endif }
[PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]
Dear all,
we are accepting loop annotations IVDEP, UNROLL n, VECTOR, and NOVECTOR
for ordinary do loops, but ICE when such an annotation is specified
before a DO CONCURRENT loop.
Since at least the Intel compilers recognize some of the annotations
also for DO CONCURRENT, it seems natural to extend gfortran instead
of rejecting or ignoring the attributes.
The attached patch handles the annotations as needed for the control
structures of FORALL/DO CONCURRENT.
Regarding the UNROLL directive, I don't have good references, so
feedback is welcome. The current patch applies UNROLL only to
the first loop control variable (for the case of loop nests),
which translates into the innermost loop in gcc's representation.
Regtested on x86_64-pc-linux-gnu.
OK for mainline?
Comments?
Thanks,
Harald
From 0df60f02c399a6bf65850ecd5719b25b3de6676f Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Wed, 10 Jan 2024 23:10:02 +0100
Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]
gcc/fortran/ChangeLog:
PR fortran/113305
* gfortran.h: Add annotation controls to gfc_forall_iterator.
* gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR,
NOVECTOR as applied to DO CONCURRENT.
* parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n,
VECTOR, NOVECTOR as applied to DO CONCURRENT. Apply UNROLL only to
first loop control variable.
* trans-stmt.cc (gfc_trans_forall_loop): Annotate loops with IVDEP,
UNROLL n, VECTOR, NOVECTOR as needed for DO CONCURRENT.
(gfc_trans_forall_1): Handle annotations IVDEP, UNROLL n, VECTOR,
NOVECTOR.
gcc/testsuite/ChangeLog:
PR fortran/113305
* gfortran.dg/do_concurrent_7.f90: New test.
---
gcc/fortran/gfortran.h| 4 +++
gcc/fortran/gfortran.texi | 12
gcc/fortran/parse.cc | 26 -
gcc/fortran/trans-stmt.cc | 29 ++-
gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 +
5 files changed, 95 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 82f388c05f8..88502c1e3f0 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2926,6 +2926,10 @@ gfc_dt;
typedef struct gfc_forall_iterator
{
gfc_expr *var, *start, *end, *stride;
+ unsigned short unroll;
+ bool ivdep;
+ bool vector;
+ bool novector;
struct gfc_forall_iterator *next;
}
gfc_forall_iterator;
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 5615fee2897..371666dcbb6 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the
loop that follows. N is an integer constant specifying the unrolling factor.
The values of 0 and 1 block any unrolling of the loop.
+For @code{DO CONCURRENT} constructs the unrolling specification applies
+only to the first loop control variable.
+
@node BUILTIN directive
@subsection BUILTIN directive
@@ -3300,6 +3303,9 @@ whether a particular loop is vectorizable due to potential
dependencies between iterations. The purpose of the directive is to
tell the compiler that vectorization is safe.
+For @code{DO CONCURRENT} constructs this annotation is implicit to all
+loop control variables.
+
This directive is intended for annotation of existing code. For new
code it is recommended to consider OpenMP SIMD directives as potential
alternative.
@@ -3316,6 +3322,9 @@ This directive tells the compiler to vectorize the following loop. It
must be placed immediately before a @code{DO} loop and applies only to
the loop that follows.
+For @code{DO CONCURRENT} constructs this annotation applies to all loops
+specified in the concurrent header.
+
@node NOVECTOR directive
@subsection NOVECTOR directive
@@ -3328,6 +3337,9 @@ This directive tells the compiler to not vectorize the following loop.
It must be placed immediately before a @code{DO} loop and applies only
to the loop that follows.
+For @code{DO CONCURRENT} constructs this annotation applies to all loops
+specified in the concurrent header.
+
@node Non-Fortran Main Program
@section Non-Fortran Main Program
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index d8b38cfb5ac..f41cc7d3510 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -5307,7 +5307,31 @@ parse_do_block (void)
do_op = new_st.op;
s.ext.end_do_label = new_st.label1;
- if (new_st.ext.iterator != NULL)
+ if (do_op == EXEC_DO_CONCURRENT)
+{
+ gfc_forall_iterator *fa;
+ for (fa = new_st.ext.forall_iterator; fa; fa = fa->next)
+ {
+ /* Apply unroll only to innermost loop (first control
+ variable). */
+ if (directive_unroll != -1)
+ {
+ fa->unroll = directive_unroll;
+ directive_unroll = -1;
+ }
+ if (directive_ivdep)
+ fa->ivdep = directive_ivdep;
+ i
Re: [PATCH 2/8] OpenMP: lvalue parsing for map/to/from clauses (C)
Julian Brown wrote:
This patch adds support for parsing general lvalues ("locator list item
types") for OpenMP "map", "to" and "from" clauses to the C front-end,
similar to the previously-posted patch for C++. Such syntax is permitted
for OpenMP 5.0 and above. It was previously posted for mainline here
...
In libgomp/libgomp.texi, the following can now be set to 'Y':
@item C/C++'s lvalue expressions in @code{to}, @code{from}
and @code{map} clauses @tab N @tab
@@ -11253,16 +11263,41 @@ c_parser_postfix_expression_after_primary (c_parser
*parser,
case CPP_OPEN_SQUARE:
/* Array reference. */
c_parser_consume_token (parser);
- idx = c_parser_expression (parser).value;
- c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE,
-"expected %<]%>");
- start = expr.get_start ();
- finish = parser->tokens_buf[0].location;
- expr.value = build_array_ref (op_loc, expr.value, idx);
- set_c_expr_source_range (&expr, start, finish);
- expr.original_code = ERROR_MARK;
- expr.original_type = NULL;
- expr.m_decimal = 0;
+ idx = len = NULL_TREE;
+ if (!c_omp_array_section_p
+ || c_parser_next_token_is_not (parser, CPP_COLON))
+ idx = c_parser_expression (parser).value;
+
+ if (c_omp_array_section_p
+ && c_parser_next_token_is (parser, CPP_COLON))
+ {
+ c_parser_consume_token (parser);
+ if (c_parser_next_token_is_not (parser, CPP_CLOSE_SQUARE))
+ len = c_parser_expression (parser).value;
+
+ c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE,
+"expected %<]%>");
+
+ start = expr.get_start ();
+ finish = parser->tokens_buf[0].location;
+ expr.value = build_omp_array_section (op_loc, expr.value, idx,
+ len);
+ set_c_expr_source_range (&expr, start, finish);
+ expr.original_code = ERROR_MARK;
+ expr.original_type = NULL;
+ }
+ else
+ {
+ c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE,
+"expected %<]%>");
+ start = expr.get_start ();
+ finish = parser->tokens_buf[0].location;
+ expr.value = build_array_ref (op_loc, expr.value, idx);
+ set_c_expr_source_range (&expr, start, finish);
+ expr.original_code = ERROR_MARK;
+ expr.original_type = NULL;
+ expr.m_decimal = 0;
+ }
I think that's more readable when moving everything but the expr.value
assignment after the if/else (obviously for "if" also everything until
"len =" has to remain). That also adds the missing "m_decimal = 0" for
the "if" case.
@@ -13915,8 +13955,97 @@ c_parser_omp_variable_list (c_parser *parser,
...
+ else if (TREE_CODE (decl) == INDIRECT_REF)
+ {
+ /* Turn *foo into the representation previously used for
+foo[0]. */
+ decl = TREE_OPERAND (decl, 0);
+ STRIP_NOPS (decl);
+
+ decl = build_omp_array_section (loc, decl, integer_zero_node,
+ integer_one_node);
+ }
I wonder whether we shouldn't use the C++ wording, i.e.
/* If we have "*foo" and
- it's an indirection of a reference, "unconvert" it,
i.e. strip the indirection (to just "foo").
- it's an indirection of a pointer, turn it into
"foo[0:1]". */
* * *
As remarked for cp/typecheck.cc's build_omp_array_section:
+tree
+build_omp_array_section (location_t loc, tree array, tree index, tree length)
+{
+ tree idxtype;
+
+ if (index != NULL_TREE
+ && length != NULL_TREE
+ && INTEGRAL_TYPE_P (TREE_TYPE (index))
+ && INTEGRAL_TYPE_P (TREE_TYPE (length)))
+{
+ tree low = fold_convert (sizetype, index);
+ tree high = fold_convert (sizetype, length);
+ high = size_binop (PLUS_EXPR, low, high);
+ high = size_binop (MINUS_EXPR, high, size_one_node);
+ idxtype = build_range_type (sizetype, low, high);
+}
+ else if ((index == NULL_TREE || integer_zerop (index))
+ && length != NULL_TREE
+ && INTEGRAL_TYPE_P (TREE_TYPE (length)))
+idxtype = build_index_type (length);
+ else
+idxtype = NULL_TREE;
+
+ tree type = TREE_TYPE (array);
+ gcc_assert (type);
+
+ tree sectype, eltype = TREE_TYPE (type);
+
+ /* It's not an array or pointer type. Just reuse the type of the original
+ expression as the type of the array section (an error will be raised
+ anyway, later). */
+ if (eltype == NULL_TREE
+ || error_operand_p (eltype)
+ || error_operand_p (idxtype))
+sectype = TREE_TYPE (array);
+ else
+
Re: [PATCH 1/8] OpenMP: lvalue parsing for map/to/from clauses (C++)
On Wed, 10 Jan 2024 10:14:41 +0100 Jakub Jelinek wrote: > On Fri, Jan 05, 2024 at 12:23:26PM +, Julian Brown wrote: > > * g++.dg/gomp/bad-array-section-10.C: New test. > > This test FAILs in C++23/C++26 modes, just try > make check-g++ GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 > RUNTESTFLAGS=gomp.exp=bad-array-section-10.C While in C++20 comma in > array references was deprecated, in C++23 we implement > multidimensional arrays, so the diagnostics there is different. See > https://wg21.link/p2036r3 Thanks -- I've pushed a patch to fix this. The bad-array-section-11.C test covered the C++23 case already, but I don't think normal testing iterates the newer language standards, hence missing this. Julian
Re: Can you send me URL to download gfortran
https://gcc.gnu.org/wiki/GFortranBinaries Op wo 10 jan 2024 om 11:54 schreef abhay dutta : > >
Re: [PATCH v2] libgfortran: Bugfix if not define HAVE_ATOMIC_FETCH_ADD
On 05/01/2024 01:43, Lipeng Zhu wrote: This patch try to fix the bug when HAVE_ATOMIC_FETCH_ADD is not defined in dec_waiting_unlocked function. As io.h does not include async.h, the WRLOCK and RWUNLOCK macros are undefined. libgfortran/ChangeLog: * io/io.h (dec_waiting_unlocked): Use __gthread_rwlock_wrlock/__gthread_rwlock_unlock or __gthread_mutex_lock/__gthread_mutex_unlock functions to replace WRLOCK and RWUNLOCK macros. Signed-off-by: Lipeng Zhu Has this been committed yet? R. --- libgfortran/io/io.h | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index 15daa0995b1..c7f0f7d7d9e 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else - WRLOCK (&unit_rwlock); +#ifdef __GTHREAD_RWLOCK_INIT + __gthread_rwlock_wrlock (&unit_rwlock); + u->waiting--; + __gthread_rwlock_unlock (&unit_rwlock); +#else + __gthread_mutex_lock (&unit_rwlock); u->waiting--; - RWUNLOCK (&unit_rwlock); + __gthread_mutex_unlock (&unit_rwlock); +#endif #endif }
Can you send me URL to download gfortran
Re: [PATCH 1/8] OpenMP: lvalue parsing for map/to/from clauses (C++)
On Fri, Jan 05, 2024 at 12:23:26PM +, Julian Brown wrote: > * g++.dg/gomp/bad-array-section-10.C: New test. This test FAILs in C++23/C++26 modes, just try make check-g++ GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 RUNTESTFLAGS=gomp.exp=bad-array-section-10.C While in C++20 comma in array references was deprecated, in C++23 we implement multidimensional arrays, so the diagnostics there is different. See https://wg21.link/p2036r3 Jakub
