[Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran
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 func
Re: [Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran
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. > > > >
