[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 anlauf at gcc dot gnu.org changed: What|Removed |Added Resolution|--- |FIXED Status|REOPENED|RESOLVED --- Comment #24 from anlauf at gcc dot gnu.org --- I didn't see any testsuite failures left. Closing again. (Fingers crossed.)
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #23 from Jerry DeLisle --- John, Your original case in Comment 1 now gives: $ gfc original.f90 $ ./a.out tstuff fstuff T tstuff fstuff F So I think it is OK, Harald's test case has function calls embedded in the print statements within the main program and those functions have embedded print statements themselves to the same output device which is stdout and this is not allowed per the Fortran standard. So the issue is only in the test case he previously committed and it is now fixed.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #22 from CVS Commits --- The master branch has been updated by Harald Anlauf : https://gcc.gnu.org/g:36a4ee406b95ae24a59b8b3f8ebe29af6fd5261e commit r13-4473-g36a4ee406b95ae24a59b8b3f8ebe29af6fd5261e Author: Harald Anlauf Date: Fri Dec 2 22:30:16 2022 +0100 Fortran: intrinsic MERGE shall use all its arguments [PR107874] gcc/testsuite/ChangeLog: PR fortran/107874 * gfortran.dg/merge_1.f90: Avoid recursive I/O.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #21 from john.harper at vuw dot ac.nz --- I now have a new test case that avoids the possibility of recursive I/O by tstuff and fstuff doing internal writes to two different character variables. It still reveals the merge problem. It compiles and runs with gfortran-12 and ifort, giving different outputs. ! Must merge evaluate all 3 arguments? program testmerge10 implicit none character(7):: string(2) = ' ' logical:: x(2) = [.true., .false.], y integer i do i = 1,2 y = merge(tstuff(),fstuff(),x(i)) print *,y,string string = ' ' end do contains logical function tstuff() write(string(1),"(A)") ' tstuff' tstuff = .true. end function tstuff logical function fstuff() write(string(2),"(A)") ' fstuff' fstuff = .false. end function fstuff end program testmerge10 Good luck with your bughunt! John H On Fri, 2 Dec 2022, anlauf at gcc dot gnu.org wrote: > Date: Fri, 2 Dec 2022 21:05:43 + > From: anlauf at gcc dot gnu.org > To: John Harper > Subject: [Bug fortran/107874] merge not using all its arguments > Resent-Date: Sat, 3 Dec 2022 10:05:53 +1300 (NZDT) > Resent-From: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 > > --- Comment #19 from anlauf at gcc dot gnu.org --- > (In reply to john.harper from comment #18) >> An interesting problem! But I thought my original test case did not have >> recursive I/O because tstuff and fstuff each print something in the >> statement >> y = merge(tstuff(),fstuff(),x(i)) >> but y itself is printed only in the next statement, >> print *,y > > John, your original testcase in comment#0 was fine. > I tried to extend it to check for constant as well as non-constant mask, > and as you see I made a mistake by trying to make it smaller. Bad idea. > >> Or does evaluating merge allow each of tstuff and fstuff to be evaluated >> at the same time? I was thinking of tstuff and fstuff being evaluated >> in succession but could there be systems in which they are evaluated >> simultaneously? > > I don't recall having seen a mentioning in the standard of the order of > evaluation of different function (or subroutine) arguments. Do you? > > (Of course, if side-effects happen during that evaluation, such as I/O, > unexpected things may happen.) > >> If so, whether the program is valid Fortran depends on the >> kind of system on which it is being executed. > > Well, even if the print in tstuff/fstuff were a problem, one could construct > other testcases with side-effects that might be conforming. > > -- > You are receiving this mail because: > You reported the bug. > -- John Harper, School of Mathematics and Statistics Victoria Univ. of Wellington, PO Box 600, Wellington 6140, New Zealand. e-mail john.har...@vuw.ac.nz phone +64(0) 4 463 5276
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #20 from anlauf at gcc dot gnu.org --- (In reply to anlauf from comment #19) > I don't recall having seen a mentioning in the standard of the order of > evaluation of different function (or subroutine) arguments. Do you? The closest I can find is the following in F2018: C.5.1 Evaluation of function references (10.1.7) (1) If more than one function reference appears in a statement, they can be executed in any order (subject to a function result being evaluated after the evaluation of its arguments) and their values cannot depend on the order of execution. This lack of dependence on order of evaluation enables parallel execution of the function references. And: A.2 Processor dependencies According to this document, the following are processor dependent: ... • the order of finalization of components of objects of derived type (7.5.6.2) • the order of finalization when several objects are finalized as the consequence of a single event (7.5.6.2);
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #19 from anlauf at gcc dot gnu.org --- (In reply to john.harper from comment #18) > An interesting problem! But I thought my original test case did not have > recursive I/O because tstuff and fstuff each print something in the > statement > y = merge(tstuff(),fstuff(),x(i)) > but y itself is printed only in the next statement, > print *,y John, your original testcase in comment#0 was fine. I tried to extend it to check for constant as well as non-constant mask, and as you see I made a mistake by trying to make it smaller. Bad idea. > Or does evaluating merge allow each of tstuff and fstuff to be evaluated > at the same time? I was thinking of tstuff and fstuff being evaluated > in succession but could there be systems in which they are evaluated > simultaneously? I don't recall having seen a mentioning in the standard of the order of evaluation of different function (or subroutine) arguments. Do you? (Of course, if side-effects happen during that evaluation, such as I/O, unexpected things may happen.) > If so, whether the program is valid Fortran depends on the > kind of system on which it is being executed. Well, even if the print in tstuff/fstuff were a problem, one could construct other testcases with side-effects that might be conforming.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #18 from john.harper at vuw dot ac.nz --- An interesting problem! But I thought my original test case did not have recursive I/O because tstuff and fstuff each print something in the statement y = merge(tstuff(),fstuff(),x(i)) but y itself is printed only in the next statement, print *,y Or does evaluating merge allow each of tstuff and fstuff to be evaluated at the same time? I was thinking of tstuff and fstuff being evaluated in succession but could there be systems in which they are evaluated simultaneously? If so, whether the program is valid Fortran depends on the kind of system on which it is being executed. John Harper On Fri, 2 Dec 2022, anlauf at gcc dot gnu.org wrote: > Date: Fri, 2 Dec 2022 20:22:23 + > From: anlauf at gcc dot gnu.org > To: John Harper > Subject: [Bug fortran/107874] merge not using all its arguments > Resent-Date: Sat, 3 Dec 2022 09:22:36 +1300 (NZDT) > Resent-From: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 > > --- Comment #17 from anlauf at gcc dot gnu.org --- > (In reply to Jerry DeLisle from comment #16) >> (In reply to anlauf from comment #15) >> --- snip --- >>> Can you please verify? >> >> Yes, this fixes the test case. > > OK, thanks for confirming. > >> However if the orginal test case is valid >> fortran we probably need to fix something else. Paul Thomas was noticing a >> similar problem with his Finalization patches. He was doing the >> finalization inside trans_transfer or similar so it was blocking on a mutex >> trying to finalize in the middle of an I/O operation. >> >> So in this case, my guess is the merge expression needs to be resolved >> before the translation phase. > > If I interprete the tree-dump correctly, we have e.g.: > > _gfortran_st_write (_parm.2); > { >logical(kind=4) D.4279; >logical(kind=4) D.4280; >logical(kind=4) D.4281; >logical(kind=4) D.4282; > >D.4279 = tstuff (); >D.4280 = fstuff (); >D.4281 = x[(integer(kind=8)) i + -1]; >D.4282 = D.4281 ? D.4279 : D.4280; >_gfortran_transfer_logical_write (_parm.2, , 4); > } > _gfortran_st_write_done (_parm.2); > > so we start the write, then evaluate the merge(), which is tstuff()/fstuff(), > and which does its own I/O, and then return to continue with the transfer. > > So this might be non-conforming code, see > > F2018:12.12 Restrictions on input/output statements > > (2) An input/output statement that is executed while another input/output > statement is being executed is a recursive input/output statement. > A recursive input/output statement shall not identify an external unit that > is identified by another input/output statement being executed except that > a child data transfer statement may identify its parent data transfer > statement external unit. > > I am not sure I fully understand the last sentence in this paragraph, > but I think the pushed testcase might be invalid and should be replaced. > > If you agree, I'll simply do this. > > -- > You are receiving this mail because: > You reported the bug. > -- John Harper, School of Mathematics and Statistics Victoria Univ. of Wellington, PO Box 600, Wellington 6140, New Zealand. e-mail john.har...@vuw.ac.nz phone +64(0) 4 463 5276
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #17 from anlauf at gcc dot gnu.org --- (In reply to Jerry DeLisle from comment #16) > (In reply to anlauf from comment #15) > --- snip --- > > Can you please verify? > > Yes, this fixes the test case. OK, thanks for confirming. > However if the orginal test case is valid > fortran we probably need to fix something else. Paul Thomas was noticing a > similar problem with his Finalization patches. He was doing the > finalization inside trans_transfer or similar so it was blocking on a mutex > trying to finalize in the middle of an I/O operation. > > So in this case, my guess is the merge expression needs to be resolved > before the translation phase. If I interprete the tree-dump correctly, we have e.g.: _gfortran_st_write (_parm.2); { logical(kind=4) D.4279; logical(kind=4) D.4280; logical(kind=4) D.4281; logical(kind=4) D.4282; D.4279 = tstuff (); D.4280 = fstuff (); D.4281 = x[(integer(kind=8)) i + -1]; D.4282 = D.4281 ? D.4279 : D.4280; _gfortran_transfer_logical_write (_parm.2, , 4); } _gfortran_st_write_done (_parm.2); so we start the write, then evaluate the merge(), which is tstuff()/fstuff(), and which does its own I/O, and then return to continue with the transfer. So this might be non-conforming code, see F2018:12.12 Restrictions on input/output statements (2) An input/output statement that is executed while another input/output statement is being executed is a recursive input/output statement. A recursive input/output statement shall not identify an external unit that is identified by another input/output statement being executed except that a child data transfer statement may identify its parent data transfer statement external unit. I am not sure I fully understand the last sentence in this paragraph, but I think the pushed testcase might be invalid and should be replaced. If you agree, I'll simply do this.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #16 from Jerry DeLisle --- (In reply to anlauf from comment #15) --- snip --- > Can you please verify? Yes, this fixes the test case. However if the orginal test case is valid fortran we probably need to fix something else. Paul Thomas was noticing a similar problem with his Finalization patches. He was doing the finalization inside trans_transfer or similar so it was blocking on a mutex trying to finalize in the middle of an I/O operation. So in this case, my guess is the merge expression needs to be resolved before the translation phase.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #15 from anlauf at gcc dot gnu.org --- Created attachment 54006 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54006=edit Modified testcase I found that I get a hang on my system when I specify -fopenmp. It appears that there is a deadlock due to the evaluation of the merge arguments during the print. So changing the testcase as attached might fix the problem. Can you please verify?
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #14 from Jerry DeLisle --- Now this is interesting. Compling with -static I get: $ gfc -static merge_1.f90 [jerry@r7laptop merge]$ ./a.out tstuff fstuff T tstuff fstuff F tstuff fstuff T tstuff fstuff F tstuff T tstuff F fstuff T fstuff F So there is something wrong with the linking as it hangs without -static $ echo $LD_LIBRARY_PATH /home/jerry/dev/usr/lib64/:/home/jerry/dev/usr/lib/ $ gfc -v Using built-in specs. COLLECT_GCC=gfc COLLECT_LTO_WRAPPER=/home/jerry/dev/usr/libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: ../trunk/configure --prefix=/home/jerry/dev/usr --enable-languages=c,c++,fortran --enable-libgomp --disable-bootstrap Thread model: posix Supported LTO compression algorithms: zlib gcc version 13.0.0 20221201 (experimental) (GCC) [jerry@r7laptop bin]$ ls -l gfc lrwxrwxrwx. 1 jerry jerry 32 Nov 20 15:50 gfc -> /home/jerry/dev/usr/bin/gfortran Am I doing something wrong? Also Valgrind does show some issues, but one can not be sure.: $ gfc -static merge_1.f90 [jerry@r7laptop merge]$ valgrind --leak-check=full --show-leak-kinds=all ./a.out ==17321== Memcheck, a memory error detector ==17321== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==17321== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info ==17321== Command: ./a.out ==17321== ==17321== Syscall param set_robust_list(head) points to uninitialised byte(s) ==17321==at 0x48848A: __tls_init_tp (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x439361: __libc_setup_tls (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x438630: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== Address 0x40006b0 is in the brk data segment 0x400-0x4000d3f ==17321== ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x460387: malloc (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x489416: _dl_non_dynamic_init (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x48ABE8: __libc_init_first (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x460459: malloc (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x489416: _dl_non_dynamic_init (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x48ABE8: __libc_init_first (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x45EFBB: _int_malloc (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x45FC34: tcache_init.part.0 (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x460463: malloc (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4B3B9A: _dl_get_origin (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x489416: _dl_non_dynamic_init (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x48ABE8: __libc_init_first (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x4757D4: __strcspn_sse42 (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4B8CFC: strsep (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4AEE1E: fillin_rpath.isra.0 (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4AF52C: _dl_init_paths (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x489A3B: _dl_non_dynamic_init (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x48ABE8: __libc_init_first (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x4757B2: __strcspn_sse42 (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4B8CFC: strsep (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4AEE1E: fillin_rpath.isra.0 (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4AF52C: _dl_init_paths (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x489A3B: _dl_non_dynamic_init (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x48ABE8: __libc_init_first (in /home/jerry/dev/test/merge/a.out) ==17321==by 0x4386A8: (below main) (in /home/jerry/dev/test/merge/a.out) ==17321== tstuff fstuff T tstuff fstuff F tstuff fstuff T tstuff fstuff F tstuff T tstuff F fstuff T fstuff F ==17321== Conditional jump or move depends on uninitialised value(s) ==17321==at 0x45B3D9: __libc_cleanup_push_defer (in /home/jerry/dev/test/merge/a.out) ==17321==by
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #13 from Jerry DeLisle --- A debug session gives: (gdb) c Continuing. ^C Program received signal SIGINT, Interrupt. futex_wait (private=0, expected=2, futex_word=0x405950) at ../sysdeps/nptl/futex-internal.h:146 146 int err = lll_futex_timed_wait (futex_word, expected, NULL, private); (gdb) bt #0 futex_wait (private=0, expected=2, futex_word=0x405950) at ../sysdeps/nptl/futex-internal.h:146 #1 __GI___lll_lock_wait (futex=futex@entry=0x405950, private=0) at lowlevellock.c:49 #2 0x779d1432 in lll_mutex_lock_optimized (mutex=0x405950) at pthread_mutex_lock.c:48 #3 ___pthread_mutex_lock (mutex=0x405950) at pthread_mutex_lock.c:93 #4 0x77e73503 in __gthread_mutex_lock (__mutex=0x405950) at ../libgcc/gthr-default.h:749 #5 get_gfc_unit (n=6, do_create=1) at ../../../trunk/libgfortran/io/unit.c:395 #6 0x77e71b85 in data_transfer_init (dtp=0x7fffd9d0, read_flag=0) at ../../../trunk/libgfortran/io/transfer.c:3007 #7 0x004017dd in testmerge9::tstuff () at merge_1.f90:39 #8 0x0040128c in testmerge9 () at merge_1.f90:14 #9 0x0040184f in main (argc=1, argv=0x7fffe2c4) at merge_1.f90:36 #10 0x7796a510 in __libc_start_call_main ( main=main@entry=0x40181b , argc=argc@entry=1, argv=argv@entry=0x7fffdfa8) at ../sysdeps/nptl/libc_start_call_main.h:58 #11 0x7796a5c9 in __libc_start_main_impl (main=0x40181b , argc=1, argv=0x7fffdfa8, init=, fini=, rtld_fini=, stack_end=0x7fffdf98) at ../csu/libc-start.c:381 I would speculate we are trashing some memory since there is no reason to hang in there.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 Jerry DeLisle changed: What|Removed |Added CC||jvdelisle at gcc dot gnu.org --- Comment #12 from Jerry DeLisle --- Created attachment 54003 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54003=edit -fdump-tree-original output I get no output at runtime. The program simply hangs and does nothing. This is on: $ uname -a Linux r7laptop 6.0.10-300.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Nov 26 16:55:13 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #11 from anlauf at gcc dot gnu.org --- And this is the output I'm getting on stdout: tstuff fstuff T tstuff fstuff F tstuff fstuff T tstuff fstuff F tstuff T tstuff F fstuff T fstuff F Can you compare your output with the above to narrow down what might be wrong?
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 anlauf at gcc dot gnu.org changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #10 from anlauf at gcc dot gnu.org --- Hmmm, I do not see the failures here, but gcc-testresults also shows them. Can you please attach a tree-dump?
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #9 from Jerry DeLisle --- I am seeing timeout failures with merge_1.f90. Can you look into this? Thanks, Jerry On Thu, Dec 1, 2022, 12:28 PM anlauf at gcc dot gnu.org via Gcc-bugs < gcc-bugs@gcc.gnu.org> wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 > > anlauf at gcc dot gnu.org changed: > >What|Removed |Added > > >Target Milestone|--- |13.0 > Resolution|--- |FIXED > Status|ASSIGNED|RESOLVED > > --- Comment #8 from anlauf at gcc dot gnu.org --- > Fixed on mainline for gcc-13. Closing. > > Thanks for the report!
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 anlauf at gcc dot gnu.org changed: What|Removed |Added Target Milestone|--- |13.0 Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #8 from anlauf at gcc dot gnu.org --- Fixed on mainline for gcc-13. Closing. Thanks for the report!
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #7 from CVS Commits --- The master branch has been updated by Harald Anlauf : https://gcc.gnu.org/g:3832c6f7e672e76bba74a508bf3a49740ea38046 commit r13-4394-g3832c6f7e672e76bba74a508bf3a49740ea38046 Author: Harald Anlauf Date: Mon Nov 28 20:43:02 2022 +0100 Fortran: intrinsic MERGE shall use all its arguments [PR107874] gcc/fortran/ChangeLog: PR fortran/107874 * simplify.cc (gfc_simplify_merge): When simplifying MERGE with a constant scalar MASK, ensure that arguments TSOURCE and FSOURCE are either constant or will be evaluated. * trans-intrinsic.cc (gfc_conv_intrinsic_merge): Evaluate arguments before generating conditional expression. gcc/testsuite/ChangeLog: PR fortran/107874 * gfortran.dg/merge_init_expr_2.f90: Adjust code to the corrected simplification. * gfortran.dg/merge_1.f90: New test. Co-authored-by: Steven G. Kargl
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 anlauf at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |anlauf at gcc dot gnu.org --- Comment #6 from anlauf at gcc dot gnu.org --- Submitted: https://gcc.gnu.org/pipermail/fortran/2022-November/058558.html Taking.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #5 from Steve Kargl --- On Sun, Nov 27, 2022 at 08:00:35PM +, anlauf at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 > > --- Comment #3 from anlauf at gcc dot gnu.org --- > (In reply to kargl from comment #2) > > Harald, you are likely right the patch can be moved down. I'll programmed > > up the example from the Fortran 2018 standard, which works as expected. So, > > there is definitely something about a scalar mask choosing the actual > > argument before both are evaluated. > > > >program foo > > Steve, > > this example from the standard seems to be working down to 7.5 for me. > Am I missing something? Do we need this in the testsuite? You are not missing anything. I wanted an example that works with or without the patch John included, so that we don't accidently introduce a regression. > I'd say it's rather the following two lines replacing the loop in the > reproducer in comment#0: > > print *, merge(tstuff(),fstuff(),.true.) > print *, merge(tstuff(),fstuff(),.false.) > > This is mis-simplified in simplify.cc:4909 Good find! This may indeed be a source of the issue.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #4 from anlauf at gcc dot gnu.org --- The following patch fixes comment#3: diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 9c2fea8c5f2..2f69c4369ab 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -4913,6 +4914,11 @@ gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) if (mask->expr_type == EXPR_CONSTANT) { + /* The standard requires evaluation of all function arguments. +Simplify only when TSOURCE, FSOURCE are constant expressions. */ + if (!gfc_is_constant_expr (tsource) || !gfc_is_constant_expr (fsource)) + return NULL; + result = gfc_copy_expr (mask->value.logical ? tsource : fsource); /* Parenthesis is needed to get lower bounds of 1. */ result = gfc_get_parentheses (result); This leads to a "regression" for gfortran.dg/merge_init_expr_2.f90, which is due to the pattern matching the old, faulty simplification result. That's trivial to fix, though.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 --- Comment #3 from anlauf at gcc dot gnu.org --- (In reply to kargl from comment #2) > Harald, you are likely right the patch can be moved down. I'll programmed > up the example from the Fortran 2018 standard, which works as expected. So, > there is definitely something about a scalar mask choosing the actual > argument before both are evaluated. > >program foo Steve, this example from the standard seems to be working down to 7.5 for me. Am I missing something? Do we need this in the testsuite? I'd say it's rather the following two lines replacing the loop in the reproducer in comment#0: print *, merge(tstuff(),fstuff(),.true.) print *, merge(tstuff(),fstuff(),.false.) This is mis-simplified in simplify.cc:4909 gfc_expr * gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) { gfc_expr * result; gfc_constructor *tsource_ctor, *fsource_ctor, *mask_ctor; if (mask->expr_type == EXPR_CONSTANT) { result = gfc_copy_expr (mask->value.logical ? tsource : fsource); /* Parenthesis is needed to get lower bounds of 1. */ result = gfc_get_parentheses (result); gfc_simplify_expr (result, 1); return result; } So unless tsource and fsource are both constant, we have to give up here.
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 kargl at gcc dot gnu.org changed: What|Removed |Added CC||kargl at gcc dot gnu.org --- Comment #2 from kargl at gcc dot gnu.org --- John thanks for posting the bug. Harald, you are likely right the patch can be moved down. I'll programmed up the example from the Fortran 2018 standard, which works as expected. So, there is definitely something about a scalar mask choosing the actual argument before both are evaluated. program foo call bah contains subroutine bah logical, parameter :: t = .true., f = .false. logical, parameter :: mask(2,3)= reshape([t, f, f, f, t, t], [2,3]) integer tsrc(2,3), fsrc(2,3), res(2,3) ! ! Direct reference to merge ! tsrc = reshape([1, 2, 6, 4, 5, 6], [2,3]) fsrc = reshape([0, 7, 3, 4, 2, 8], [2,3]) res = 42 res = merge(tsrc, fsrc, mask) write(*,'(*(I0,1X))') res(1,:) ! Should be 1 3 5 write(*,'(*(I0,1X))') res(2,:) ! Should be 7 4 6 write(*,*) ! ! Load matrices via a function. ! res = 0 ! Clear res = merge(load('t'), load('f'), mask) ! Clear write(*,'(*(I0,1X))') res(1,:) ! Should be 1 3 5 write(*,'(*(I0,1X))') res(2,:) ! Should be 7 4 6 end subroutine bah function load(c) result(r) integer r(2,3) character, intent(in) :: c if (c == 't') then r = reshape([1, 2, 6, 4, 5, 6], [2,3]) print *, 'loaded t' else if (c == 'f') then r = reshape([0, 7, 3, 4, 2, 8], [2,3]) print *, 'loaded f' else stop 'whoops' end if end function load end program foo
[Bug fortran/107874] merge not using all its arguments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107874 anlauf at gcc dot gnu.org changed: What|Removed |Added Keywords||wrong-code CC||anlauf at gcc dot gnu.org See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=105371 Ever confirmed|0 |1 Last reconfirmed||2022-11-26 Status|UNCONFIRMED |NEW --- Comment #1 from anlauf at gcc dot gnu.org --- Confirmed. The patch by Steve looks good, but I think it needs to be moved down slightly so that it also handles character. With the patch, a few cases that are discussed in pr105371 are also fixed. Still missing is a fix for constant mask argument that we mishandle during simplification.