Re: [PATCH, v2, OpenMP 5.2, Fortran] Strictly-structured block support for OpenMP directives

2021-10-21 Thread Chung-Lin Tang



On 2021/10/21 12:15 AM, Jakub Jelinek wrote:

+program main
+  integer :: x, i, n
+
+  !$omp parallel
+  block
+x = x + 1
+  end block

I'd prefer not to use those x = j or x = x + 1 etc.
as statements that do random work here whenever possible.
While those are dg-do compile testcases, especially if
it is without dg-errors I think it is preferrable not to show
bad coding examples.
E.g. the x = x + 1 above is wrong for 2 reasons, x is uninitialized
before the parallel, and there is a data race, the threads, teams etc.
can write to x concurrently.
I think better would be to use something like
 call do_work
which doesn't have to be defined anywhere and will just stand there
as a black box for unspecified work.


+  !$omp workshare
+  block
+x = x + 1
+  end block

There are exceptions though, e.g. workshare is such a case, because
e.g. call do_work is not valid in workshare.
So, it is ok to keep using x = x + 1 here if you initialize it
first at the start of the program.


+  !$omp workshare
+  block
+x = 1
+!$omp critical
+block
+  x = 3
+end block
+  end block

And then there are cases like the above, please
just use different variables there (all initialized) or
say an array and access different elements in the different spots.

Jakub



Thanks, attached is what I finally committed.

Chung-Lin



From 2e4659199e814b7ee0f6bd925fd2c0a7610da856 Mon Sep 17 00:00:00 2001
From: Chung-Lin Tang 
Date: Thu, 21 Oct 2021 14:56:20 +0800
Subject: [PATCH] openmp: Fortran strictly-structured blocks support

This implements strictly-structured blocks support for Fortran, as specified in
OpenMP 5.2. This now allows using a Fortran BLOCK construct as the body of most
OpenMP constructs, with a "!$omp end ..." ending directive optional for that
form.

gcc/fortran/ChangeLog:

* decl.c (gfc_match_end): Add COMP_OMP_STRICTLY_STRUCTURED_BLOCK case
together with COMP_BLOCK.
* parse.c (parse_omp_structured_block): Change return type to
'gfc_statement', add handling for strictly-structured block case, adjust
recursive calls to parse_omp_structured_block.
(parse_executable): Adjust calls to parse_omp_structured_block.
* parse.h (enum gfc_compile_state): Add
COMP_OMP_STRICTLY_STRUCTURED_BLOCK.
* trans-openmp.c (gfc_trans_omp_workshare): Add EXEC_BLOCK case
handling.

gcc/testsuite/ChangeLog:

* gfortran.dg/gomp/cancel-1.f90: Adjust testcase.
* gfortran.dg/gomp/nesting-3.f90: Adjust testcase.
* gfortran.dg/gomp/strictly-structured-block-1.f90: New test.
* gfortran.dg/gomp/strictly-structured-block-2.f90: New test.
* gfortran.dg/gomp/strictly-structured-block-3.f90: New test.

libgomp/ChangeLog:

* libgomp.texi (Support of strictly structured blocks in Fortran):
Adjust to 'Y'.
* testsuite/libgomp.fortran/task-reduction-16.f90: Adjust testcase.
---
 gcc/fortran/decl.c|   1 +
 gcc/fortran/parse.c   |  69 +-
 gcc/fortran/parse.h   |   2 +-
 gcc/fortran/trans-openmp.c|   6 +-
 gcc/testsuite/gfortran.dg/gomp/cancel-1.f90   |   3 +
 gcc/testsuite/gfortran.dg/gomp/nesting-3.f90  |  20 +-
 .../gomp/strictly-structured-block-1.f90  | 214 ++
 .../gomp/strictly-structured-block-2.f90  | 139 
 .../gomp/strictly-structured-block-3.f90  |  52 +
 libgomp/libgomp.texi  |   2 +-
 .../libgomp.fortran/task-reduction-16.f90 |   1 +
 11 files changed, 484 insertions(+), 25 deletions(-)
 create mode 100644 
gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-1.f90
 create mode 100644 
gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-2.f90
 create mode 100644 
gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-3.f90

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 6784b07ae9e..6043e100fbb 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -8429,6 +8429,7 @@ gfc_match_end (gfc_statement *st)
   break;
 
 case COMP_BLOCK:
+case COMP_OMP_STRICTLY_STRUCTURED_BLOCK:
   *st = ST_END_BLOCK;
   target = " block";
   eos_ok = 0;
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 2a454be79b0..b1e73ee6801 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -5459,7 +5459,7 @@ parse_oacc_loop (gfc_statement acc_st)
 
 /* Parse the statements of an OpenMP structured block.  */
 
-static void
+static gfc_statement
 parse_omp_structured_block (gfc_statement omp_st, bool workshare_stmts_only)
 {
   gfc_statement st, omp_end_st;
@@ -5546,6 +5546,32 @@ parse_omp_structured_block (gfc_statement omp_st, bool 
workshare_stmts_only)
   gcc_unreachable ();
 }
 
