[Bug fortran/44235] array temporary with high upper bound
--- Comment #13 from tkoenig at gcc dot gnu dot org 2010-08-27 12:12 --- Fixed on trunk. Closing. -- tkoenig at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #12 from tkoenig at gcc dot gnu dot org 2010-08-09 19:35 --- Subject: Bug 44235 Author: tkoenig Date: Mon Aug 9 19:34:49 2010 New Revision: 163041 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=163041 Log: 2010-08-09 Thomas Koenig tkoe...@gcc.gnu.org PR fortran/44235 * array.c (gfc_ref_dimen_size): Add end argument. If end is non-NULL, calculate it. (ref_size): Adjust call to gfc_ref_dimen_size. (gfc_array_dimen_size): Likewise. (gfc_array_res_shape): Likewise. * gfortran.h: Adjust prototype for gfc_ref_dimen_size. * resolve.c (resolve_array_ref): For stride not equal to -1, fill in the lowest possible end. 2010-08-09 Thomas Koenig tkoe...@gcc.gnu.org PR fortran/44235 * gfortran.dg/dependency_32.f90: New test. Added: trunk/gcc/testsuite/gfortran.dg/dependency_32.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/array.c trunk/gcc/fortran/gfortran.h trunk/gcc/fortran/resolve.c trunk/gcc/fortran/simplify.c trunk/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #9 from tkoenig at gcc dot gnu dot org 2010-08-08 16:08 --- Created an attachment (id=21437) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21437action=view) Proposed patch This patch seems to work, and do more or less the right thing. It inserts the upper bound only for strides not equal to one. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #10 from dominiq at lps dot ens dot fr 2010-08-08 20:08 --- This patch seems to work, and do more or less the right thing. No counter-example of that!-) thanks. It inserts the upper bound only for strides not equal to one. I am unable to follow the code. Do you mean that you are putting the section in some canonical form with the minimal value of 'end'? What puzzle me is that I do not see any division, is it hidden in the gmp calls? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #11 from tkoenig at gcc dot gnu dot org 2010-08-08 22:40 --- (In reply to comment #10) This patch seems to work, and do more or less the right thing. No counter-example of that!-) thanks. That's good. It inserts the upper bound only for strides not equal to one. I am unable to follow the code. Do you mean that you are putting the section in some canonical form with the minimal value of 'end'? That is correct. If the stride is not equal to one, and if the size can be calculated at compile-time (which the call to gfc_ref_dimen_size checks, and does), then the end of the array reference is set to the minimum. I extended that function to also calculate the end. What puzzle me is that I do not see any division, is it hidden in the gmp calls? The division is handled in gfc_ref_dimen_size, where I simply used the size calculatd there to also calculate the minimum end. You'll find the division there. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #6 from dominiq at lps dot ens dot fr 2010-08-07 09:01 --- It turns out that the test in comment #0 was not fixed by the patch in comment #5, but by revision 162966. However with the slight change a(4:23:3) = a(4:22:3) a temporary is still created. The patch in comment #5 avoid temporaries in situations such as a(4:19:3) = a(7:22:3) a(4:20:3) = a(7:22:3) a(4:19:3) = a(7:23:3) nl = 4 nu = 20 a(1:16:3) = a(4:nu:3) a(1:16:3) = a(nl:20:3) but not for a(1:16:3) = a(nl:nu:3) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #7 from tkoenig at gcc dot gnu dot org 2010-08-07 11:17 --- (In reply to comment #6) Hi Dominique, It turns out that the test in comment #0 was not fixed by the patch in comment #5, but by revision 162966. However with the slight change a(4:23:3) = a(4:22:3) a temporary is still created. The patch in comment #5 avoid temporaries in situations such as a(4:19:3) = a(7:22:3) a(4:20:3) = a(7:22:3) a(4:19:3) = a(7:23:3) nl = 4 nu = 20 a(1:16:3) = a(4:nu:3) a(1:16:3) = a(nl:20:3) but not for a(1:16:3) = a(nl:nu:3) the only unnecessary temporary still created with current trunk is your first example. I am more in favor of changing the upper bound to its actual value, probably during resolution. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #8 from dominiq at lps dot ens dot fr 2010-08-07 13:40 --- the only unnecessary temporary still created with current trunk is your first example. Again it is because the lower bound of the section is that or the array, but if increase it I see pr44235_1_db.f90:22.14: a(4:19:3) = a(7:nu:3) 1 Warning: Creating array temporary at (1) pr44235_1_db.f90:23.14: a(4:19:3) = a(nl:23:3) 1 Warning: Creating array temporary at (1) with a clean trunk that disappear with the patch in comment #5 I am more in favor of changing the upper bound to its actual value, probably during resolution. I agree, it's what I tried to say in comment #4. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #3 from steven at gcc dot gnu dot org 2010-08-06 21:24 --- What's the plan with the patch of comment #2? NB, the result of gfc_dep_compare_expr (l_start, r_start) could be cached instead of computed twice: + ((res = gfc_dep_compare_expr (l_start, r_start)) == 0 + || res == -1) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #4 from dominiq at lps dot ens dot fr 2010-08-06 21:34 --- I am not to sure about the patch in comment #2 because the case should probably be handled by gfc_is_same_range and the patch does it in gfc_check_section_vs_section. Note that gfc_is_same_range has a line /* TODO: More sophisticated range comparison. */ In my opinion the right way to fix this pr would be to compare the starts and the extends, but so far I did not find how to do it. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #5 from dominiq at lps dot ens dot fr 2010-08-06 22:49 ---
Note that the following patch I have in my tree for some time now also fix the
pr
--- gcc/fortran/dependency.c2010-08-07 00:37:34.0 +0200
+++ ../work/gcc/fortran/dependency.c2010-08-05 19:11:58.0 +0200
@@ -1172,8 +1172,8 @@ check_section_vs_section (gfc_array_ref
/* Check for forward dependencies x:y vs. x+1:z. */
if (l_dir == 1 r_dir == 1
- l_start r_start gfc_dep_compare_expr (l_start, r_start) == -1
- l_end r_end gfc_dep_compare_expr (l_end, r_end) == -1)
+ ((l_start r_start gfc_dep_compare_expr (l_start, r_start) ==
-1)
+ || (l_end r_end gfc_dep_compare_expr (l_end, r_end) == -1)))
{
/* Check that the strides are the same. */
if (!l_stride !r_stride)
@@ -1185,8 +1185,8 @@ check_section_vs_section (gfc_array_ref
/* Check for forward dependencies x:y:-1 vs. x-1:z:-1. */
if (l_dir == -1 r_dir == -1
- l_start r_start gfc_dep_compare_expr (l_start, r_start) == 1
- l_end r_end gfc_dep_compare_expr (l_end, r_end) == 1)
+ ((l_start r_start gfc_dep_compare_expr (l_start, r_start) == 1)
+ || (l_end r_end gfc_dep_compare_expr (l_end, r_end) == 1)))
{
/* Check that the strides are the same. */
if (!l_stride !r_stride)
This followed a discussion with Tobias Burnus on IRC. It assumes that the code
is valid, i.e., lhs and rhs has the same extent, otherwise creating a temporary
does make the code valid, even if the result is not the same with or without
temporary.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #2 from dominiq at lps dot ens dot fr 2010-07-01 20:23 ---
Lightly tested patch that does not create temporaries for the test in comment
#1
--- ../_clean/gcc/fortran/dependency.c 2010-06-21 17:31:37.0 +0200
+++ gcc/fortran/dependency.c2010-07-01 22:16:51.0 +0200
@@ -1037,8 +1037,9 @@ gfc_check_section_vs_section (gfc_ref *l
/* Check for forward dependencies x:y vs. x+1:z. */
if (l_dir == 1 r_dir == 1
- l_start r_start gfc_dep_compare_expr (l_start, r_start) == -1
- l_end r_end gfc_dep_compare_expr (l_end, r_end) == -1)
+ l_start r_start (gfc_dep_compare_expr (l_start, r_start) == 0
+|| gfc_dep_compare_expr (l_start, r_start) == -1)
+ /* l_end r_end gfc_dep_compare_expr (l_end, r_end) == -1 */)
{
/* Check that the strides are the same. */
if (!l_stride !r_stride)
@@ -1050,8 +1051,9 @@ gfc_check_section_vs_section (gfc_ref *l
/* Check for forward dependencies x:y:-1 vs. x-1:z:-1. */
if (l_dir == -1 r_dir == -1
- l_start r_start gfc_dep_compare_expr (l_start, r_start) == 1
- l_end r_end gfc_dep_compare_expr (l_end, r_end) == 1)
+ l_start r_start (gfc_dep_compare_expr (l_start, r_start) == 0
+ || gfc_dep_compare_expr (l_start, r_start) == 1)
+ /* l_end r_end gfc_dep_compare_expr (l_end, r_end) == 1 */)
{
/* Check that the strides are the same. */
if (!l_stride !r_stride)
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
--- Comment #1 from tkoenig at gcc dot gnu dot org 2010-06-04 22:39 --- The particular test case from comment #1 is now fixed. Here's one that still fails: subroutine foo(a, b) real :: a(40), b(40) a(1:20:3) = a(1:19:3) a(1:19:3) = a(1:19:3) end subroutine foo g...@linux-fd1f:/tmp gfortran -Warray-temporaries -S dep2.f90 dep2.f90:3.14: a(1:20:3) = a(1:19:3) 1 Warning: Creating array temporary at (1) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
[Bug fortran/44235] array temporary with high upper bound
-- tkoenig at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |tkoenig at gcc dot gnu dot |dot org |org Status|UNCONFIRMED |ASSIGNED Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2010-05-22 08:20:04 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44235
