Created attachment 24997 [details] compile it and run Must be compiled and run. g95 and Intel ifort deliver the correct result.
Wild guess: Dependency analysis (forward/backward etc.) does not take character substrings into account. If so, it vaguely reminds me of PR 47674.
Draft patch: diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index a911a5b..7d85413 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -2452,6 +2452,9 @@ forall_make_variable_temp (gfc_code *c, stmtblock_t *pre, stmtblock_t *post) tmp = gfc_conv_array_offset (old_sym->backend_decl); gfc_conv_descriptor_offset_set (pre, tse.expr, tmp); } + else + gfc_conv_descriptor_offset_set (pre, tse.expr, + build_int_cst (gfc_array_index_type, -1)); } else {
(In reply to comment #2) > Draft patch: This patch fails for gfortran.dg/forall_12.f90 test (of PR31217 and PR33811): character(LEN=12) :: a(2) = "123456789012" forall (i = 3:10) a(:)(i:i+2) = a(:)(i-2:i) The result with the patch is 12@+@+ instead of 121234567890 * * * For the test of comment 0, gfortran (unpatched) produces: atmp.1.offset = 0; and accesses atmp.1.data)[atmp.1.offset + 1] While for the now failing test case forall_12.f90, gfortran (unpatched) produces also atmp.1.offset = 0; but uses D.1578 = atmp.1.offset; D.1577 = ... atmp.1.data; &(*D.1577)[S.5 + D.1578] where S.5 is the loop variable, which runs from 0 to 1. Thus, the actual problem seems to be that "a(1)" is translated into "a[1+offset]" instead of "a[0+offset]".
I still have the same bug on gfortran 4.8.0.
Still on gfortran 5.2.0
A am not sure if its the same bug, but I get ICE for the function !----------------------------------------------------------------------- !Function !----------------------------------------------------------------------- function reverse(string) ! bind(c, name='reverse') implicit none character(len=*), intent(in) :: string character(len=:),allocatable :: reverse reverse = string forall (i=1:len(reverse)) reverse(i:i) = reverse(len(reverse)-i+1:len(reverse)-i+1) end function reverse forall (i=1:len(reverse)) reverse(i:i) = reverse(len(reverse)-i+1:len(reverse)-i+1) 1 internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:2368 gfortran --version GNU Fortran (SUSE Linux) 5.2.1 20150721 [gcc-5-branch revision 226027]
Still in 5.3.0
Related to pr55086, but the two tests are not fixed by the patch at https://gcc.gnu.org/ml/fortran/2016-10/msg00228.html even when compiled with the option -fforce-forall-temp.
Created attachment 39979 [details] patch.55086_50069.txt The attached patch adds a slight variation of Tobias Burnus's patch for 50069 to my patch for 55086, and it seems to fix the two tests in 50069. "make check-fortran" runs with no surprises. Louis ---- On Sun, 06 Nov 2016 09:09:26 -0800 dominiq at lps dot ens.fr <gcc-bugzilla@gcc.gnu.org> wrote ---- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50069 > > Dominique d'Humieres <dominiq at lps dot ens.fr> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |lkrupp at gcc dot gnu.org > > --- Comment #8 from Dominique d'Humieres <dominiq at lps dot ens.fr> --- > Related to pr55086, but the two tests are not fixed by the patch at > https://gcc.gnu.org/ml/fortran/2016-10/msg00228.html even when compiled with > the option -fforce-forall-temp. > > -- > You are receiving this mail because: > You are on the CC list for the bug.
> The attached patch adds a slight variation of Tobias Burnus's patch > for 50069 to my patch for 55086, and it seems to fix the two tests in 50069. I have applied the patch without the last hunk. It fixes the first test by supplying the needed temporary. However compiling the test (variant of the test in comment 6) function reverse(string) ! bind(c, name='reverse') implicit none character(len=*), intent(in) :: string character(len=:),allocatable :: reverse integer :: i reverse = string forall (i=1:len(reverse)) reverse(i:i) = reverse(len(reverse)-i+1:len(reverse)-i+1) end function reverse still gives an ICE forall (i=1:len(reverse)) reverse(i:i) = reverse(len(reverse)-i+1:len(reverse)-i+1) internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:2550 The patch also fixes the ICE for the test reduced test from pr55086 implicit none character(len=5), pointer :: c, d allocate (c, d) d = '12345' c = "abcde" call test2p (d, c) print *, d if (d /= '1cb15') stop 'WRONG' contains subroutine test2p(o, i) character(len=*), pointer :: o, i integer :: nl1, nu1 integer :: i1 nl1 = 2 nu1 = 4 forall (i1 = nl1:nu1) o(i1:i1) = i(i1:i1) ! <<<< ICE forall (i1 = nl1:nu1) o(i1:i1) = o(nu1+1-i1:nu1+1-i1) end subroutine test2p end but at the expense of an unneeded temporary: { integer(kind=4) i1.0; integer(kind=4) D.3446; integer(kind=8) count1.1; integer(kind=8) num.2; character(kind=1)[0:][1:1] * temp.4; void * restrict D.3452; D.3446 = nu1; count1.1 = 0; num.2 = 0; { integer(kind=4) count.3; i1.0 = nl1; count.3 = (1 - nl1) + nu1; while (1) { if (count.3 <= 0) goto L.1; num.2 = num.2 + 1; i1.0 = i1.0 + 1; count.3 = count.3 + -1; } L.1:; } D.3452 = (void * restrict) __builtin_malloc (MAX_EXPR <(unsigned long) num.2, 1>); temp.4 = (character(kind=1)[0:][1:1] *) D.3452; { integer(kind=4) count.5; i1.0 = nl1; count.5 = (1 - nl1) + nu1; while (1) { if (count.5 <= 0) goto L.2; (*temp.4)[count1.1][1]{lb: 1 sz: 1} = (**i)[i1.0]{lb: 1 sz: 1}; count1.1 = count1.1 + 1; i1.0 = i1.0 + 1; count.5 = count.5 + -1; } L.2:; } count1.1 = 0; { integer(kind=4) count.6; i1.0 = nl1; count.6 = (1 - nl1) + nu1; while (1) { if (count.6 <= 0) goto L.3; (**o)[i1.0]{lb: 1 sz: 1} = (*temp.4)[count1.1][1]{lb: 1 sz: 1}; count1.1 = count1.1 + 1; i1.0 = i1.0 + 1; count.6 = count.6 + -1; } L.3:; } __builtin_free ((void *) temp.4); }
Created attachment 39997 [details] patch.55086_50069-1.txt You're right. I wasn't paying attention to the third ("function reverse ...") in the bug report. I believe that's fixed with the attached patch to trans-expr.c along with my other patch. I no longer see the ICE, and "make check-fortran" turns up no surprises. I haven't tried this second patch (to trans-expr.c) by itself. The force-forall-temp flag in the first patch isn't intended to fix anything; its sole purpose is to get code with obscure bugs to fail sooner, in testing, instead of later when forall really needs to create and use a temporary under circumstances that no one had anticipated. Louis ---- On Tue, 08 Nov 2016 07:03:59 -0800 dominiq at lps dot ens.fr <gcc-bugzilla@gcc.gnu.org> wrote ---- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50069 > > --- Comment #10 from Dominique d'Humieres <dominiq at lps dot ens.fr> --- > > The attached patch adds a slight variation of Tobias Burnus's patch > > for 50069 to my patch for 55086, and it seems to fix the two tests in 50069. > > I have applied the patch without the last hunk. It fixes the first test by > supplying the needed temporary. However compiling the test (variant of the test > in comment 6) > > function reverse(string) ! bind(c, name='reverse') > implicit none > character(len=*), intent(in) :: string > character(len=:),allocatable :: reverse > integer :: i > reverse = string > forall (i=1:len(reverse)) reverse(i:i) = > reverse(len(reverse)-i+1:len(reverse)-i+1) > end function reverse > > still gives an ICE > > forall (i=1:len(reverse)) reverse(i:i) = > reverse(len(reverse)-i+1:len(reverse)-i+1) > > internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:2550 > > The patch also fixes the ICE for the test reduced test from pr55086 > > implicit none > character(len=5), pointer :: c, d > allocate (c, d) > > d = '12345' > c = "abcde" > > call test2p (d, c) > print *, d > if (d /= '1cb15') stop 'WRONG' > > contains > subroutine test2p(o, i) > character(len=*), pointer :: o, i > integer :: nl1, nu1 > integer :: i1 > nl1 = 2 > nu1 = 4 > forall (i1 = nl1:nu1) o(i1:i1) = i(i1:i1) ! <<<< ICE > forall (i1 = nl1:nu1) o(i1:i1) = o(nu1+1-i1:nu1+1-i1) > end subroutine test2p > end > > but at the expense of an unneeded temporary: > > { > integer(kind=4) i1.0; > integer(kind=4) D.3446; > integer(kind=8) count1.1; > integer(kind=8) num.2; > character(kind=1)[0:][1:1] * temp.4; > void * restrict D.3452; > > D.3446 = nu1; > count1.1 = 0; > num.2 = 0; > { > integer(kind=4) count.3; > > i1.0 = nl1; > count.3 = (1 - nl1) + nu1; > while (1) > { > if (count.3 <= 0) goto L.1; > num.2 = num.2 + 1; > i1.0 = i1.0 + 1; > count.3 = count.3 + -1; > } > L.1:; > } > D.3452 = (void * restrict) __builtin_malloc (MAX_EXPR <(unsigned long) > num.2, 1>); > temp.4 = (character(kind=1)[0:][1:1] *) D.3452; > { > integer(kind=4) count.5; > > i1.0 = nl1; > count.5 = (1 - nl1) + nu1; > while (1) > { > if (count.5 <= 0) goto L.2; > (*temp.4)[count1.1][1]{lb: 1 sz: 1} = (**i)[i1.0]{lb: 1 sz: 1}; > count1.1 = count1.1 + 1; > i1.0 = i1.0 + 1; > count.5 = count.5 + -1; > } > L.2:; > } > count1.1 = 0; > { > integer(kind=4) count.6; > > i1.0 = nl1; > count.6 = (1 - nl1) + nu1; > while (1) > { > if (count.6 <= 0) goto L.3; > (**o)[i1.0]{lb: 1 sz: 1} = (*temp.4)[count1.1][1]{lb: 1 sz: 1}; > count1.1 = count1.1 + 1; > i1.0 = i1.0 + 1; > count.6 = count.6 + -1; > } > L.3:; > } > __builtin_free ((void *) temp.4); > } > > -- > You are receiving this mail because: > You are on the CC list for the bug.
Author: lkrupp Date: Wed Jan 18 21:41:48 2017 New Revision: 244601 URL: https://gcc.gnu.org/viewcvs?rev=244601&root=gcc&view=rev Log: 2017-01-18 Louis Krupp <louis.krupp@zoho.com> PR fortran/50069 PR fortran/55086 * gfortran.dg/pr50069_1.f90: New test. * gfortran.dg/pr50069_2.f90: New test. * gfortran.dg/pr55086_1.f90: New test. * gfortran.dg/pr55086_1_tfat.f90: New test. * gfortran.dg/pr55086_2.f90: New test. * gfortran.dg/pr55086_2_tfat.f90: New test. * gfortran.dg/pr55086_aliasing_dummy_4_tfat.f90: New test. 2017-01-18 Louis Krupp <louis.krupp@zoho.com> PR fortran/50069 PR fortran/55086 * trans-expr.c (gfc_conv_variable): Don't treat temporary variables as function arguments. * trans-stmt.c (forall_make_variable_temp, generate_loop_for_temp_to_lhs, gfc_trans_assign_need_temp, gfc_trans_forall_1): Don't adjust offset of forall temporary for array sections, make forall temporaries work for substring expressions, improve test coverage by adding -ftest-forall-temp option to request usage of temporary array in forall code. * lang.opt: Add -ftest-forall-temp option. * invoke.texi: Add -ftest-forall-temp option. Added: trunk/gcc/testsuite/gfortran.dg/pr50069_1.f90 trunk/gcc/testsuite/gfortran.dg/pr50069_2.f90 trunk/gcc/testsuite/gfortran.dg/pr55086_1.f90 trunk/gcc/testsuite/gfortran.dg/pr55086_1_tfat.f90 trunk/gcc/testsuite/gfortran.dg/pr55086_2.f90 trunk/gcc/testsuite/gfortran.dg/pr55086_2_tfat.f90 trunk/gcc/testsuite/gfortran.dg/pr55086_aliasing_dummy_4_tfat.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/invoke.texi trunk/gcc/fortran/lang.opt trunk/gcc/fortran/trans-expr.c trunk/gcc/fortran/trans-stmt.c trunk/gcc/testsuite/ChangeLog
Fixed in revision 244601. Offset adjustment for temporary arrays is fixed, or at least improved. Temporary arrays created for function return values are treated as variables and not as functions when passed as procedure arguments.