RE: [PATCH v4] libgfortran: Replace mutex with rwlock

2023-09-14 Thread Zhu, Lipeng via Fortran
> Hi Thomas,
> 
> >
> > Hi Lipeng,
> >
> > > May I know any comment or concern on this patch, thanks for your
> > > time
> > > 😄
> >
> > Thanks for your patience in getting this reviewed.
> >
> > A few remarks / questions.
> >
> > Which strategy is used in this implementation, read-preferring or
> > write- preferring?  And if read-preferring is used, is there a danger
> > of deadlock if people do unreasonable things?
> > Maybe you could explain that, also in a comment in the code.
> >
> 
> Yes, the implementation use the read-preferring strategy, and comments in
> code.
> When adding the test cases, I didn’t meet the situation which may cause the
> deadlock.
> Maybe you can give more guidance about that.
> 
> > Can you add some sort of torture test case(s) which does a lot of
> > opening/closing/reading/writing, possibly with asynchronous I/O and/or
> > pthreads, to catch possible problems?  If there is a system dependency
> > or some race condition, chances are that regression testers will catch this.
> >
> 
> Sure, as your comments, in the patch V6, I added 3 test cases with OpenMP to
> test different cases in concurrency respectively:
> 1. find and create unit very frequently to stress read lock and write lock.
> 2. only access the unit which exist in cache to stress read lock.
> 3. access the same unit in concurrency.
> For the third test case, it also help to find a bug:  When unit can't be 
> found in
> cache nor unit list in read phase, then threads will try to acquire write 
> lock to
> insert the same unit, this will cause duplicate key error.
> To fix this bug, I get the unit from unit list once again before insert in 
> write lock.
> More details you can refer the patch v6.
> 

Could you help to review this update? I really appreciate your assistance.

> > With this, the libgfortran parts are OK, unless somebody else has more
> > comments, so give this a couple of days.  I cannot approve the libgcc
> > parts, that would be somebody else (Jakub?)
> >
> > Best regards
> >
> > Thomas
> >
> 
> Best Regards,
> Lipeng Zhu

Best Regards,
Lipeng Zhu


Re: [PATCH 8/8] OpenMP: Fortran "!$omp declare mapper" support

2023-09-14 Thread Bernhard Reutner-Fischer via Fortran
On Tue, 5 Sep 2023 12:28:28 -0700
Julian Brown  wrote:

> +  static bool
> +  equal (const omp_name_type &a,
> +  const omp_name_type &b)
> +  {
> +if (a.name == NULL_TREE && b.name == NULL_TREE)
> +  return a.type == b.type;

I'm curious if (and why) the type comparison above is safe and does not
use gfc_compare_types () ?

thanks,

> +else if (a.name == NULL_TREE || b.name == NULL_TREE)
> +  return false;
> +else
> +  return a.name == b.name && gfc_compare_types (a.type, b.type);
> +  }


[PATCH] Fortran: improve bounds-checking for array sections [PR30802]

2023-09-14 Thread Harald Anlauf via Fortran
Dear all,

array bounds checking was missing a few cases of array sections
that are handled via gfc_conv_expr_descriptor.  Bounds checking
was done for the dimensions with ranges, but not for elemental
dimensions.

The attached patch implements that and fixes pr30802 and also
pr97039, maybe a few more similar cases.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald

From bb2a765f56b440c8d086329f55c8ff0eaee2b97d Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Thu, 14 Sep 2023 22:08:26 +0200
Subject: [PATCH] Fortran: improve bounds-checking for array sections [PR30802]

gcc/fortran/ChangeLog:

	PR fortran/30802
	* trans-array.cc (trans_array_bound_check): Add optional argument
	COMPNAME for explicit specification of array component name.
	(array_bound_check_elemental): Helper function for generating
	bounds-checking code for elemental dimensions.
	(gfc_conv_expr_descriptor): Use bounds-checking also for elemental
	dimensions, i.e. those not handled by the scalarizer.

