[Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran

[Andre Vehreschild <vehre@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 4725 bytes --]

Hi folks,

please find attached the library part for supporting RANDOM_INIT for
coarray=lib enabled fortran translations. There is also a patch for the
Opencoarray library to add RANDOM_INIT there.

I am not sure, whether I have modified the gfortran.map file in the libgfortran
directory correctly, or even had to. So please take specific care if that is
correct.

Bootstrapped and regtested fine on x86_64/f33.

Ok for trunk?

Regards,
	Andre

PR fortran/98301 - random_init() is broken

Correct implementation of random_init() when -fcoarray=lib is given.

gcc/fortran/ChangeLog:

	PR fortran/98301
	* trans-decl.c (gfc_build_builtin_function_decls): Move decl.
	* trans-intrinsic.c (conv_intrinsic_random_init): Use bool for
	lib-call of caf_random_init instead of logical (4-byte).

libgfortran/ChangeLog:

	PR fortran/98301
	* caf/libcaf.h (_gfortran_caf_random_init): New function.
	* caf/single.c (_gfortran_caf_random_init): New function.
	* gfortran.map: Added fndecl.
	* intrinsics/random_init.f90: Modified comment.



On Sat, 3 Apr 2021 22:33:31 -0700
Steve Kargl via Fortran <fortran@gcc.gnu.org> wrote:

> On Sat, Apr 03, 2021 at 08:30:31PM -0700, Damian Rouson wrote:
> > Hi Steve,
> >
> > I hope the gfortran developers won't commit a patch that replaces
> > existing behavior with a stub that simply emits an error message.
>
> The current behavior is incorrect with respect to the Fortran standard
> if one runs a compiled program multiple times.  The patch fixes a bug.
>
> > It has been a while since I looked at the previous emails regarding
> > problems with the current behavior so I'm not expressing an opinion
> > about the current behavior.  I'm simply advocating against breaking
> > existing codes that rely on the current behavior if nothing better is
> > being provided.
>
> It cannot be helped.  The underlying coarray implementation must
> handle RANDOM_INIT().  AFAICT, there must be communication between
> images if RANDOM_INIT() is used.  libgfortran is not compiled with
> -fcoarray=lib (or -fcoarray=shared), so the coarray implementation(s)
> must deal with RANDOM_INIT().
>
> > Or as a compromise, would you mind changing the patch so that the
> > error message is emitted only in the problematic cases? You presented
> > one problematic case out of the four possible combinations of
> > RANDOM_INIT() arguments.  With only four possible combinations to
> > enumerate, I hope this suggestion isn't burdensome.
> Consider,
>
>    read(*,*) repeatable, image_distinct
>    call random_init(repeatable, image_distinct)
>
> for -fcoarray=none or -fcoarray=single, the image_distinct
> argument is irrevelant.   This is simply translated to
>
> _gfortran_random_init(repeatable, image_distinct)
>
> For -fcoarray=lib (and by extension OpenCoarray), a simple 1 line
> patch on top of my patch would generate
>
> _gfortran_caf_random_init(repeatable, image_distinct)
>
> where is it assumed the function is coarray aware.  Similarly, if
> -fcoarray=shared, then a 1 line patch would generate
>
> _gfortran_cas_random_init(repeatable, image_distinct)
>
> where is it assumed the function is coarray aware.
>
> There are 4 cases:
> (1)   repeatable=.true., image_distinct=.true.
> (2)   repeatable=.true., image_distinct=.false.
> (3)   repeatable=.false., image_distinct=.true.
> (4)   repeatable=.false., image_distinct=.false.
>
> IIRC, cases (1)-(3) don't require communication.  case (4) does.
> That is, case (4) requires the same set of random seeds to be
> used on all images.
>
> > Regarding "someone who cares about coarray Fortran," finding people to
> > work on such an effort is quite challenging.
>
> Finding people willing to work on gfortran is as challenging if
> not more so.  A year ago, I posted in c.l.f a list of 20+ PRs
> with patches that I had created.  I don't do git.  It would take
> someone with git-fu little time to take my patches and apply
> them to a source tree.  Testing was done by me, but I would
> encourage git-fu aware individuals to bootstrap and test the patches.
> Then commit the patch.  Harald has stepped up and grabbed a few.
> TO BE CLEAR, I AM NOT RANTING AT THE PEOPLE WHO HAVE CONTRIBUTED
> AND MAINTAINED GFORTRAN FOR YEARS.  gfortran needs new blood, or
> it is destined for the bit bucket.
>
> > I believe Andre
> > Verheschild has some limited availability so I'm cc'ing him and will
> > discuss it with him if he's interested.  If you know others who might
> > be interested, please let us know.
>
> Don't know any new people who are interested in gfortran.
>


--
Andre Vehreschild * Email: vehre ad gmx dot de

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr98301.patch --]
[-- Type: text/x-patch, Size: 6287 bytes --]

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index c99336cb19d..de0e8fb0314 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -170,6 +170,7 @@ tree gfor_fndecl_co_min;
 tree gfor_fndecl_co_reduce;
 tree gfor_fndecl_co_sum;
 tree gfor_fndecl_caf_is_present;
