Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
* PING * 2016-09-21 19:03 GMT+01:00 Alessandro Fanfarillo : > Thanks Andre. > > 2016-09-19 9:55 GMT-06:00 Andre Vehreschild : >> Hi Alessandro, > >> The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is >> doing nothing. So do you plan to add more code, or will there never be >> anything. If the later I recommend to just put a comment there and remove the >> empty if. > > I added the if statement during the development and I forgot to remove it. > >> >> There still is no test when -fcoarray=single is used. This shouldn't be so >> hard, should it? > > Done. > > Built and regtested on x86_64-pc-linux-gnu. > >> >> Regards, >> Andre >> >> On Mon, 19 Sep 2016 08:30:12 -0700 >> Alessandro Fanfarillo wrote: >> >>> * PING * >>> >>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" >>> wrote: >>> >>> > Dear all, >>> > the attached patch supports failed images also when -fcoarray=single is >>> > used. >>> > >>> > Built and regtested on x86_64-pc-linux-gnu. >>> > >>> > Cheers, >>> > Alessandro >>> > >>> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas < >>> > paul.richard.tho...@gmail.com>: >>> > > Hi Sandro, >>> > > >>> > > As far as I can see, this is OK barring a couple of minor wrinkles and >>> > > a question: >>> > > >>> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you >>> > > have used the option -fdump-tree-original without making use of the >>> > > tree dump. >>> > > >>> > > Mikael asked you to provide an executable test with -fcoarray=single. >>> > > Is this not possible for some reason? >>> > > >>> > > Otherwise, this is OK for trunk. >>> > > >>> > > Thanks for the patch. >>> > > >>> > > Paul >>> > > >>> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo >>> > > wrote: >>> > >> * PING * >>> > >> >>> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo < >>> > fanfarillo@gmail.com>: >>> > >>> Dear Mikael and all, >>> > >>> >>> > >>> in attachment the new patch, built and regtested on >>> > x86_64-pc-linux-gnu. >>> > >>> >>> > >>> Cheers, >>> > >>> Alessandro >>> > >>> >>> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin : >>> > Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : >>> > > >>> > > Hi Mikael, >>> > > >>> > > >>> > >>> + if(st == ST_FAIL_IMAGE) >>> > >>> +new_st.op = EXEC_FAIL_IMAGE; >>> > >>> + else >>> > >>> +gcc_unreachable(); >>> > >> >>> > >> You can use >>> > >> gcc_assert (st == ST_FAIL_IMAGE); >>> > >> foo...; >>> > >> instead of >>> > >> if (st == ST_FAIL_IMAGE) >>> > >> foo...; >>> > >> else >>> > >> gcc_unreachable (); >>> > > >>> > > >>> > > Be careful, this is not 100% identical in the general case. For >>> > > older >>> > > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not >>> > to >>> > > an abort(), so the behavior can change. But in this case everything >>> > is >>> > > fine, because the patch is most likely not backported. >>> > > >>> > Didn't know about this. The difference seems to be very subtle. >>> > I don't mind much anyway. The original version can stay if preferred, >>> > this >>> > was just a suggestion. >>> > >>> > By the way, if the function is inlined in its single caller, the >>> > assert or >>> > unreachable statement can be removed, which avoids choosing between >>> > them. >>> > That's another suggestion. >>> > >>> > >>> > >>> + >>> > >>> + return MATCH_YES; >>> > >>> + >>> > >>> + syntax: >>> > >>> + gfc_syntax_error (st); >>> > >>> + >>> > >>> + return MATCH_ERROR; >>> > >>> +} >>> > >>> + >>> > >>> +match >>> > >>> +gfc_match_fail_image (void) >>> > >>> +{ >>> > >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement >>> > >>> at %C")) */ >>> > >>> + /* return MATCH_ERROR; */ >>> > >>> + >>> > >> >>> > >> Can this be uncommented? >>> > >> >>> > >>> + return fail_image_statement (ST_FAIL_IMAGE); >>> > >>> +} >>> > >>> >>> > >>> /* Match LOCK/UNLOCK statement. Syntax: >>> > >>> LOCK ( lock-variable [ , lock-stat-list ] ) >>> > >>> diff --git a/gcc/fortran/trans-intrinsic.c >>> > >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 >>> > >>> --- a/gcc/fortran/trans-intrinsic.c >>> > >>> +++ b/gcc/fortran/trans-intrinsic.c >>> > >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr >>> > >>> *expr) m, lbound)); >>> > >>> } >>> > >>> >>> > >>> +static void >>> > >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) >>> > >>> +{ >>> > >>> + unsigned int num_args; >>> > >>> + tree *args,tmp; >>> > >>> + >>> > >>> + num_args = gfc_intrinsic_argument_list_length (expr); >>> > >>> + args = XALLOCAVEC (tree, num_args); >>> > >>> + >>> > >>>
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Thanks Andre. 2016-09-19 9:55 GMT-06:00 Andre Vehreschild : > Hi Alessandro, > The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is > doing nothing. So do you plan to add more code, or will there never be > anything. If the later I recommend to just put a comment there and remove the > empty if. I added the if statement during the development and I forgot to remove it. > > There still is no test when -fcoarray=single is used. This shouldn't be so > hard, should it? Done. Built and regtested on x86_64-pc-linux-gnu. > > Regards, > Andre > > On Mon, 19 Sep 2016 08:30:12 -0700 > Alessandro Fanfarillo wrote: > >> * PING * >> >> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" >> wrote: >> >> > Dear all, >> > the attached patch supports failed images also when -fcoarray=single is >> > used. >> > >> > Built and regtested on x86_64-pc-linux-gnu. >> > >> > Cheers, >> > Alessandro >> > >> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas < >> > paul.richard.tho...@gmail.com>: >> > > Hi Sandro, >> > > >> > > As far as I can see, this is OK barring a couple of minor wrinkles and >> > > a question: >> > > >> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you >> > > have used the option -fdump-tree-original without making use of the >> > > tree dump. >> > > >> > > Mikael asked you to provide an executable test with -fcoarray=single. >> > > Is this not possible for some reason? >> > > >> > > Otherwise, this is OK for trunk. >> > > >> > > Thanks for the patch. >> > > >> > > Paul >> > > >> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo >> > > wrote: >> > >> * PING * >> > >> >> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo < >> > fanfarillo@gmail.com>: >> > >>> Dear Mikael and all, >> > >>> >> > >>> in attachment the new patch, built and regtested on >> > x86_64-pc-linux-gnu. >> > >>> >> > >>> Cheers, >> > >>> Alessandro >> > >>> >> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin : >> > Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : >> > > >> > > Hi Mikael, >> > > >> > > >> > >>> + if(st == ST_FAIL_IMAGE) >> > >>> +new_st.op = EXEC_FAIL_IMAGE; >> > >>> + else >> > >>> +gcc_unreachable(); >> > >> >> > >> You can use >> > >> gcc_assert (st == ST_FAIL_IMAGE); >> > >> foo...; >> > >> instead of >> > >> if (st == ST_FAIL_IMAGE) >> > >> foo...; >> > >> else >> > >> gcc_unreachable (); >> > > >> > > >> > > Be careful, this is not 100% identical in the general case. For older >> > > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not >> > to >> > > an abort(), so the behavior can change. But in this case everything >> > is >> > > fine, because the patch is most likely not backported. >> > > >> > Didn't know about this. The difference seems to be very subtle. >> > I don't mind much anyway. The original version can stay if preferred, >> > this >> > was just a suggestion. >> > >> > By the way, if the function is inlined in its single caller, the >> > assert or >> > unreachable statement can be removed, which avoids choosing between >> > them. >> > That's another suggestion. >> > >> > >> > >>> + >> > >>> + return MATCH_YES; >> > >>> + >> > >>> + syntax: >> > >>> + gfc_syntax_error (st); >> > >>> + >> > >>> + return MATCH_ERROR; >> > >>> +} >> > >>> + >> > >>> +match >> > >>> +gfc_match_fail_image (void) >> > >>> +{ >> > >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement >> > >>> at %C")) */ >> > >>> + /* return MATCH_ERROR; */ >> > >>> + >> > >> >> > >> Can this be uncommented? >> > >> >> > >>> + return fail_image_statement (ST_FAIL_IMAGE); >> > >>> +} >> > >>> >> > >>> /* Match LOCK/UNLOCK statement. Syntax: >> > >>> LOCK ( lock-variable [ , lock-stat-list ] ) >> > >>> diff --git a/gcc/fortran/trans-intrinsic.c >> > >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 >> > >>> --- a/gcc/fortran/trans-intrinsic.c >> > >>> +++ b/gcc/fortran/trans-intrinsic.c >> > >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr >> > >>> *expr) m, lbound)); >> > >>> } >> > >>> >> > >>> +static void >> > >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) >> > >>> +{ >> > >>> + unsigned int num_args; >> > >>> + tree *args,tmp; >> > >>> + >> > >>> + num_args = gfc_intrinsic_argument_list_length (expr); >> > >>> + args = XALLOCAVEC (tree, num_args); >> > >>> + >> > >>> + gfc_conv_intrinsic_function_args (se, expr, args, num_args); >> > >>> + >> > >>> + if (flag_coarray == GFC_FCOARRAY_LIB) >> > >>> +{ >> > >> >> > >> Can everything be put under the if? >> > >> Does i
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Hi Alessandro, there are still some violations of the style guide: contrib/check_GNU_style.sh first_complete_patch_REV2.diff emits: Lines should not exceed 80 characters. 154:+ add_sym_2 ("failed_images", GFC_ISYM_FAILED_IMAGES, CLASS_TRANSFORMATIONAL, ACTUAL_NO, BT_INTEGER, 155:+ dd, GFC_STD_F2008_TS, gfc_check_failed_images, gfc_simplify_failed_images, 156:+ gfc_resolve_failed_images, "team", BT_INTEGER, di, OPTIONAL, "kind", BT_INTEGER, di, OPTIONAL); 165:+ add_sym_2 ("image_status", GFC_ISYM_IMAGE_STATUS, CLASS_ELEMENTAL, ACTUAL_NO, BT_INTEGER, 166:+ di, GFC_STD_F2008_TS, gfc_check_image_status, gfc_simplify_image_status, 167:+ gfc_resolve_image_status, "image", BT_INTEGER, di, REQUIRED, "team", BT_INTEGER, di, OPTIONAL); 247:+gfc_resolve_image_status (gfc_expr *f, gfc_expr *image, gfc_expr *team ATTRIBUTE_UNUSED) 409:+ result = transformational_result (result, 0, BT_INTEGER, kind->ts.kind, &gfc_current_locus); 420:+gfc_simplify_image_status(gfc_expr *image ATTRIBUTE_UNUSED, gfc_expr *team ATTRIBUTE_UNUSED) 469:+ gfor_fndecl_caf_failed_images = gfc_build_library_function_decl_with_spec ( 471:+pvoid_type_node, 3, pvoid_type_node, integer_type_node, integer_type_node); The remainder of the script output needs no fix, because its in Fortran code. You should fix the above, where they are not in a Fortran testcases. This allows people with 80 column terminals to read the whole line without scrolling. The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is doing nothing. So do you plan to add more code, or will there never be anything. If the later I recommend to just put a comment there and remove the empty if. There still is no test when -fcoarray=single is used. This shouldn't be so hard, should it? Regards, Andre On Mon, 19 Sep 2016 08:30:12 -0700 Alessandro Fanfarillo wrote: > * PING * > > On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" > wrote: > > > Dear all, > > the attached patch supports failed images also when -fcoarray=single is > > used. > > > > Built and regtested on x86_64-pc-linux-gnu. > > > > Cheers, > > Alessandro > > > > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas < > > paul.richard.tho...@gmail.com>: > > > Hi Sandro, > > > > > > As far as I can see, this is OK barring a couple of minor wrinkles and > > > a question: > > > > > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you > > > have used the option -fdump-tree-original without making use of the > > > tree dump. > > > > > > Mikael asked you to provide an executable test with -fcoarray=single. > > > Is this not possible for some reason? > > > > > > Otherwise, this is OK for trunk. > > > > > > Thanks for the patch. > > > > > > Paul > > > > > > On 4 August 2016 at 05:07, Alessandro Fanfarillo > > > wrote: > > >> * PING * > > >> > > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo < > > fanfarillo@gmail.com>: > > >>> Dear Mikael and all, > > >>> > > >>> in attachment the new patch, built and regtested on > > x86_64-pc-linux-gnu. > > >>> > > >>> Cheers, > > >>> Alessandro > > >>> > > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin : > > Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : > > > > > > Hi Mikael, > > > > > > > > >>> + if(st == ST_FAIL_IMAGE) > > >>> +new_st.op = EXEC_FAIL_IMAGE; > > >>> + else > > >>> +gcc_unreachable(); > > >> > > >> You can use > > >> gcc_assert (st == ST_FAIL_IMAGE); > > >> foo...; > > >> instead of > > >> if (st == ST_FAIL_IMAGE) > > >> foo...; > > >> else > > >> gcc_unreachable (); > > > > > > > > > Be careful, this is not 100% identical in the general case. For older > > > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not > > to > > > an abort(), so the behavior can change. But in this case everything > > is > > > fine, because the patch is most likely not backported. > > > > > Didn't know about this. The difference seems to be very subtle. > > I don't mind much anyway. The original version can stay if preferred, > > this > > was just a suggestion. > > > > By the way, if the function is inlined in its single caller, the > > assert or > > unreachable statement can be removed, which avoids choosing between > > them. > > That's another suggestion. > > > > > > >>> + > > >>> + return MATCH_YES; > > >>> + > > >>> + syntax: > > >>> + gfc_syntax_error (st); > > >>> + > > >>> + return MATCH_ERROR; > > >>> +} > > >>> + > > >>> +match > > >>> +gfc_match_fail_image (void) > > >>> +{ > > >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement > > >>> at %C")) */ > > >>> + /* return MATCH_ERROR; */ > > >>> + > > >> > > >> Can this be uncommented? > > >>
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Dear all, the attached patch supports failed images also when -fcoarray=single is used. Built and regtested on x86_64-pc-linux-gnu. Cheers, Alessandro 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas : > Hi Sandro, > > As far as I can see, this is OK barring a couple of minor wrinkles and > a question: > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you > have used the option -fdump-tree-original without making use of the > tree dump. > > Mikael asked you to provide an executable test with -fcoarray=single. > Is this not possible for some reason? > > Otherwise, this is OK for trunk. > > Thanks for the patch. > > Paul > > On 4 August 2016 at 05:07, Alessandro Fanfarillo > wrote: >> * PING * >> >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo : >>> Dear Mikael and all, >>> >>> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu. >>> >>> Cheers, >>> Alessandro >>> >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin : Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : > > Hi Mikael, > > >>> + if(st == ST_FAIL_IMAGE) >>> +new_st.op = EXEC_FAIL_IMAGE; >>> + else >>> +gcc_unreachable(); >> >> You can use >> gcc_assert (st == ST_FAIL_IMAGE); >> foo...; >> instead of >> if (st == ST_FAIL_IMAGE) >> foo...; >> else >> gcc_unreachable (); > > > Be careful, this is not 100% identical in the general case. For older > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to > an abort(), so the behavior can change. But in this case everything is > fine, because the patch is most likely not backported. > Didn't know about this. The difference seems to be very subtle. I don't mind much anyway. The original version can stay if preferred, this was just a suggestion. By the way, if the function is inlined in its single caller, the assert or unreachable statement can be removed, which avoids choosing between them. That's another suggestion. >>> + >>> + return MATCH_YES; >>> + >>> + syntax: >>> + gfc_syntax_error (st); >>> + >>> + return MATCH_ERROR; >>> +} >>> + >>> +match >>> +gfc_match_fail_image (void) >>> +{ >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement >>> at %C")) */ >>> + /* return MATCH_ERROR; */ >>> + >> >> Can this be uncommented? >> >>> + return fail_image_statement (ST_FAIL_IMAGE); >>> +} >>> >>> /* Match LOCK/UNLOCK statement. Syntax: >>> LOCK ( lock-variable [ , lock-stat-list ] ) >>> diff --git a/gcc/fortran/trans-intrinsic.c >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 >>> --- a/gcc/fortran/trans-intrinsic.c >>> +++ b/gcc/fortran/trans-intrinsic.c >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr >>> *expr) m, lbound)); >>> } >>> >>> +static void >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) >>> +{ >>> + unsigned int num_args; >>> + tree *args,tmp; >>> + >>> + num_args = gfc_intrinsic_argument_list_length (expr); >>> + args = XALLOCAVEC (tree, num_args); >>> + >>> + gfc_conv_intrinsic_function_args (se, expr, args, num_args); >>> + >>> + if (flag_coarray == GFC_FCOARRAY_LIB) >>> +{ >> >> Can everything be put under the if? >> Does it work with -fcoarray=single? > > > IMO coarray=single should not generate code here, therefore putting > everything under the if should to fine. > My point was more avoiding generating code for the arguments if they are not used in the end. Regarding the -fcoarray=single case, the function returns a result, which can be used in an expression, so I don't think it will work without at least hardcoding a fixed value as result in that case. But even that wouldn't be enough, as the function wouldn't work consistently with the fail image statement. > Sorry for the comments ... > Comments are welcome here, as far as I know. ;-) Mikael > > > > -- > The difference between genius and stupidity is; genius has its limits. > > Albert Einstein commit 13213642603b4941a2e4ea085b0bfd5cb37f Author: Alessandro Fanfarillo Date: Wed Sep 7 13:00:17 2016 -0600 Second Review of failed image patch diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index ff5e80b..110bec0 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -1217,6 +1217,82 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *count, gfc_expr *stat) return true; } +bool +gfc_check_image_status (gfc_expr *image, gfc_expr *team) +{ + if (!type_check (image, 1, BT_INTEGER)) +return false; + + int i = gfc_validate_kind (BT_INTEGER, image->ts.kind,
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Thanks Paul, I fixed the unused -fdump-tree-original on the tests. About -fcoarray=single, I agree with Andre about not producing code for failed images functions when running in single-image mode. If you, or anybody else, thing otherwise I can adjust the functions to return a constant value (except for fail image... :)). 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas : > Hi Sandro, > > As far as I can see, this is OK barring a couple of minor wrinkles and > a question: > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you > have used the option -fdump-tree-original without making use of the > tree dump. > > Mikael asked you to provide an executable test with -fcoarray=single. > Is this not possible for some reason? > > Otherwise, this is OK for trunk. > > Thanks for the patch. > > Paul > > On 4 August 2016 at 05:07, Alessandro Fanfarillo > wrote: >> * PING * >> >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo : >>> Dear Mikael and all, >>> >>> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu. >>> >>> Cheers, >>> Alessandro >>> >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin : Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : > > Hi Mikael, > > >>> + if(st == ST_FAIL_IMAGE) >>> +new_st.op = EXEC_FAIL_IMAGE; >>> + else >>> +gcc_unreachable(); >> >> You can use >> gcc_assert (st == ST_FAIL_IMAGE); >> foo...; >> instead of >> if (st == ST_FAIL_IMAGE) >> foo...; >> else >> gcc_unreachable (); > > > Be careful, this is not 100% identical in the general case. For older > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to > an abort(), so the behavior can change. But in this case everything is > fine, because the patch is most likely not backported. > Didn't know about this. The difference seems to be very subtle. I don't mind much anyway. The original version can stay if preferred, this was just a suggestion. By the way, if the function is inlined in its single caller, the assert or unreachable statement can be removed, which avoids choosing between them. That's another suggestion. >>> + >>> + return MATCH_YES; >>> + >>> + syntax: >>> + gfc_syntax_error (st); >>> + >>> + return MATCH_ERROR; >>> +} >>> + >>> +match >>> +gfc_match_fail_image (void) >>> +{ >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement >>> at %C")) */ >>> + /* return MATCH_ERROR; */ >>> + >> >> Can this be uncommented? >> >>> + return fail_image_statement (ST_FAIL_IMAGE); >>> +} >>> >>> /* Match LOCK/UNLOCK statement. Syntax: >>> LOCK ( lock-variable [ , lock-stat-list ] ) >>> diff --git a/gcc/fortran/trans-intrinsic.c >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 >>> --- a/gcc/fortran/trans-intrinsic.c >>> +++ b/gcc/fortran/trans-intrinsic.c >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr >>> *expr) m, lbound)); >>> } >>> >>> +static void >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) >>> +{ >>> + unsigned int num_args; >>> + tree *args,tmp; >>> + >>> + num_args = gfc_intrinsic_argument_list_length (expr); >>> + args = XALLOCAVEC (tree, num_args); >>> + >>> + gfc_conv_intrinsic_function_args (se, expr, args, num_args); >>> + >>> + if (flag_coarray == GFC_FCOARRAY_LIB) >>> +{ >> >> Can everything be put under the if? >> Does it work with -fcoarray=single? > > > IMO coarray=single should not generate code here, therefore putting > everything under the if should to fine. > My point was more avoiding generating code for the arguments if they are not used in the end. Regarding the -fcoarray=single case, the function returns a result, which can be used in an expression, so I don't think it will work without at least hardcoding a fixed value as result in that case. But even that wouldn't be enough, as the function wouldn't work consistently with the fail image statement. > Sorry for the comments ... > Comments are welcome here, as far as I know. ;-) Mikael > > > > -- > The difference between genius and stupidity is; genius has its limits. > > Albert Einstein
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Hi Sandro, As far as I can see, this is OK barring a couple of minor wrinkles and a question: For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you have used the option -fdump-tree-original without making use of the tree dump. Mikael asked you to provide an executable test with -fcoarray=single. Is this not possible for some reason? Otherwise, this is OK for trunk. Thanks for the patch. Paul On 4 August 2016 at 05:07, Alessandro Fanfarillo wrote: > * PING * > > 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo : >> Dear Mikael and all, >> >> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu. >> >> Cheers, >> Alessandro >> >> 2016-07-20 13:17 GMT-06:00 Mikael Morin : >>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : Hi Mikael, >> + if(st == ST_FAIL_IMAGE) >> +new_st.op = EXEC_FAIL_IMAGE; >> + else >> +gcc_unreachable(); > > You can use > gcc_assert (st == ST_FAIL_IMAGE); > foo...; > instead of > if (st == ST_FAIL_IMAGE) > foo...; > else > gcc_unreachable (); Be careful, this is not 100% identical in the general case. For older gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to an abort(), so the behavior can change. But in this case everything is fine, because the patch is most likely not backported. >>> Didn't know about this. The difference seems to be very subtle. >>> I don't mind much anyway. The original version can stay if preferred, this >>> was just a suggestion. >>> >>> By the way, if the function is inlined in its single caller, the assert or >>> unreachable statement can be removed, which avoids choosing between them. >>> That's another suggestion. >>> >>> >> + >> + return MATCH_YES; >> + >> + syntax: >> + gfc_syntax_error (st); >> + >> + return MATCH_ERROR; >> +} >> + >> +match >> +gfc_match_fail_image (void) >> +{ >> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement >> at %C")) */ >> + /* return MATCH_ERROR; */ >> + > > Can this be uncommented? > >> + return fail_image_statement (ST_FAIL_IMAGE); >> +} >> >> /* Match LOCK/UNLOCK statement. Syntax: >> LOCK ( lock-variable [ , lock-stat-list ] ) >> diff --git a/gcc/fortran/trans-intrinsic.c >> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 >> --- a/gcc/fortran/trans-intrinsic.c >> +++ b/gcc/fortran/trans-intrinsic.c >> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr >> *expr) m, lbound)); >> } >> >> +static void >> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) >> +{ >> + unsigned int num_args; >> + tree *args,tmp; >> + >> + num_args = gfc_intrinsic_argument_list_length (expr); >> + args = XALLOCAVEC (tree, num_args); >> + >> + gfc_conv_intrinsic_function_args (se, expr, args, num_args); >> + >> + if (flag_coarray == GFC_FCOARRAY_LIB) >> +{ > > Can everything be put under the if? > Does it work with -fcoarray=single? IMO coarray=single should not generate code here, therefore putting everything under the if should to fine. >>> My point was more avoiding generating code for the arguments if they are not >>> used in the end. >>> Regarding the -fcoarray=single case, the function returns a result, which >>> can be used in an expression, so I don't think it will work without at least >>> hardcoding a fixed value as result in that case. >>> But even that wouldn't be enough, as the function wouldn't work consistently >>> with the fail image statement. >>> Sorry for the comments ... >>> Comments are welcome here, as far as I know. ;-) >>> >>> Mikael -- The difference between genius and stupidity is; genius has its limits. Albert Einstein
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Hi, The failed images features of gfortran are exciting, and folks here would like to start testing with their scientific codes. I’d like to build a new gfortran to support them, but I must build from a trusted source, which means from trunk, without custom patches. Can Alessandro’s patch get a review? Many thanks! > On Jun 21, 2016, at 10:59, Alessandro Fanfarillo > wrote: > > * PING * > > 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : >> Dear all, >> >> please find in attachment the first patch (of n) for the FAILED IMAGES >> capability defined in the coarray TS 18508. >> The patch adds support for three new intrinsic functions defined in >> the TS for simulating a failure (fail image), checking an image status >> (image_status) and getting the list of failed images (failed_images). >> The patch has been built and regtested on x86_64-pc-linux-gnu. >> >> Ok for trunk? >> >> Alessandro -- Cheers! Dan Nagle
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
* PING * 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo : > Dear Mikael and all, > > in attachment the new patch, built and regtested on x86_64-pc-linux-gnu. > > Cheers, > Alessandro > > 2016-07-20 13:17 GMT-06:00 Mikael Morin : >> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : >>> >>> Hi Mikael, >>> >>> > + if(st == ST_FAIL_IMAGE) > +new_st.op = EXEC_FAIL_IMAGE; > + else > +gcc_unreachable(); You can use gcc_assert (st == ST_FAIL_IMAGE); foo...; instead of if (st == ST_FAIL_IMAGE) foo...; else gcc_unreachable (); >>> >>> >>> Be careful, this is not 100% identical in the general case. For older >>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to >>> an abort(), so the behavior can change. But in this case everything is >>> fine, because the patch is most likely not backported. >>> >> Didn't know about this. The difference seems to be very subtle. >> I don't mind much anyway. The original version can stay if preferred, this >> was just a suggestion. >> >> By the way, if the function is inlined in its single caller, the assert or >> unreachable statement can be removed, which avoids choosing between them. >> That's another suggestion. >> >> > + > + return MATCH_YES; > + > + syntax: > + gfc_syntax_error (st); > + > + return MATCH_ERROR; > +} > + > +match > +gfc_match_fail_image (void) > +{ > + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement > at %C")) */ > + /* return MATCH_ERROR; */ > + Can this be uncommented? > + return fail_image_statement (ST_FAIL_IMAGE); > +} > > /* Match LOCK/UNLOCK statement. Syntax: > LOCK ( lock-variable [ , lock-stat-list ] ) > diff --git a/gcc/fortran/trans-intrinsic.c > b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 > --- a/gcc/fortran/trans-intrinsic.c > +++ b/gcc/fortran/trans-intrinsic.c > @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr > *expr) m, lbound)); > } > > +static void > +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) > +{ > + unsigned int num_args; > + tree *args,tmp; > + > + num_args = gfc_intrinsic_argument_list_length (expr); > + args = XALLOCAVEC (tree, num_args); > + > + gfc_conv_intrinsic_function_args (se, expr, args, num_args); > + > + if (flag_coarray == GFC_FCOARRAY_LIB) > +{ Can everything be put under the if? Does it work with -fcoarray=single? >>> >>> >>> IMO coarray=single should not generate code here, therefore putting >>> everything under the if should to fine. >>> >> My point was more avoiding generating code for the arguments if they are not >> used in the end. >> Regarding the -fcoarray=single case, the function returns a result, which >> can be used in an expression, so I don't think it will work without at least >> hardcoding a fixed value as result in that case. >> But even that wouldn't be enough, as the function wouldn't work consistently >> with the fail image statement. >> >>> Sorry for the comments ... >>> >> Comments are welcome here, as far as I know. ;-) >> >> Mikael
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Dear Mikael and all, in attachment the new patch, built and regtested on x86_64-pc-linux-gnu. Cheers, Alessandro 2016-07-20 13:17 GMT-06:00 Mikael Morin : > Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : >> >> Hi Mikael, >> >> + if(st == ST_FAIL_IMAGE) +new_st.op = EXEC_FAIL_IMAGE; + else +gcc_unreachable(); >>> >>> You can use >>> gcc_assert (st == ST_FAIL_IMAGE); >>> foo...; >>> instead of >>> if (st == ST_FAIL_IMAGE) >>> foo...; >>> else >>> gcc_unreachable (); >> >> >> Be careful, this is not 100% identical in the general case. For older >> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to >> an abort(), so the behavior can change. But in this case everything is >> fine, because the patch is most likely not backported. >> > Didn't know about this. The difference seems to be very subtle. > I don't mind much anyway. The original version can stay if preferred, this > was just a suggestion. > > By the way, if the function is inlined in its single caller, the assert or > unreachable statement can be removed, which avoids choosing between them. > That's another suggestion. > > + + return MATCH_YES; + + syntax: + gfc_syntax_error (st); + + return MATCH_ERROR; +} + +match +gfc_match_fail_image (void) +{ + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement at %C")) */ + /* return MATCH_ERROR; */ + >>> >>> Can this be uncommented? >>> + return fail_image_statement (ST_FAIL_IMAGE); +} /* Match LOCK/UNLOCK statement. Syntax: LOCK ( lock-variable [ , lock-stat-list ] ) diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr *expr) m, lbound)); } +static void +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) +{ + unsigned int num_args; + tree *args,tmp; + + num_args = gfc_intrinsic_argument_list_length (expr); + args = XALLOCAVEC (tree, num_args); + + gfc_conv_intrinsic_function_args (se, expr, args, num_args); + + if (flag_coarray == GFC_FCOARRAY_LIB) +{ >>> >>> Can everything be put under the if? >>> Does it work with -fcoarray=single? >> >> >> IMO coarray=single should not generate code here, therefore putting >> everything under the if should to fine. >> > My point was more avoiding generating code for the arguments if they are not > used in the end. > Regarding the -fcoarray=single case, the function returns a result, which > can be used in an expression, so I don't think it will work without at least > hardcoding a fixed value as result in that case. > But even that wouldn't be enough, as the function wouldn't work consistently > with the fail image statement. > >> Sorry for the comments ... >> > Comments are welcome here, as far as I know. ;-) > > Mikael commit d6c91b2c14a12d1d012738f13f4920e207113982 Author: Alessandro Fanfarillo Date: Thu Jul 21 10:01:33 2016 -0600 First review of failed images patch diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index d26e45e..121551c 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -1210,6 +1210,97 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *count, gfc_expr *stat) return true; } +bool +gfc_check_image_status (gfc_expr *image, gfc_expr *team) +{ + + if (flag_coarray != GFC_FCOARRAY_LIB) +{ + gfc_fatal_error ("Failed images features " + "usable only with %<-fcoarray=lib%>"); + return false; +} + + if (!type_check (image, 1, BT_INTEGER)) +return false; + + int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false); + int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false); + + if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range) +{ + gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L " +"shall have at least the range of the default integer", +&image->where); + return false; +} + + j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false); + + if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range) +{ + gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L " +"shall have at most the range of the double precision integer", +&image->where); + return false; +} + + if (team) +{ + gfc_error ("TEAM argument of the IMAGE_STATUS intrinsic function at %L " +"not yet supported", +&team->where); + return false; +} + + return true; +} + +bool +gfc_check_failed_images (gfc_expr *team, gfc_expr *kind) +{ +
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Le 20/07/2016 à 11:39, Andre Vehreschild a écrit : Hi Mikael, + if(st == ST_FAIL_IMAGE) +new_st.op = EXEC_FAIL_IMAGE; + else +gcc_unreachable(); You can use gcc_assert (st == ST_FAIL_IMAGE); foo...; instead of if (st == ST_FAIL_IMAGE) foo...; else gcc_unreachable (); Be careful, this is not 100% identical in the general case. For older gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to an abort(), so the behavior can change. But in this case everything is fine, because the patch is most likely not backported. Didn't know about this. The difference seems to be very subtle. I don't mind much anyway. The original version can stay if preferred, this was just a suggestion. By the way, if the function is inlined in its single caller, the assert or unreachable statement can be removed, which avoids choosing between them. That's another suggestion. + + return MATCH_YES; + + syntax: + gfc_syntax_error (st); + + return MATCH_ERROR; +} + +match +gfc_match_fail_image (void) +{ + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement at %C")) */ + /* return MATCH_ERROR; */ + Can this be uncommented? + return fail_image_statement (ST_FAIL_IMAGE); +} /* Match LOCK/UNLOCK statement. Syntax: LOCK ( lock-variable [ , lock-stat-list ] ) diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr *expr) m, lbound)); } +static void +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) +{ + unsigned int num_args; + tree *args,tmp; + + num_args = gfc_intrinsic_argument_list_length (expr); + args = XALLOCAVEC (tree, num_args); + + gfc_conv_intrinsic_function_args (se, expr, args, num_args); + + if (flag_coarray == GFC_FCOARRAY_LIB) +{ Can everything be put under the if? Does it work with -fcoarray=single? IMO coarray=single should not generate code here, therefore putting everything under the if should to fine. My point was more avoiding generating code for the arguments if they are not used in the end. Regarding the -fcoarray=single case, the function returns a result, which can be used in an expression, so I don't think it will work without at least hardcoding a fixed value as result in that case. But even that wouldn't be enough, as the function wouldn't work consistently with the fail image statement. Sorry for the comments ... Comments are welcome here, as far as I know. ;-) Mikael
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Hi Mikael, > > + if(st == ST_FAIL_IMAGE) > > +new_st.op = EXEC_FAIL_IMAGE; > > + else > > +gcc_unreachable(); > You can use > gcc_assert (st == ST_FAIL_IMAGE); > foo...; > instead of > if (st == ST_FAIL_IMAGE) > foo...; > else > gcc_unreachable (); Be careful, this is not 100% identical in the general case. For older gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to an abort(), so the behavior can change. But in this case everything is fine, because the patch is most likely not backported. > > + > > + return MATCH_YES; > > + > > + syntax: > > + gfc_syntax_error (st); > > + > > + return MATCH_ERROR; > > +} > > + > > +match > > +gfc_match_fail_image (void) > > +{ > > + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement > > at %C")) */ > > + /* return MATCH_ERROR; */ > > + > Can this be uncommented? > > > + return fail_image_statement (ST_FAIL_IMAGE); > > +} > > > > /* Match LOCK/UNLOCK statement. Syntax: > > LOCK ( lock-variable [ , lock-stat-list ] ) > > diff --git a/gcc/fortran/trans-intrinsic.c > > b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 > > --- a/gcc/fortran/trans-intrinsic.c > > +++ b/gcc/fortran/trans-intrinsic.c > > @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr > > *expr) m, lbound)); > > } > > > > +static void > > +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) > > +{ > > + unsigned int num_args; > > + tree *args,tmp; > > + > > + num_args = gfc_intrinsic_argument_list_length (expr); > > + args = XALLOCAVEC (tree, num_args); > > + > > + gfc_conv_intrinsic_function_args (se, expr, args, num_args); > > + > > + if (flag_coarray == GFC_FCOARRAY_LIB) > > +{ > Can everything be put under the if? > Does it work with -fcoarray=single? IMO coarray=single should not generate code here, therefore putting everything under the if should to fine. Sorry for the comments ... - Andre -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Hello, this is mostly good in general, but is lacking tests. Especially, tests for successfull matching, and tests for every error you are adding in the patch (except maybe the -fcoarray= one). Also tests that the code executes successfullly with -fcoarray=single, and that it produces the right function calls with -fcoarray=lib. more specific comments below. Mikael Le 15/07/2016 à 19:34, Alessandro Fanfarillo a écrit : Third *PING* 2016-07-04 16:46 GMT-06:00 Alessandro Fanfarillo : * PING * 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo : * PING * 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : Dear all, please find in attachment the first patch (of n) for the FAILED IMAGES capability defined in the coarray TS 18508. The patch adds support for three new intrinsic functions defined in the TS for simulating a failure (fail image), checking an image status (image_status) and getting the list of failed images (failed_images). The patch has been built and regtested on x86_64-pc-linux-gnu. Ok for trunk? Alessandro first_complete_patch.diff commit b3bca5b09f4cbcf18f2409dae2485a16a7c06498 Author: Alessandro Fanfarillo Date: Mon Jun 6 14:27:37 2016 -0600 First patch Failed Images CAF TS-18508 diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c index f3a4a43..9f519ff 100644 --- a/gcc/fortran/match.c +++ b/gcc/fortran/match.c @@ -1594,6 +1594,7 @@ gfc_match_if (gfc_statement *if_type) match ("event post", gfc_match_event_post, ST_EVENT_POST) match ("event wait", gfc_match_event_wait, ST_EVENT_WAIT) match ("exit", gfc_match_exit, ST_EXIT) + match ("fail image", gfc_match_fail_image, ST_FAIL_IMAGE) match ("flush", gfc_match_flush, ST_FLUSH) match ("forall", match_simple_forall, ST_FORALL) match ("go to", gfc_match_goto, ST_GOTO) @@ -3073,6 +3074,41 @@ gfc_match_event_wait (void) return event_statement (ST_EVENT_WAIT); } +/* Match a FAIl IMAGE statement */ + +static match +fail_image_statement (gfc_statement st) +{ + if (flag_coarray == GFC_FCOARRAY_NONE) +{ + gfc_fatal_error ("Coarrays disabled at %C, use %<-fcoarray=%> to enable"); + return MATCH_ERROR; +} + + if (gfc_match_char ('(') == MATCH_YES) +goto syntax; + + if(st == ST_FAIL_IMAGE) +new_st.op = EXEC_FAIL_IMAGE; + else +gcc_unreachable(); You can use gcc_assert (st == ST_FAIL_IMAGE); foo...; instead of if (st == ST_FAIL_IMAGE) foo...; else gcc_unreachable (); + + return MATCH_YES; + + syntax: + gfc_syntax_error (st); + + return MATCH_ERROR; +} + +match +gfc_match_fail_image (void) +{ + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement at %C")) */ + /* return MATCH_ERROR; */ + Can this be uncommented? + return fail_image_statement (ST_FAIL_IMAGE); +} /* Match LOCK/UNLOCK statement. Syntax: LOCK ( lock-variable [ , lock-stat-list ] ) diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr *expr) m, lbound)); } +static void +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) +{ + unsigned int num_args; + tree *args,tmp; + + num_args = gfc_intrinsic_argument_list_length (expr); + args = XALLOCAVEC (tree, num_args); + + gfc_conv_intrinsic_function_args (se, expr, args, num_args); + + if (flag_coarray == GFC_FCOARRAY_LIB) +{ Can everything be put under the if? Does it work with -fcoarray=single? + tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_image_status, 2, +args[0], build_int_cst (integer_type_node, -1)); + se->expr = tmp; +} +} static void diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 7d3cf8c..ce0eae7 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -674,6 +674,31 @@ gfc_trans_stop (gfc_code *code, bool error_stop) return gfc_finish_block (&se.pre); } +/* Translate the FAIL IMAGE statement. We have to translate this statement + to a runtime library call. */ + +tree +gfc_trans_fail_image (gfc_code *code ATTRIBUTE_UNUSED) +{ + tree gfc_int4_type_node = gfc_get_int_type (4); + gfc_se se; + tree tmp; + + /* Start a new block for this statement. */ + gfc_init_se (&se, NULL); + gfc_start_block (&se.pre); + + tmp = build_int_cst (gfc_int4_type_node, 0); This tmp doesn't seem to be used. + tmp = build_call_expr_loc (input_location, +gfor_fndecl_caf_fail_image, 1, +build_int_cst (pchar_type_node, 0)); + + gfc_add_expr_to_block (&se.pre, tmp); + + gfc_add_block_to_block (&se.pre, &se.post); + + return gfc_finish_block (&se.pre); +} tree gfc_trans_lock_unlock (gfc_code *code, gfc_exec_op op)
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Third *PING* 2016-07-04 16:46 GMT-06:00 Alessandro Fanfarillo : > * PING * > > 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo : >> * PING * >> >> 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : >>> Dear all, >>> >>> please find in attachment the first patch (of n) for the FAILED IMAGES >>> capability defined in the coarray TS 18508. >>> The patch adds support for three new intrinsic functions defined in >>> the TS for simulating a failure (fail image), checking an image status >>> (image_status) and getting the list of failed images (failed_images). >>> The patch has been built and regtested on x86_64-pc-linux-gnu. >>> >>> Ok for trunk? >>> >>> Alessandro
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
* PING * 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo : > * PING * > > 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : >> Dear all, >> >> please find in attachment the first patch (of n) for the FAILED IMAGES >> capability defined in the coarray TS 18508. >> The patch adds support for three new intrinsic functions defined in >> the TS for simulating a failure (fail image), checking an image status >> (image_status) and getting the list of failed images (failed_images). >> The patch has been built and regtested on x86_64-pc-linux-gnu. >> >> Ok for trunk? >> >> Alessandro
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
* PING * 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : > Dear all, > > please find in attachment the first patch (of n) for the FAILED IMAGES > capability defined in the coarray TS 18508. > The patch adds support for three new intrinsic functions defined in > the TS for simulating a failure (fail image), checking an image status > (image_status) and getting the list of failed images (failed_images). > The patch has been built and regtested on x86_64-pc-linux-gnu. > > Ok for trunk? > > Alessandro
[Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Dear all, please find in attachment the first patch (of n) for the FAILED IMAGES capability defined in the coarray TS 18508. The patch adds support for three new intrinsic functions defined in the TS for simulating a failure (fail image), checking an image status (image_status) and getting the list of failed images (failed_images). The patch has been built and regtested on x86_64-pc-linux-gnu. Ok for trunk? Alessandro commit b3bca5b09f4cbcf18f2409dae2485a16a7c06498 Author: Alessandro Fanfarillo Date: Mon Jun 6 14:27:37 2016 -0600 First patch Failed Images CAF TS-18508 diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index d26e45e..71931cb 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -1210,6 +1210,62 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *count, gfc_expr *stat) return true; } +bool +gfc_check_image_status (gfc_expr *image, gfc_expr *team) +{ + if (!type_check (image, 1, BT_INTEGER)) +return false; + + if(team) +{ + gfc_error ("TEAM argument of the IMAGE_STATUS intrinsic function at %L " +"not yet supported", +&team->where); + return false; +} + + int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false); + int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false); + + if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range) +{ + gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L " +"shall have at least the range of the default integer", +&image->where); + return false; +} + + return true; +} + +bool +gfc_check_failed_images (gfc_expr *team, gfc_expr *kind) +{ + if (team) +{ + gfc_error ("TEAM argument of the FAILED_IMAGES intrinsic function at %L " +"not yet supported", +&team->where); + return false; +} + + if (kind) +{ + int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false); + int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false); + + if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range) + { + gfc_error ("KIND argument of the FAILED_IMAGES intrinsic function at %L " +"shall have at least the range of the default integer", +&kind->where); + return false; + } +} + + return true; +} + bool gfc_check_atomic_fetch_op (gfc_expr *atom, gfc_expr *value, gfc_expr *old, diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c index f507434..41ed664 100644 --- a/gcc/fortran/dump-parse-tree.c +++ b/gcc/fortran/dump-parse-tree.c @@ -1628,6 +1628,9 @@ show_code_node (int level, gfc_code *c) break; +case EXEC_FAIL_IMAGE: + fputs ("FAIL IMAGE ", dumpfile); + case EXEC_SYNC_ALL: fputs ("SYNC ALL ", dumpfile); if (c->expr2 != NULL) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 0bb71cb..6d87632 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -253,7 +253,7 @@ enum gfc_statement ST_OMP_END_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD, ST_PROCEDURE, ST_GENERIC, ST_CRITICAL, ST_END_CRITICAL, ST_GET_FCN_CHARACTERISTICS, ST_LOCK, ST_UNLOCK, ST_EVENT_POST, - ST_EVENT_WAIT,ST_NONE + ST_EVENT_WAIT,ST_FAIL_IMAGE,ST_NONE }; /* Types of interfaces that we can have. Assignment interfaces are @@ -411,6 +411,7 @@ enum gfc_isym_id GFC_ISYM_EXP, GFC_ISYM_EXPONENT, GFC_ISYM_EXTENDS_TYPE_OF, + GFC_ISYM_FAILED_IMAGES, GFC_ISYM_FDATE, GFC_ISYM_FE_RUNTIME_ERROR, GFC_ISYM_FGET, @@ -454,6 +455,7 @@ enum gfc_isym_id GFC_ISYM_IEOR, GFC_ISYM_IERRNO, GFC_ISYM_IMAGE_INDEX, + GFC_ISYM_IMAGE_STATUS, GFC_ISYM_INDEX, GFC_ISYM_INT, GFC_ISYM_INT2, @@ -2382,7 +2384,7 @@ enum gfc_exec_op EXEC_OPEN, EXEC_CLOSE, EXEC_WAIT, EXEC_READ, EXEC_WRITE, EXEC_IOLENGTH, EXEC_TRANSFER, EXEC_DT_END, EXEC_BACKSPACE, EXEC_ENDFILE, EXEC_INQUIRE, EXEC_REWIND, EXEC_FLUSH, - EXEC_LOCK, EXEC_UNLOCK, EXEC_EVENT_POST, EXEC_EVENT_WAIT, + EXEC_LOCK, EXEC_UNLOCK, EXEC_EVENT_POST, EXEC_EVENT_WAIT, EXEC_FAIL_IMAGE, EXEC_OACC_KERNELS_LOOP, EXEC_OACC_PARALLEL_LOOP, EXEC_OACC_ROUTINE, EXEC_OACC_PARALLEL, EXEC_OACC_KERNELS, EXEC_OACC_DATA, EXEC_OACC_HOST_DATA, EXEC_OACC_LOOP, EXEC_OACC_UPDATE, EXEC_OACC_WAIT, EXEC_OACC_CACHE, diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c index 1d7503d..8dfb568 100644 --- a/gcc/fortran/intrinsic.c +++ b/gcc/fortran/intrinsic.c @@ -1823,6 +1823,10 @@ add_functions (void) a, BT_UNKNOWN, 0, REQUIRED, mo, BT_UNKNOWN, 0, REQUIRED); + add_sym_2 ("failed_images", GFC_ISYM_FAILED_IMAGES, CLASS_TRANSFORMATIONAL, ACTUAL_NO, BT_INTEGER, +dd, GFC_STD_F2008_TS, gfc_check_failed_images, NULL, +gfc_resolve_failed_images, "team", BT_INTEGER, di, OPTIONAL, "kind", BT_INTEGER, di, OPTIONAL); + add_sym_0 ("fdate", GFC_ISYM_FDATE, CLASS_IMPURE, ACTUAL_NO, B