* [Fortran, Patch] Passing function pointer to co_reduce @ 2015-07-13 15:31 Alessandro Fanfarillo 2015-07-15 13:13 ` Mikael Morin 0 siblings, 1 reply; 10+ messages in thread From: Alessandro Fanfarillo @ 2015-07-13 15:31 UTC (permalink / raw) To: gcc-patches, gfortran [-- Attachment #1: Type: text/plain, Size: 482 bytes --] Dear all, during the implementation of co_reduce in OpenCoarrays I noticed that GFortran passes a pointer to function instead of the function name to co_reduce. Currently the compiler produces the following call: _gfortran_caf_co_reduce (&desc.0, &simple_reduction, 0, 0, 0B, 0B, 0, 0); where simple_reduction is the pure function that has to be used by co_reduce. The attached patch seems to fix the issue, any comments? Regards Alessandro PS: I also attach the test case [-- Attachment #2: co_reduce_patch.diff --] [-- Type: text/plain, Size: 778 bytes --] commit a12b6ce6993df109c81a32d5684b4b9f41f69ea4 Author: Alessandro Fanfarillo <fanfarillo@ing.uniroma2.it> Date: Mon Jul 13 15:46:19 2015 +0200 Fix function pointer argument for co_reduce diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 66bc72a..967a741 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -8804,7 +8804,7 @@ conv_co_collective (gfc_code *code) } opr_flags = build_int_cst (integer_type_node, opr_flag_int); gfc_conv_expr (&argse, opr_expr); - opr = gfc_build_addr_expr (NULL_TREE, argse.expr); + opr = argse.expr; fndecl = build_call_expr_loc (input_location, fndecl, 8, array, opr, opr_flags, image_index, stat, errmsg, strlen, errmsg_len); } [-- Attachment #3: simple_reduce.f90 --] [-- Type: text/x-fortran, Size: 359 bytes --] program simple_reduce implicit none integer :: me me = this_image() sync all call co_reduce(me,simple_reduction) write(*,*) this_image(),me contains pure function simple_reduction(a,b) integer,intent(in) :: a,b integer :: simple_reduction simple_reduction = a * b end function simple_reduction end program simple_reduce ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-13 15:31 [Fortran, Patch] Passing function pointer to co_reduce Alessandro Fanfarillo @ 2015-07-15 13:13 ` Mikael Morin 2015-07-16 14:58 ` Damian Rouson 0 siblings, 1 reply; 10+ messages in thread From: Mikael Morin @ 2015-07-15 13:13 UTC (permalink / raw) To: Alessandro Fanfarillo, gcc-patches, gfortran Hello, Le 13/07/2015 17:31, Alessandro Fanfarillo a écrit : > Dear all, > > during the implementation of co_reduce in OpenCoarrays I noticed that > GFortran passes a pointer to function instead of the function name to > co_reduce. > More exactly there are two address operators ('&') in a row (the dumping removes one of them). > Currently the compiler produces the following call: > > _gfortran_caf_co_reduce (&desc.0, &simple_reduction, 0, 0, 0B, 0B, 0, 0); > > where simple_reduction is the pure function that has to be used by co_reduce. > > The attached patch seems to fix the issue, any comments? > The patch itself looks good to me. A ChangeLog entry should be provided with it. The test is missing the usual dejagnu pattern matching directives to check the generated code. Do you have commit rights? Mikael ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-15 13:13 ` Mikael Morin @ 2015-07-16 14:58 ` Damian Rouson 2015-07-17 9:28 ` Mikael Morin 0 siblings, 1 reply; 10+ messages in thread From: Damian Rouson @ 2015-07-16 14:58 UTC (permalink / raw) To: Mikael Morin; +Cc: Alessandro Fanfarillo, gcc-patches, gfortran > On Jul 15, 2015, at 8:58 AM, Mikael Morin <mikael.morin@sfr.fr> wrote: > > The patch itself looks good to me. > A ChangeLog entry should be provided with it. > The test is missing the usual dejagnu pattern matching directives to > check the generated code. > Do you have commit rights? Hi Mikael, Thanks for the quick review. I know Alessandro is not a reviewer. Assuming commit rights go with review rights, I’m guessing he doesn’t have commit rights. I’ll let him confirm. If he doesn’t have commit rights, could you commit it for us after he adds the derange pattern-matching directives? Alternatively, if it’s easy, please feel free to add the directives and commit. Your fast response is very helpful to us. The co_reduce collective is the final feature that we hope to finish before taking a snapshot of OpenCoarrays and designating it as our 1.0.0 release. :) Toward that end, could this fix also be applied to the GCC 5 branch? Soon after posting a tar ball of OpenCoarrays 1.0.0 on the web, Alessandro will submit a Portfile to the Macports developers that will enable OS X users to install OpenCoarrays 1.0.0 for use with GCC 5. It would be great for Macports users to have co_reduce when the next GCC 5 update releases. Damian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-16 14:58 ` Damian Rouson @ 2015-07-17 9:28 ` Mikael Morin 2015-07-17 13:54 ` Mikael Morin 0 siblings, 1 reply; 10+ messages in thread From: Mikael Morin @ 2015-07-17 9:28 UTC (permalink / raw) To: Damian Rouson; +Cc: Alessandro Fanfarillo, gcc-patches, gfortran Le 16/07/2015 16:34, Damian Rouson a écrit : > Alternatively, if it’s easy, please feel free to add the directives and commit. > It's not very difficult, but there is some pain getting the number of escape backslashes right. Never mind, I'll take care of it all. Mikael ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 9:28 ` Mikael Morin @ 2015-07-17 13:54 ` Mikael Morin 2015-07-17 14:42 ` Damian Rouson ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Mikael Morin @ 2015-07-17 13:54 UTC (permalink / raw) To: Damian Rouson; +Cc: Alessandro Fanfarillo, gcc-patches, gfortran [-- Attachment #1: Type: text/plain, Size: 340 bytes --] Le 17/07/2015 11:02, Mikael Morin a écrit : > Le 16/07/2015 16:34, Damian Rouson a écrit : >> Alternatively, if it’s easy, please feel free to add the directives and commit. >> > Never mind, I'll take care of it all. > This is what I have committed: https://gcc.gnu.org/r225930 (trunk) https://gcc.gnu.org/r225932 (5 branch) Mikael [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: r225930.diff --] [-- Type: text/x-patch; name="r225930.diff", Size: 2443 bytes --] Index: gcc/testsuite/gfortran.dg/co_reduce_1.f90 =================================================================== --- gcc/testsuite/gfortran.dg/co_reduce_1.f90 (révision 0) +++ gcc/testsuite/gfortran.dg/co_reduce_1.f90 (révision 225930) @@ -0,0 +1,33 @@ +! { dg-do compile } +! { dg-additional-options "-fdump-tree-original -fcoarray=lib" } +! +! Check that we don't take twice the address of procedure simple_reduction +! in the generated code. +! +! Contributed by Alessandro Fanfarillo <fanfarillo.gcc@gmail.com> + +program simple_reduce + implicit none + + integer :: me + + me = this_image() + + sync all + + call co_reduce(me,simple_reduction) + + write(*,*) this_image(),me + +contains + + pure function simple_reduction(a,b) + integer,intent(in) :: a,b + integer :: simple_reduction + + simple_reduction = a * b + end function simple_reduction + +end program simple_reduce + +! { dg-final { scan-tree-dump "_gfortran_caf_co_reduce \\(&desc\\.\\d+,\\s*simple_reduction," "original" } } Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (révision 225929) +++ gcc/testsuite/ChangeLog (révision 225930) @@ -1,3 +1,7 @@ +2015-07-17 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com> + + * gfortran.dg/co_reduce_1.f90: New file. + 2015-07-17 H.J. Lu <hongjiu.lu@intel.com> PR target/66824 Index: gcc/fortran/ChangeLog =================================================================== --- gcc/fortran/ChangeLog (révision 225929) +++ gcc/fortran/ChangeLog (révision 225930) @@ -1,3 +1,8 @@ +2015-07-17 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com> + + * trans-intrinsic.c (conv_co_collective): Remove redundant address + operator in the generated code. + 2015-07-17 Andre Vehreschild <vehre@gcc.gnu.org> PR fortran/66035 Index: gcc/fortran/trans-intrinsic.c =================================================================== --- gcc/fortran/trans-intrinsic.c (révision 225929) +++ gcc/fortran/trans-intrinsic.c (révision 225930) @@ -8804,7 +8804,7 @@ } opr_flags = build_int_cst (integer_type_node, opr_flag_int); gfc_conv_expr (&argse, opr_expr); - opr = gfc_build_addr_expr (NULL_TREE, argse.expr); + opr = argse.expr; fndecl = build_call_expr_loc (input_location, fndecl, 8, array, opr, opr_flags, image_index, stat, errmsg, strlen, errmsg_len); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 13:54 ` Mikael Morin @ 2015-07-17 14:42 ` Damian Rouson 2015-07-17 14:43 ` Alessandro Fanfarillo 2015-07-17 18:47 ` Steve Kargl 2 siblings, 0 replies; 10+ messages in thread From: Damian Rouson @ 2015-07-17 14:42 UTC (permalink / raw) To: Mikael Morin; +Cc: Alessandro Fanfarillo, gcc-patches, gfortran Thank you! Damian > On Jul 17, 2015, at 9:50 AM, Mikael Morin <mikael.morin@sfr.fr> wrote: > > Le 17/07/2015 11:02, Mikael Morin a écrit : >> Le 16/07/2015 16:34, Damian Rouson a écrit : >>> Alternatively, if it’s easy, please feel free to add the directives and commit. >>> > >> Never mind, I'll take care of it all. >> > This is what I have committed: > https://gcc.gnu.org/r225930 (trunk) > https://gcc.gnu.org/r225932 (5 branch) > > Mikael > <r225930.diff> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 13:54 ` Mikael Morin 2015-07-17 14:42 ` Damian Rouson @ 2015-07-17 14:43 ` Alessandro Fanfarillo 2015-07-17 18:47 ` Steve Kargl 2 siblings, 0 replies; 10+ messages in thread From: Alessandro Fanfarillo @ 2015-07-17 14:43 UTC (permalink / raw) To: Mikael Morin; +Cc: Damian Rouson, gcc-patches, gfortran Thanks a lot! 2015-07-17 15:50 GMT+02:00 Mikael Morin <mikael.morin@sfr.fr>: > Le 17/07/2015 11:02, Mikael Morin a écrit : >> Le 16/07/2015 16:34, Damian Rouson a écrit : >>> Alternatively, if it’s easy, please feel free to add the directives and commit. >>> > >> Never mind, I'll take care of it all. >> > This is what I have committed: > https://gcc.gnu.org/r225930 (trunk) > https://gcc.gnu.org/r225932 (5 branch) > > Mikael ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 13:54 ` Mikael Morin 2015-07-17 14:42 ` Damian Rouson 2015-07-17 14:43 ` Alessandro Fanfarillo @ 2015-07-17 18:47 ` Steve Kargl 2015-07-17 19:47 ` Mikael Morin 2 siblings, 1 reply; 10+ messages in thread From: Steve Kargl @ 2015-07-17 18:47 UTC (permalink / raw) To: Mikael Morin; +Cc: Damian Rouson, Alessandro Fanfarillo, gcc-patches, gfortran On Fri, Jul 17, 2015 at 03:50:26PM +0200, Mikael Morin wrote: > Le 17/07/2015 11:02, Mikael Morin a écrit : > > Le 16/07/2015 16:34, Damian Rouson a écrit : > >> Alternatively, if it?s easy, please feel free to add the directives and commit. > >> > > > Never mind, I'll take care of it all. > > > This is what I have committed: > https://gcc.gnu.org/r225930 (trunk) > https://gcc.gnu.org/r225932 (5 branch) > Hi Mikael, It seems that this commit has changed the scanning of other testcases. FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&desc.., &fr, 4, _gfortran_caf_num_images \\(0, -1\\), &stat1, errmesg1, 0, 6\\);" 1 FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&val2, &gz, 0, 4, &stat2, errmesg2, 0, 7\\);" 1 FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&desc.., &hc, 1, res, &stat3, errmesg3, 99, 8\\);" -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 18:47 ` Steve Kargl @ 2015-07-17 19:47 ` Mikael Morin 2015-07-17 20:47 ` Mikael Morin 0 siblings, 1 reply; 10+ messages in thread From: Mikael Morin @ 2015-07-17 19:47 UTC (permalink / raw) To: Steve Kargl; +Cc: Damian Rouson, Alessandro Fanfarillo, gcc-patches, gfortran Le 17/07/2015 20:40, Steve Kargl a écrit : > On Fri, Jul 17, 2015 at 03:50:26PM +0200, Mikael Morin wrote: >> Le 17/07/2015 11:02, Mikael Morin a écrit : >>> Le 16/07/2015 16:34, Damian Rouson a écrit : >>>> Alternatively, if it?s easy, please feel free to add the directives and commit. >>>> >> >>> Never mind, I'll take care of it all. >>> >> This is what I have committed: >> https://gcc.gnu.org/r225930 (trunk) >> https://gcc.gnu.org/r225932 (5 branch) >> > > Hi Mikael, > > It seems that this commit has changed the scanning of > other testcases. > > FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&desc.., &fr, 4, _gfortran_caf_num_images \\(0, -1\\), &stat1, errmesg1, 0, 6\\);" 1 > FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&val2, &gz, 0, 4, &stat2, errmesg2, 0, 7\\);" 1 > FAIL: gfortran.dg/coarray_collectives_16.f90 -O scan-tree-dump-times original "_gfortran_caf_co_reduce \\(&desc.., &hc, 1, res, &stat3, errmesg3, 99, 8\\);" > Hmm, now you bring the topic, I realize I haven't tested the patch against the testsuite (except the new file). Will fix. Mikael ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Fortran, Patch] Passing function pointer to co_reduce 2015-07-17 19:47 ` Mikael Morin @ 2015-07-17 20:47 ` Mikael Morin 0 siblings, 0 replies; 10+ messages in thread From: Mikael Morin @ 2015-07-17 20:47 UTC (permalink / raw) To: Steve Kargl; +Cc: Damian Rouson, Alessandro Fanfarillo, gcc-patches, gfortran [-- Attachment #1: Type: text/plain, Size: 232 bytes --] Le 17/07/2015 21:39, Mikael Morin a écrit : >> It seems that this commit has changed the scanning of >> other testcases. >> > Will fix. > Done: https://gcc.gnu.org/r225965 (trunk) https://gcc.gnu.org/r225969 (5 branch) Mikael [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: r225965.diff --] [-- Type: text/x-patch; name="r225965.diff", Size: 1615 bytes --] Index: gcc/testsuite/gfortran.dg/coarray_collectives_16.f90 =================================================================== --- gcc/testsuite/gfortran.dg/coarray_collectives_16.f90 (révision 225964) +++ gcc/testsuite/gfortran.dg/coarray_collectives_16.f90 (révision 225965) @@ -33,6 +33,6 @@ contains end function hc end program test -! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&desc.., &fr, 4, _gfortran_caf_num_images \\(0, -1\\), &stat1, errmesg1, 0, 6\\);" 1 "original" } } -! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&val2, &gz, 0, 4, &stat2, errmesg2, 0, 7\\);" 1 "original" } } -! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&desc.., &hc, 1, res, &stat3, errmesg3, 99, 8\\);" 1 "original" } } +! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&desc.., fr, 4, _gfortran_caf_num_images \\(0, -1\\), &stat1, errmesg1, 0, 6\\);" 1 "original" } } +! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&val2, gz, 0, 4, &stat2, errmesg2, 0, 7\\);" 1 "original" } } +! { dg-final { scan-tree-dump-times "_gfortran_caf_co_reduce \\(&desc.., hc, 1, res, &stat3, errmesg3, 99, 8\\);" 1 "original" } } Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (révision 225964) +++ gcc/testsuite/ChangeLog (révision 225965) @@ -1,3 +1,8 @@ +2015-07-17 Mikael Morin <mikael@gcc.gnu.org> + + * gfortran.dg/coarray_collectives_16.f90: Fix pattern + as follow-up to r225930. + 2015-07-17 Paul Thomas <pault@gcc.gnu.org> PR fortran/52846 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-17 20:33 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-13 15:31 [Fortran, Patch] Passing function pointer to co_reduce Alessandro Fanfarillo 2015-07-15 13:13 ` Mikael Morin 2015-07-16 14:58 ` Damian Rouson 2015-07-17 9:28 ` Mikael Morin 2015-07-17 13:54 ` Mikael Morin 2015-07-17 14:42 ` Damian Rouson 2015-07-17 14:43 ` Alessandro Fanfarillo 2015-07-17 18:47 ` Steve Kargl 2015-07-17 19:47 ` Mikael Morin 2015-07-17 20:47 ` Mikael Morin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).