| Message ID | 20170113112323.7aa27b42@vepi2 |
|---|---|
| State | New |
| Headers | show |
On 01/18/2017 04:26 AM, Andre Vehreschild wrote: > Hi all, > > the patch I proposed for this pr unfortunately did not catch all errors. > Dominique figured, that the original testcase was not resolved (thanks for > that). > > This patch resolves the linker problem by putting the static token into the > parent function's decl list. Furthermore does the patch beautify the > retrieval of the symbol in gfc_get_tree_for_caf_expr () and remove the following > assert which is unnecessary then, because the symbol is either already present > or created. And gfc_get_symbol_decl () can not return NULL. > > Bootstrapped and regtested ok on x86_64-linux/f25 and x86-linux/f25 for trunk. > Bootstrapped and regtested ok on x86_64-linux/f25 for gcc-6 (x86-linux has not > been tested, because the VM is not that fast). > > Ok for trunk and gcc-6? > > Regards, > Andre > This one is OK, thanks. Jerry
Hi Jerry, thanks for the fast review. Committed as r244587. Regards, Andre On Wed, 18 Jan 2017 09:38:40 -0800 Jerry DeLisle <jvdelisle@charter.net> wrote: > On 01/18/2017 04:26 AM, Andre Vehreschild wrote: > > Hi all, > > > > the patch I proposed for this pr unfortunately did not catch all errors. > > Dominique figured, that the original testcase was not resolved (thanks for > > that). > > > > This patch resolves the linker problem by putting the static token into the > > parent function's decl list. Furthermore does the patch beautify the > > retrieval of the symbol in gfc_get_tree_for_caf_expr () and remove the > > following assert which is unnecessary then, because the symbol is either > > already present or created. And gfc_get_symbol_decl () can not return NULL. > > > > Bootstrapped and regtested ok on x86_64-linux/f25 and x86-linux/f25 for > > trunk. Bootstrapped and regtested ok on x86_64-linux/f25 for gcc-6 > > (x86-linux has not been tested, because the VM is not that fast). > > > > Ok for trunk and gcc-6? > > > > Regards, > > Andre > > > > This one is OK, thanks. > > Jerry -- Andre Vehreschild * Email: vehre ad gmx dot deIndex: gcc/fortran/ChangeLog =================================================================== --- gcc/fortran/ChangeLog (Revision 244585) +++ gcc/fortran/ChangeLog (Arbeitskopie) @@ -1,3 +1,12 @@ +2017-01-17 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + Missed some cases, here they are: + * trans-decl.c (gfc_build_qualified_array): Add static tokens to the + parent function's scope. + * trans-expr.c (gfc_get_tree_for_caf_expr): Shorten code. Remove + unnecessary assert. + 2017-01-13 Andre Vehreschild <vehre@gcc.gnu.org> PR fortran/70697 Index: gcc/fortran/trans-decl.c =================================================================== --- gcc/fortran/trans-decl.c (Revision 244585) +++ gcc/fortran/trans-decl.c (Arbeitskopie) @@ -971,6 +971,8 @@ DECL_CONTEXT (token) = sym->ns->proc_name->backend_decl; gfc_module_add_decl (cur_module, token); } + else if (sym->attr.host_assoc) + gfc_add_decl_to_parent_function (token); else gfc_add_decl_to_function (token); } Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (Revision 244585) +++ gcc/fortran/trans-expr.c (Arbeitskopie) @@ -1839,11 +1839,10 @@ } /* Make sure the backend_decl is present before accessing it. */ - if (expr->symtree->n.sym->backend_decl == NULL_TREE) - expr->symtree->n.sym->backend_decl - = gfc_get_symbol_decl (expr->symtree->n.sym); - caf_decl = expr->symtree->n.sym->backend_decl; - gcc_assert (caf_decl); + caf_decl = expr->symtree->n.sym->backend_decl == NULL_TREE + ? gfc_get_symbol_decl (expr->symtree->n.sym) + : expr->symtree->n.sym->backend_decl; + if (expr->symtree->n.sym->ts.type == BT_CLASS) { if (expr->ref && expr->ref->type == REF_ARRAY) Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (Revision 244585) +++ gcc/testsuite/ChangeLog (Arbeitskopie) @@ -1,3 +1,8 @@ +2017-01-17 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * gfortran.dg/coarray_event_1.f08: New test. + 2017-01-18 Jakub Jelinek <jakub@redhat.com> PR target/77416 Index: gcc/testsuite/gfortran.dg/coarray_event_1.f08 =================================================================== --- gcc/testsuite/gfortran.dg/coarray_event_1.f08 (nicht existent) +++ gcc/testsuite/gfortran.dg/coarray_event_1.f08 (Arbeitskopie) @@ -0,0 +1,15 @@ +! { dg-do compile } +! { dg-options "-fcoarray=lib -lcaf_single" } + +! Check that pr70696 is really fixed. + + use iso_fortran_env + type(event_type) :: x[*] + + ! exchange must not be called or the link problem before the patch + ! does not occur. +contains + subroutine exchange + event post (x[1]) + end subroutine +end
Hi all, unfortunately triggered this patch a regression in the opencoarray's testsuite, which also occurs outside of opencoarray, when a caf-function is used in a block in the main-program. This patch fixes the error and adds a testcase. Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk? Regards, Andre On Wed, 18 Jan 2017 19:35:59 +0100 Andre Vehreschild <vehre@gmx.de> wrote: > Hi Jerry, > > thanks for the fast review. Committed as r244587. > > Regards, > Andre > > On Wed, 18 Jan 2017 09:38:40 -0800 > Jerry DeLisle <jvdelisle@charter.net> wrote: > > > On 01/18/2017 04:26 AM, Andre Vehreschild wrote: > > > Hi all, > > > > > > the patch I proposed for this pr unfortunately did not catch all errors. > > > Dominique figured, that the original testcase was not resolved (thanks for > > > that). > > > > > > This patch resolves the linker problem by putting the static token into > > > the parent function's decl list. Furthermore does the patch beautify the > > > retrieval of the symbol in gfc_get_tree_for_caf_expr () and remove the > > > following assert which is unnecessary then, because the symbol is either > > > already present or created. And gfc_get_symbol_decl () can not return > > > NULL. > > > > > > Bootstrapped and regtested ok on x86_64-linux/f25 and x86-linux/f25 for > > > trunk. Bootstrapped and regtested ok on x86_64-linux/f25 for gcc-6 > > > (x86-linux has not been tested, because the VM is not that fast). > > > > > > Ok for trunk and gcc-6? > > > > > > Regards, > > > Andre > > > > > > > This one is OK, thanks. > > > > Jerry > > -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-01-19 Andre Vehreschild <vehre@gcc.gnu.org> PR fortran/70696 * trans-decl.c (gfc_build_qualified_array): Add static decl to parent function only, when the decl-context is not the translation unit. gcc/testsuite/ChangeLog: 2017-01-19 Andre Vehreschild <vehre@gcc.gnu.org> * gfortran.dg/coarray_43.f90: New test.diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 51c23e8..5d246cd 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -971,7 +971,9 @@ gfc_build_qualified_array (tree decl, gfc_symbol * sym) DECL_CONTEXT (token) = sym->ns->proc_name->backend_decl; gfc_module_add_decl (cur_module, token); } - else if (sym->attr.host_assoc) + else if (sym->attr.host_assoc + && TREE_CODE (DECL_CONTEXT (current_function_decl)) + != TRANSLATION_UNIT_DECL) gfc_add_decl_to_parent_function (token); else gfc_add_decl_to_function (token); diff --git a/gcc/testsuite/gfortran.dg/coarray_43.f90 b/gcc/testsuite/gfortran.dg/coarray_43.f90 new file mode 100644 index 0000000..d5ee4e1 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray_43.f90 @@ -0,0 +1,13 @@ +! { dg-do link } +! { dg-options "-fcoarray=lib -lcaf_single" } + +program coarray_43 + implicit none + integer, parameter :: STR_LEN = 50 + character(len=STR_LEN) :: str[*] + integer :: pos + write(str,"(2(a,i2))") "Greetings from image ",this_image()," of ",num_images() + block + pos = scan(str[5], set="123456789") + end block +end program
On Thu, Jan 19, 2017 at 01:07:50PM +0100, Andre Vehreschild wrote: > Hi all, > > unfortunately triggered this patch a regression in the opencoarray's testsuite, > which also occurs outside of opencoarray, when a caf-function is used in a > block in the main-program. This patch fixes the error and adds a testcase. > > Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk? > Yes. -- Steve 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Hi Steve, thanks for the review. Committed as r244637. Regards, Andre On Thu, 19 Jan 2017 06:51:19 -0800 Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > On Thu, Jan 19, 2017 at 01:07:50PM +0100, Andre Vehreschild wrote: > > Hi all, > > > > unfortunately triggered this patch a regression in the opencoarray's > > testsuite, which also occurs outside of opencoarray, when a caf-function is > > used in a block in the main-program. This patch fixes the error and adds a > > testcase. > > > > Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk? > > > > Yes. > -- Andre Vehreschild * Email: vehre ad gmx dot de
Hi all, just applied the backport to gcc-6 as promised as r245014. Regards, Andre On Thu, 19 Jan 2017 16:53:14 +0100 Andre Vehreschild <vehre@gmx.de> wrote: > Hi Steve, > > thanks for the review. Committed as r244637. > > Regards, > Andre > > On Thu, 19 Jan 2017 06:51:19 -0800 > Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > > > On Thu, Jan 19, 2017 at 01:07:50PM +0100, Andre Vehreschild wrote: > > > Hi all, > > > > > > unfortunately triggered this patch a regression in the opencoarray's > > > testsuite, which also occurs outside of opencoarray, when a caf-function > > > is used in a block in the main-program. This patch fixes the error and > > > adds a testcase. > > > > > > Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk? > > > > > > > Yes. > > > > -- Andre Vehreschild * Email: vehre ad gmx dot deIndex: gcc/fortran/ChangeLog =================================================================== --- gcc/fortran/ChangeLog (Revision 245013) +++ gcc/fortran/ChangeLog (Arbeitskopie) @@ -1,3 +1,13 @@ +2017-01-29 Andre Vehreschild <vehre@gcc.gnu.org> + + Backport from trunk + PR fortran/70696 + * trans-expr.c (gfc_get_tree_for_caf_expr): Ensure the backend_decl + is valid before accessing it. Remove unnecessary assert. + * trans-decl.c (gfc_build_qualified_array): Add static tokens to the + parent function's scope only, when the decl-context is not the + translation unit. + 2017-01-17 Jakub Jelinek <jakub@redhat.com> Backported from mainline Index: gcc/fortran/trans-decl.c =================================================================== --- gcc/fortran/trans-decl.c (Revision 245013) +++ gcc/fortran/trans-decl.c (Arbeitskopie) @@ -887,6 +887,10 @@ DECL_CONTEXT (token) = sym->ns->proc_name->backend_decl; gfc_module_add_decl (cur_module, token); } + else if (sym->attr.host_assoc + && TREE_CODE (DECL_CONTEXT (current_function_decl)) + != TRANSLATION_UNIT_DECL) + gfc_add_decl_to_parent_function (token); else gfc_add_decl_to_function (token); } Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (Revision 245013) +++ gcc/fortran/trans-expr.c (Arbeitskopie) @@ -1898,8 +1898,11 @@ &expr->where); } - caf_decl = expr->symtree->n.sym->backend_decl; - gcc_assert (caf_decl); + /* Make sure the backend_decl is present before accessing it. */ + caf_decl = expr->symtree->n.sym->backend_decl == NULL_TREE + ? gfc_get_symbol_decl (expr->symtree->n.sym) + : expr->symtree->n.sym->backend_decl; + if (expr->symtree->n.sym->ts.type == BT_CLASS) caf_decl = gfc_class_data_get (caf_decl); if (expr->symtree->n.sym->attr.codimension) Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (Revision 245013) +++ gcc/testsuite/ChangeLog (Arbeitskopie) @@ -1,3 +1,21 @@ +2017-01-29 Andre Vehreschild <vehre@gcc.gnu.org> + + Backport from trunk + 2017-01-19 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * gfortran.dg/coarray_43.f90: New test. + + 2017-01-18 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * gfortran.dg/coarray_event_1.f08: New test. + + 2017-01-13 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * gfortran.dg/coarray/event_3.f08: New test. + 2017-01-28 John David Anglin <danglin@gcc.gnu.org> PR testsuite/70583 Index: gcc/testsuite/gfortran.dg/coarray/event_3.f08 =================================================================== --- gcc/testsuite/gfortran.dg/coarray/event_3.f08 (nicht existent) +++ gcc/testsuite/gfortran.dg/coarray/event_3.f08 (Arbeitskopie) @@ -0,0 +1,20 @@ +! { dg-do run } +! +! Check PR fortran/70696 is fixed. + +program global_event + use iso_fortran_env , only : event_type + implicit none + type(event_type) :: x[*] + + call exchange + contains + subroutine exchange + integer :: cnt + event post(x[1]) + event post(x[1]) + call event_query(x, cnt) + if (cnt /= 2) error stop 1 + event wait(x, until_count=2) + end subroutine +end Index: gcc/testsuite/gfortran.dg/coarray_43.f90 =================================================================== --- gcc/testsuite/gfortran.dg/coarray_43.f90 (nicht existent) +++ gcc/testsuite/gfortran.dg/coarray_43.f90 (Arbeitskopie) @@ -0,0 +1,13 @@ +! { dg-do link } +! { dg-options "-fcoarray=lib -lcaf_single" } + +program coarray_43 + implicit none + integer, parameter :: STR_LEN = 50 + character(len=STR_LEN) :: str[*] + integer :: pos + write(str,"(2(a,i2))") "Greetings from image ",this_image()," of ",num_images() + block + pos = scan(str[5], set="123456789") + end block +end program Index: gcc/testsuite/gfortran.dg/coarray_event_1.f08 =================================================================== --- gcc/testsuite/gfortran.dg/coarray_event_1.f08 (nicht existent) +++ gcc/testsuite/gfortran.dg/coarray_event_1.f08 (Arbeitskopie) @@ -0,0 +1,15 @@ +! { dg-do compile } +! { dg-options "-fcoarray=lib -lcaf_single" } + +! Check that pr70696 is really fixed. + + use iso_fortran_env + type(event_type) :: x[*] + + ! exchange must not be called or the link problem before the patch + ! does not occur. +contains + subroutine exchange + event post (x[1]) + end subroutine +end
Hi all, during the backport of pr70696 I forgot to backport the runtime part in libgfortran/caf/single.c. This is fixed by commit r245016. Sorry for the noise, regards, Andre On Sun, 29 Jan 2017 14:51:03 +0100 Andre Vehreschild <vehre@gmx.de> wrote: > Hi all, > > just applied the backport to gcc-6 as promised as r245014. > > Regards, > Andre > > On Thu, 19 Jan 2017 16:53:14 +0100 > Andre Vehreschild <vehre@gmx.de> wrote: > > > Hi Steve, > > > > thanks for the review. Committed as r244637. > > > > Regards, > > Andre > > > > On Thu, 19 Jan 2017 06:51:19 -0800 > > Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > > > > > On Thu, Jan 19, 2017 at 01:07:50PM +0100, Andre Vehreschild wrote: > > > > Hi all, > > > > > > > > unfortunately triggered this patch a regression in the opencoarray's > > > > testsuite, which also occurs outside of opencoarray, when a caf-function > > > > is used in a block in the main-program. This patch fixes the error and > > > > adds a testcase. > > > > > > > > Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk? > > > > > > > > > > Yes. > > > > > > > > > -- Andre Vehreschild * Email: vehre ad gmx dot deIndex: libgfortran/caf/single.c =================================================================== --- libgfortran/caf/single.c (Revision 245015) +++ libgfortran/caf/single.c (Arbeitskopie) @@ -101,9 +101,12 @@ void *local; if (type == CAF_REGTYPE_LOCK_STATIC || type == CAF_REGTYPE_LOCK_ALLOC - || type == CAF_REGTYPE_CRITICAL || type == CAF_REGTYPE_EVENT_STATIC - || type == CAF_REGTYPE_EVENT_ALLOC) - local = calloc (size, sizeof (bool)); + || type == CAF_REGTYPE_CRITICAL) + local = calloc (size, sizeof (bool)); + else if (type == CAF_REGTYPE_EVENT_STATIC || type == CAF_REGTYPE_EVENT_ALLOC) + /* In the event_(wait|post) function the counter for events is a uint32, + so better allocate enough memory here. */ + local = calloc (size, sizeof (uint32_t)); else local = malloc (size); *token = malloc (sizeof (single_token_t));
Index: gcc/fortran/ChangeLog =================================================================== --- gcc/fortran/ChangeLog (Revision 244394) +++ gcc/fortran/ChangeLog (Arbeitskopie) @@ -1,3 +1,9 @@ +2017-01-13 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * trans-expr.c (gfc_get_tree_for_caf_expr): Ensure the backend_decl + is valid before accessing it. + 2017-01-09 Jakub Jelinek <jakub@redhat.com> PR translation/79019 Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (Revision 244394) +++ gcc/fortran/trans-expr.c (Arbeitskopie) @@ -1838,6 +1838,10 @@ "component at %L is not supported", &expr->where); } + /* Make sure the backend_decl is present before accessing it. */ + if (expr->symtree->n.sym->backend_decl == NULL_TREE) + expr->symtree->n.sym->backend_decl + = gfc_get_symbol_decl (expr->symtree->n.sym); caf_decl = expr->symtree->n.sym->backend_decl; gcc_assert (caf_decl); if (expr->symtree->n.sym->ts.type == BT_CLASS) Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (Revision 244394) +++ gcc/testsuite/ChangeLog (Arbeitskopie) @@ -1,3 +1,8 @@ +2017-01-13 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * gfortran.dg/coarray/event_3.f08: New test. + 2017-01-13 Richard Biener <rguenther@suse.de> PR tree-optimization/77283 Index: gcc/testsuite/gfortran.dg/coarray/event_3.f08 =================================================================== --- gcc/testsuite/gfortran.dg/coarray/event_3.f08 (nicht existent) +++ gcc/testsuite/gfortran.dg/coarray/event_3.f08 (Arbeitskopie) @@ -0,0 +1,20 @@ +! { dg-do run } +! +! Check PR fortran/70696 is fixed. + +program global_event + use iso_fortran_env , only : event_type + implicit none + type(event_type) :: x[*] + + call exchange + contains + subroutine exchange + integer :: cnt + event post(x[1]) + event post(x[1]) + call event_query(x, cnt) + if (cnt /= 2) error stop 1 + event wait(x, until_count=2) + end subroutine +end Index: libgfortran/ChangeLog =================================================================== --- libgfortran/ChangeLog (Revision 244394) +++ libgfortran/ChangeLog (Arbeitskopie) @@ -1,3 +1,9 @@ +2017-01-13 Andre Vehreschild <vehre@gcc.gnu.org> + + PR fortran/70696 + * caf/single.c (_gfortran_caf_register): Allocate enough memory for + the event counter. + 2017-01-07 Andre Vehreschild <vehre@gcc.gnu.org> PR fortran/78781 Index: libgfortran/caf/single.c =================================================================== --- libgfortran/caf/single.c (Revision 244394) +++ libgfortran/caf/single.c (Arbeitskopie) @@ -141,9 +141,12 @@ caf_single_token_t single_token; if (type == CAF_REGTYPE_LOCK_STATIC || type == CAF_REGTYPE_LOCK_ALLOC - || type == CAF_REGTYPE_CRITICAL || type == CAF_REGTYPE_EVENT_STATIC - || type == CAF_REGTYPE_EVENT_ALLOC) + || type == CAF_REGTYPE_CRITICAL) local = calloc (size, sizeof (bool)); + else if (type == CAF_REGTYPE_EVENT_STATIC || type == CAF_REGTYPE_EVENT_ALLOC) + /* In the event_(wait|post) function the counter for events is a uint32, + so better allocate enough memory here. */ + local = calloc (size, sizeof (uint32_t)); else if (type == CAF_REGTYPE_COARRAY_ALLOC_REGISTER_ONLY) local = NULL; else