gcc/testsuite/ChangeLog:

	PR fortran/30802
	* gfortran.dg/bounds_check_fail_6.f90: New test.
---
 gcc/fortran/trans-array.cc| 72 ++-
 .../gfortran.dg/bounds_check_fail_6.f90   | 29 
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 6ca58e98547..71123e37477 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -3452,7 +3452,8 @@ gfc_conv_array_ubound (tree descriptor, int dim)

 static tree
 trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
-			 locus * where, bool check_upper)
+			 locus * where, bool check_upper,
+			 const char *compname = NULL)
 {
   tree fault;
   tree tmp_lo, tmp_up;
@@ -3474,6 +3475,10 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
   if (VAR_P (descriptor))
 name = IDENTIFIER_POINTER (DECL_NAME (descriptor));

+  /* Use given (array component) name.  */
+  if (compname)
+name = compname;
+
   /* If upper bound is present, include both bounds in the error message.  */
   if (check_upper)
 {
@@ -3524,6 +3529,67 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
 }


+/* Generate code for bounds checking for elemental dimensions.  */
+
+static void
+array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr)
+{
+  gfc_array_ref *ar;
+  gfc_ref *ref;
+  gfc_symbol *sym;
+  char *var_name = NULL;
+  size_t len;
+  int dim;
+
+  if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS))
+return;
+
+  if (expr->expr_type == EXPR_VARIABLE)
+{
+  sym = expr->symtree->n.sym;
+  len = strlen (sym->name) + 1;
+
+  for (ref = expr->ref; ref; ref = ref->next)
+	if (ref->type == REF_COMPONENT)
+	  len += 2 + strlen (ref->u.c.component->name);
+
+  var_name = XALLOCAVEC (char, len);
+  strcpy (var_name, sym->name);
+
+  for (ref = expr->ref; ref; ref = ref->next)
+	{
+	  /* Append component name.  */
+	  if (ref->type == REF_COMPONENT)
+	{
+	  strcat (var_name, "%%");
+	  strcat (var_name, ref->u.c.component->name);
+	  continue;
+	}
+
+	  if (ref->type == REF_ARRAY && ref->u.ar.dimen > 0)
+	{
+	  ar = &ref->u.ar;
+	  for (dim = 0; dim < ar->dimen; dim++)
+		{
+		  if (ar->dimen_type[dim] == DIMEN_ELEMENT)
+		{
+		  gfc_se indexse;
+		  gfc_init_se (&indexse, NULL);
+		  gfc_conv_expr_type (&indexse, ar->start[dim],
+	  gfc_array_index_type);
+		  trans_array_bound_check (se, ss, indexse.expr, dim,
+	   &ar->where,
+	   ar->as->type != AS_ASSUMED_SIZE
+	   || dim < ar->dimen - 1,
+	   var_name);
+		}
+		}
+	}
+	}
+}
+}
+
+
 /* Return the offset for an index.  Performs bound checking for elemental
dimensions.  Single element references are processed separately.
DIM is the array dimension, I is the loop dimension.  */
@@ -7823,6 +7889,10 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
   /* Setup the scalarizing loops and bounds.  */
   gfc_conv_ss_startstride (&loop);

+  /* Add bounds-checking for elemental dimensions.  */
+  if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) && !expr->no_bounds_check)
+array_bound_check_elemental (se, ss, expr);
+
   if (need_tmp)
 {
   if (expr->ts.type == BT_CHARACTER
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90
new file mode 100644
index 000..90329131158
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90
@@ -0,0 +1,29 @@
+! { dg-do run }
+! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" }
+! { dg-output "At line 18 .*" }
+! { dg-shouldfail "dimension 3 of array 'u%z' outside of expected range" }
+!
+! PR fortran/30802 - improve bounds-checking for array sections
+
+program test
+  implicit none
+  integer :: k = 0
+  int