From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from troutmask.apl.washington.edu (troutmask.apl.washington.edu [128.95.76.21]) by sourceware.org (Postfix) with ESMTPS id 7C94C3857805 for ; Fri, 23 Apr 2021 17:18:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7C94C3857805 Received: from troutmask.apl.washington.edu (localhost [127.0.0.1]) by troutmask.apl.washington.edu (8.16.1/8.16.1) with ESMTPS id 13NHIHGv075090 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Fri, 23 Apr 2021 10:18:17 -0700 (PDT) (envelope-from sgk@troutmask.apl.washington.edu) Received: (from sgk@localhost) by troutmask.apl.washington.edu (8.16.1/8.16.1/Submit) id 13NHIHCZ075089; Fri, 23 Apr 2021 10:18:17 -0700 (PDT) (envelope-from sgk) Date: Fri, 23 Apr 2021 10:18:17 -0700 From: Steve Kargl To: Andre Vehreschild Cc: GCC-Fortran-ML , Damian Rouson , Andre Vehreschild Subject: Re: [Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran Message-ID: <20210423171817.GA75072@troutmask.apl.washington.edu> References: <20210403172846.GA14134@troutmask.apl.washington.edu> <20210404053331.GA18048@troutmask.apl.washington.edu> <20210423184357.643efbdf@vepi2> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210423184357.643efbdf@vepi2> X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Apr 2021 17:18:29 -0000 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 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