+tree gfor_fndecl_caf_random_init;


 /* Math functions.  Many other math functions are handled in
@@ -234,7 +235,6 @@ tree gfor_fndecl_zgemm;

 /* RANDOM_INIT function.  */
 tree gfor_fndecl_random_init;      /* libgfortran, 1 image only.  */
-tree gfor_fndecl_caf_random_init;  /* OpenCoarray call.  */
 tree gfor_fndecl_cas_random_init;  /* Shared memory coarray.  */

 static void
@@ -3517,23 +3517,17 @@ gfc_build_intrinsic_function_decls (void)
 	void_type_node, 3, gfc_logical4_type_node, gfc_logical4_type_node,
 	gfc_int4_type_node);

+ // gfor_fndecl_caf_rand_init is defined in the lib-coarray section below.
+
 /* FIXME: This is a temporary workaround until someone that uses coarray
    Fortran and random_init() can implement the OpenCoarray and/or the
    shared memory routines.  Both require communication between images, so
    these routines cannot live in libgfortran.  */
 #if 1
-  gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
-	get_identifier (PREFIX("random_init_foobar")),
-	void_type_node, 2, gfc_logical4_type_node, gfc_logical4_type_node);
-
   gfor_fndecl_cas_random_init = gfc_build_library_function_decl (
 	get_identifier (PREFIX("random_init_foobar")),
 	void_type_node, 2, gfc_logical4_type_node, gfc_logical4_type_node);
 #else
-  gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
-	get_identifier (PREFIX("caf_random_init")),
-	void_type_node, 2, gfc_logical4_type_node, gfc_logical4_type_node);
-
   gfor_fndecl_cas_random_init = gfc_build_library_function_decl (
 	get_identifier (PREFIX("cas_random_init")),
 	void_type_node, 2, gfc_logical4_type_node, gfc_logical4_type_node);
@@ -4104,6 +4098,10 @@ gfc_build_builtin_function_decls (void)
 	get_identifier (PREFIX("caf_is_present")), ". r . r ",
 	integer_type_node, 3, pvoid_type_node, integer_type_node,
 	pvoid_type_node);
+
+      gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
+	    get_identifier (PREFIX("caf_random_init")),
+	    void_type_node, 2, logical_type_node, logical_type_node);
     }

   gfc_build_intrinsic_function_decls ();
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 900c6198d9e..8319e94b893 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -3828,27 +3828,27 @@ conv_intrinsic_random_init (gfc_code *code)
   stmtblock_t block;
   gfc_se se;
   tree arg1, arg2, tmp;
