Andre,
Thanks for taking care of OpenCoarray portion of RANDOM_INIT.
My last non-coarray aware patch is attached to the PR in bugzilla.
Since the change over to git, I no longer commit to the source tree.
I suggest combining your patch with my patch if you intend to
commit; otherwise, attach your patch to the PR and sit patiently
until someone can do the combined commit.
--
steve
On Fri, Apr 23, 2021 at 06:43:57PM +0200, Andre Vehreschild wrote:
> 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
> 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)
>
--
Steve