+  bool block_construct = false;
+  gfc_namespace *my_ns = NULL;
+  gfc_namespace *my_parent = NULL;
+
+  st = next_statement ();
+
+  if (st == ST_BLOCK)
+{
+  /* Adjust stat

[wwwdocs, committed] GCC 12: Add release note for Fortran TS29113 improvements

2021-10-21 Thread Sandra Loosemore
I've checked in the attached patch to announce the cleanup project that 
Tobias and I have been working on over the last several months in the 
GCC 12 release notes.  I also updated the page for TS29113 on the GCC 
wiki to reflect that anything that still doesn't work ought to be 
considered a bug, not just incomplete work-in-progress.


I know that the conformance sections of the Fortran manual are badly in 
need of updating too (not just for TS29113, but the various versions of 
the standard).  That's in my queue to fix up, but I'm likely going to 
need assistance from the gfortran maintainers to get the details right.  :-S


-Sandra
commit f5971f451ae8834e928738bbfe465670aa481cea
Author: Sandra Loosemore 
Date:   Thu Oct 21 09:00:16 2021 -0700

GCC 12: Add release note for Fortran TS29113 improvements.

diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html
index 5be1570..5974b9b 100644
--- a/htdocs/gcc-12/changes.html
+++ b/htdocs/gcc-12/changes.html
@@ -147,7 +147,17 @@ a work-in-progress.
 
 
 
-
+Fortran
+
+  WG5/N1942, "TS 29113 Further Interoperability of Fortran with C",
+is now fully supported.  In addition to implementing previously
+missing functionality, such as support for character arguments of
+length greater than one in functions marked bind(c)
+and gaps in the handling for assumed-rank arrays, numerous other bugs
+have been fixed, and an extensive set of new conformance test cases
+has been added.
+  
+
 
 
 


Re: [wwwdocs, committed] GCC 12: Add release note for Fortran TS29113 improvements

2021-10-21 Thread Thomas Koenig via Fortran

Hi Sandra,

I've checked in the attached patch to announce the cleanup project that 
Tobias and I have been working on over the last several months in the 
GCC 12 release notes.  I also updated the page for TS29113 on the GCC 
wiki to reflect that anything that still doesn't work ought to be 
considered a bug, not just incomplete work-in-progress.


Thanks for the work that the both of you put into this, and also thanks
for putting this into the release notes.  I was about to suggest you do
so, so you beat me to it :-)

I know that the conformance sections of the Fortran manual are badly in 
need of updating too (not just for TS29113, but the various versions of 
the standard).  That's in my queue to fix up, but I'm likely going to 
need assistance from the gfortran maintainers to get the details right.  
:-S


Just ask :-)

Regards

Thomas



[PATCH, Fortran] Add testcase for PR100906

2021-10-21 Thread Sandra Loosemore
PR100906 ("Bind(c): failure handling character with len/=1") has been 
fixed by Tobias's rewrite of the GFC <-> C descriptor conversions.  I'd 
like to add José's testcase for that issue before closing it.  OK?


-Sandra
commit 4c2fa9cf74162015710ccfd913c827779151aa52
Author: Sandra Loosemore 
Date:   Thu Oct 21 19:17:50 2021 -0700

Add testcase for PR fortran/100906

2021-10-21  José Rui Faustino de Sousa  
	Sandra Loosemore  

	gcc/testsuite/

	PR fortran/100906
	* gfortran.dg/PR100906.f90: New.
	* gfortran.dg/PR100906.c: New.

diff --git a/gcc/testsuite/gfortran.dg/PR100906.c b/gcc/testsuite/gfortran.dg/PR100906.c
new file mode 100644
index 000..f71d567
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR100906.c
@@ -0,0 +1,169 @@
+/* Test the fix for PR100906 */
+
+#include 
+#include 
+#include 
+#include 
+/* #include  */
+
+#include 
+
+#define _CFI_type_mask 0xFF
+#define _CFI_type_kind_shift 8
+
+#define _CFI_decode_type(NAME) (signed char)((NAME) & CFI_type_mask)
+#define _CFI_decode_kind(NAME) (signed char)(((NAME) >> CFI_type_kind_shift) & CFI_type_mask)
+
+#define _CFI_encode_type(TYPE, KIND) (int16_t)\
+KIND) & CFI_type_mask) << CFI_type_kind_shift)\
+ | ((TYPE) & CFI_type_mask))
+
+#define N 11
+#define M 7
+
+typedef char c_char;
+/* typedef char32_t c_ucs4_char; */
+typedef uint32_t char32_t;
+typedef uint32_t c_ucs4_char;
+ 
+bool charcmp (char *, char, size_t);
+
+bool ucharcmp (char32_t *, char32_t, size_t);
+
+bool c_vrfy_c_char (const CFI_cdesc_t *restrict, const size_t);
+
+bool c_vrfy_c_ucs4_char (const CFI_cdesc_t *restrict, const size_t);
+
+bool c_vrfy_character (const CFI_cdesc_t *restrict, const size_t);
+ 
+void check_tk (const CFI_cdesc_t*restrict, const CFI_type_t, const signed char, const size_t, const size_t);
+
+bool
+charcmp (char *c, char v, size_t n)
+{
+  bool res = true;
+  char b = (char)'A';
+  size_t i;
+
+  for (i=0; ((ibase_addr);
+  assert (auxp->elem_len>0);
+  lb = auxp->dim[0].lower_bound;
+  ex = auxp->dim[0].extent;
+  assert (ex==N);
+  sz = (size_t)auxp->elem_len / sizeof (c_char);
+  assert (sz==len);
+  ub = ex + lb - 1;
+  ip = (c_char*)auxp->base_addr;
+  for (i=0; ibase_addr);
+  assert (auxp->elem_len>0);
+  lb = auxp->dim[0].lower_bound;
+  ex = auxp->dim[0].extent;
+  assert (ex==N);
+  sz = (size_t)auxp->elem_len / sizeof (c_ucs4_char);
+  assert (sz==len);
+  ub = ex + lb - 1;
+  ip = (c_ucs4_char*)auxp->base_addr;
+  for (i=0; itype);
+  kind = _CFI_decode_kind(auxp->type);
+  assert (type == CFI_type_Character);
+  switch (kind)
+{
+case 1:
+  return c_vrfy_c_char (auxp, len);
+  break;
+case 4:
+  return c_vrfy_c_ucs4_char (auxp, len);
+  break;
+default:
+  assert (false);
+}
+  return true;
+}
+
+void
+check_tk (const CFI_cdesc_t *restrict auxp, const CFI_type_t type, const signed char kind, const size_t elem_len, const size_t nelem)
+{
+  signed char ityp, iknd;
+
+  assert (auxp);
+  assert (auxp->elem_len==elem_len*nelem);
+  assert (auxp->rank==1);
+  assert (auxp->dim[0].sm>0);
+  assert ((size_t)auxp->dim[0].sm==elem_len*nelem);
+  /*  */
+  assert (auxp->type==type);
+  ityp = _CFI_decode_type(auxp->type);
+  assert (ityp == CFI_type_Character);
+  iknd = _CFI_decode_kind(auxp->type);
+  assert (_CFI_decode_type(type)==ityp);
+  assert (kind==iknd);
+  assert (c_vrfy_character (auxp, nelem));
+  return;
+}
+
+// Local Variables:
+// mode: C
+// End:
diff --git a/gcc/testsuite/gfortran.dg/PR100906.f90 b/gcc/testsuite/gfortran.dg/PR100906.f90
new file mode 100644
index 000..f6cb3af
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR100906.f90
@@ -0,0 +1,1699 @@
+! { dg-do run }
+! { dg-additional-sources PR100906.c }
+!
+! Test the fix for PR100906
+! 
+
+module isof_m
+
+  use, intrinsic :: iso_c_binding, only: &
+c_signed_char, c_int16_t
+  
+  implicit none
+
+  private
+  
+  public :: &
+CFI_type_character
+
+  public :: &
+CFI_type_char,  &
+CFI_type_ucs4_char
+ 
+  public ::  &
+check_tk_as, &
+check_tk_ar
+  
+  
+  public ::  &
+cfi_encode_type
+  
+  integer, parameter :: CFI_type_t = c_int16_t
+  
+  integer(kind=c_int16_t), parameter :: CFI_type_mask = int(z"FF", kind=c_int16_t)
+  integer(kind=c_int16_t), parameter :: CFI_type_kind_shift = 8_c_int16_t
+
+  ! Intrinsic types. Their kind number defines their storage size. */
+  integer(kind=c_signed_char), parameter :: CFI_type_Character = 5
+
+  ! C-Fortran Interoperability types.
+  integer(kind=cfi_type_t), parameter :: CFI_type_char  = &
+ior(int(CFI_type_Character, kind=c_int16_t), shiftl(1_c_int16_t, CFI_type_kind_shift))
+  integer(kind=cfi_type_t), parameter :: CFI_type_ucs4_char = &
+ior(int(CFI_type_Character, kind=c_int16_t), shiftl(4_c_int16_t, CFI_type_kind_shift))
+
+  interface
+subroutine check_tk_as(a, t, k, e, n) &
+  bind(c, name="check_tk")
+  use, intrinsic :: iso_c_binding, 

Re: [PATCH, Fortran] Add testcase for PR100906

2021-10-21 Thread Thomas Koenig via Fortran



Hi Sandra,

PR100906 ("Bind(c): failure handling character with len/=1") has been 
fixed by Tobias's rewrite of the GFC <-> C descriptor conversions.  I'd 
like to add José's testcase for that issue before closing it.  OK?


OK.  I think adding undisputed passing test cases from PRs for something
that works can also be considered simple and obvious.

Best regards

Thomas