-  tree logical4_type_node = gfc_get_logical_type (4);
+  /* On none coarray == lib compiles use LOGICAL(4) else regular LOGICAL.  */
+  tree used_bool_type_node = flag_coarray == GFC_FCOARRAY_LIB
+			     ? logical_type_node
+			     : gfc_get_logical_type (4);

   /* Make the function call.  */
   gfc_init_block (&block);
   gfc_init_se (&se, NULL);

-  /* Convert REPEATABLE to a LOGICAL(4) entity.  */
+  /* Convert REPEATABLE to the desired LOGICAL entity.  */
   gfc_conv_expr (&se, code->ext.actual->expr);
   gfc_add_block_to_block (&block, &se.pre);
-  arg1 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+  arg1 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
   gfc_add_block_to_block (&block, &se.post);

-  /* Convert IMAGE_DISTINCT to a LOGICAL(4) entity.  */
+  /* Convert IMAGE_DISTINCT to the desired LOGICAL entity.  */
   gfc_conv_expr (&se, code->ext.actual->next->expr);
   gfc_add_block_to_block (&block, &se.pre);
-  arg2 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+  arg2 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
   gfc_add_block_to_block (&block, &se.post);

-  /* Create the hidden argument.  For non-coarray codes and -fcoarray=single,
-     simply set this to 0.  For -fcoarray=lib, generate a call to
-     THIS_IMAGE() without arguments.  */
   if (flag_coarray == GFC_FCOARRAY_LIB)
     {
       tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_random_init,
diff --git a/libgfortran/caf/libcaf.h b/libgfortran/caf/libcaf.h
index 5abb753f6fd..c66d0379042 100644
--- a/libgfortran/caf/libcaf.h
+++ b/libgfortran/caf/libcaf.h
@@ -261,4 +261,6 @@ void _gfortran_caf_stopped_images (gfc_descriptor_t *,

 int _gfortran_caf_is_present (caf_token_t, int, caf_reference_t *);

+void _gfortran_caf_random_init (bool, bool);
+
 #endif  /* LIBCAF_H  */
diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c
index a291c4452c9..fc8e3b3b94a 100644
--- a/libgfortran/caf/single.c
+++ b/libgfortran/caf/single.c
@@ -3135,3 +3135,13 @@ _gfortran_caf_is_present (caf_token_t token,
     }
   return memptr != NULL;
 }
+
+/* Reference the libraries implementation.  */
+extern void _gfortran_random_init (int32_t, int32_t, int32_t);
+
+void _gfortran_caf_random_init (bool repeatable, bool image_distinct)
+{
+  /* In a single image implementation always forward to the gfortran
+     routine.  */
+  _gfortran_random_init (repeatable, image_distinct, 1);
+}
diff --git a/libgfortran/gfortran.map b/libgfortran/gfortran.map
index b594b59849a..626804c37dd 100644
--- a/libgfortran/gfortran.map
+++ b/libgfortran/gfortran.map
@@ -1634,3 +1634,8 @@ GFORTRAN_11 {
   global:
   _gfortran_random_init_foobar;
 } GFORTRAN_10.2;
+
+GFORTRAN_12 {
+  global:
+  _gfortran_caf_random_init;
+} GFORTRAN_11;
diff --git a/libgfortran/intrinsics/random_init.f90 b/libgfortran/intrinsics/random_init.f90
index bbb45937087..233a1ca073a 100644
--- a/libgfortran/intrinsics/random_init.f90
+++ b/libgfortran/intrinsics/random_init.f90
@@ -100,7 +100,7 @@ impure subroutine _gfortran_random_init(repeatable, image_distinct, image_num)
 end subroutine _gfortran_random_init
 !
 ! This is a temporary stub implementation until random_init is
-! implemented for -fcoarray=shared and -fcoarray=lib.
+! implemented for -fcoarray=shared.
 !
 subroutine _gfortran_random_init_foobar(repeatable, image_distinct)