Re: [committed] - Re: [patch] OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1 [PR107424]
Hi Tobias!
On 2023-07-19T10:26:12+0200, Tobias Burnus wrote:
> Now committed as Rev. r14-2634-g85da0b40538fb0
On devel/omp/gcc-13 branch, the corresponding
commit b003e6511754dce475f7f5b0c5cd887a177e41b3
"OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1
[PR107424]"
introduces a regression:
PASS: libgomp.fortran/loop-transforms/unroll-2.f90 -O0 (test for excess
errors)
[-PASS:-]{+FAIL:+} libgomp.fortran/loop-transforms/unroll-2.f90 -O0
execution test
Etc.
spawn [open ...]
4
8
10
11
Program aborted. Backtrace:
#0 0x400f9c in test
at [...]/libgomp.fortran/loop-transforms/unroll-2.f90:85
#1 0x400fd3 in main
at [...]/libgomp.fortran/loop-transforms/unroll-2.f90:59
Grüße
Thomas
> Changes:
>
> * I missed to updated another 'sorry' (msg wording change) - now fixed;
> I also added it to the sorry-testcase file non-rectangular-loop-5.f90.
>
> * I decided to retire the PR as several issues have been fixed and the
> original title did not fit any more. The remaining issue is now tracked
> in PR110735 (i.e. handling step != const, both the generic and possibly
> a simpler special case).
>
> * I added a link to the PR to libgomp.texi such that one can find out
> what is only partially supported for Fortran.
>
> Thanks,
>
> Tobias
>
> PS: Otherwise, the following still applies:
>
> On 18.07.23 14:11, Tobias Burnus wrote:
>> Comments regarding the validity of the Fortran assumptions are welcome!
>>
>> This patch now uses a 'simple' loop for OpenMP loops with
>> a constant loop-step size. Before, it only did so for step = ±1.
>> (Otherwise, a count variable is used from which the original
>> loop index variable is calculated from.)
>>
>> For details, see the attached patch or
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424#c12
>> (comment 12 + 14 plus the email linked in comment 12).
>>
>> Comments? Remarks? If there are none, I will relatively soonish
>> commit the attached patch to mainline, only.
> commit 85da0b40538fb0d17d89de1e7905984668e3dfef
> Author: Tobias Burnus
> Date: Wed Jul 19 10:18:49 2023 +0200
>
> OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or
> -1 [PR107424]
>
> Before this commit, gfortran produced with OpenMP for 'do i = 1,10,2'
> the code
> for (count.0 = 0; count.0 < 5; count.0 = count.0 + 1)
> i = count.0 * 2 + 1;
>
> While such an inner loop can be collapsed, a non-rectangular could not.
> With this commit and for all constant loop steps, a simple loop such
> as 'for (i = 1; i <= 10; i = i + 2)' is created. (Before only for the
> constant steps of 1 and -1.)
>
> The constant step permits to know the direction (increasing/decreasing)
> that is required for the loop condition.
>
> The new code is only valid if one assumes no overflow of the loop
> variable.
> However, the Fortran standard can be read that this must be ensured by
> the user. Namely, the Fortran standard requires (F2023, 10.1.5.2.4):
> "The execution of any numeric operation whose result is not defined by
> the arithmetic used by the processor is prohibited."
>
> And, for DO loops, F2023's "11.1.7.4.3 The execution cycle" has the
> following: The number of loop iterations handled by an iteration count,
> which would permit code like 'do i = huge(i)-5, huge(i),4'. However,
> in step (3), this count is not only decremented by one but also:
> "... The DO variable, if any, is incremented by the value of the
> incrementation parameter m3."
> And for the example above, 'i' would be 'huge(i)+3' in the last
> execution cycle, which exceeds the largest model number and should
> render the example as invalid.
>
> PR fortran/107424
>
> gcc/fortran/ChangeLog:
>
> * trans-openmp.cc (gfc_nonrect_loop_expr): Accept all
> constant loop steps.
> (gfc_trans_omp_do): Likewise; use sign to determine
> loop direction.
>
> libgomp/ChangeLog:
>
> * libgomp.texi (Impl. Status 5.0): Add link to new PR110735.
> * testsuite/libgomp.fortran/non-rectangular-loop-1.f90: Enable
> commented tests.
> * testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: Remove
> test file; tests are in non-rectangular-loop-1.f90.
> * testsuite/libgomp.fortran/non-rectangular-loop-5.f90: Change
> testcase to use a non-constant step to retain the 'sorry' test.
> * testsuite/libgomp.fortran/non-rectangular-loop-6.f90: New test.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/gomp/linear-2.f90: Update dump to remove
> the additional count variable.
> ---
> gcc/fortran/trans-openmp.cc| 18 +-
> gcc/testsuite/gfortran.dg/gomp/linear-2.f90| 4 +-
>
[committed] - Re: [patch] OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1 [PR107424]
Now committed as Rev. r14-2634-g85da0b40538fb0 Changes: * I missed to updated another 'sorry' (msg wording change) - now fixed; I also added it to the sorry-testcase file non-rectangular-loop-5.f90. * I decided to retire the PR as several issues have been fixed and the original title did not fit any more. The remaining issue is now tracked in PR110735 (i.e. handling step != const, both the generic and possibly a simpler special case). * I added a link to the PR to libgomp.texi such that one can find out what is only partially supported for Fortran. Thanks, Tobias PS: Otherwise, the following still applies: On 18.07.23 14:11, Tobias Burnus wrote: Comments regarding the validity of the Fortran assumptions are welcome! This patch now uses a 'simple' loop for OpenMP loops with a constant loop-step size. Before, it only did so for step = ±1. (Otherwise, a count variable is used from which the original loop index variable is calculated from.) For details, see the attached patch or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424#c12 (comment 12 + 14 plus the email linked in comment 12). Comments? Remarks? If there are none, I will relatively soonish commit the attached patch to mainline, only. - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 commit 85da0b40538fb0d17d89de1e7905984668e3dfef Author: Tobias Burnus Date: Wed Jul 19 10:18:49 2023 +0200 OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1 [PR107424] Before this commit, gfortran produced with OpenMP for 'do i = 1,10,2' the code for (count.0 = 0; count.0 < 5; count.0 = count.0 + 1) i = count.0 * 2 + 1; While such an inner loop can be collapsed, a non-rectangular could not. With this commit and for all constant loop steps, a simple loop such as 'for (i = 1; i <= 10; i = i + 2)' is created. (Before only for the constant steps of 1 and -1.) The constant step permits to know the direction (increasing/decreasing) that is required for the loop condition. The new code is only valid if one assumes no overflow of the loop variable. However, the Fortran standard can be read that this must be ensured by the user. Namely, the Fortran standard requires (F2023, 10.1.5.2.4): "The execution of any numeric operation whose result is not defined by the arithmetic used by the processor is prohibited." And, for DO loops, F2023's "11.1.7.4.3 The execution cycle" has the following: The number of loop iterations handled by an iteration count, which would permit code like 'do i = huge(i)-5, huge(i),4'. However, in step (3), this count is not only decremented by one but also: "... The DO variable, if any, is incremented by the value of the incrementation parameter m3." And for the example above, 'i' would be 'huge(i)+3' in the last execution cycle, which exceeds the largest model number and should render the example as invalid. PR fortran/107424 gcc/fortran/ChangeLog: * trans-openmp.cc (gfc_nonrect_loop_expr): Accept all constant loop steps. (gfc_trans_omp_do): Likewise; use sign to determine loop direction. libgomp/ChangeLog: * libgomp.texi (Impl. Status 5.0): Add link to new PR110735. * testsuite/libgomp.fortran/non-rectangular-loop-1.f90: Enable commented tests. * testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: Remove test file; tests are in non-rectangular-loop-1.f90. * testsuite/libgomp.fortran/non-rectangular-loop-5.f90: Change testcase to use a non-constant step to retain the 'sorry' test. * testsuite/libgomp.fortran/non-rectangular-loop-6.f90: New test. gcc/testsuite/ChangeLog: * gfortran.dg/gomp/linear-2.f90: Update dump to remove the additional count variable. --- gcc/fortran/trans-openmp.cc| 18 +- gcc/testsuite/gfortran.dg/gomp/linear-2.f90| 4 +- libgomp/libgomp.texi | 4 +- .../libgomp.fortran/non-rectangular-loop-1.f90 | 537 ++--- .../libgomp.fortran/non-rectangular-loop-1a.f90| 374 -- .../libgomp.fortran/non-rectangular-loop-5.f90 | 22 +- .../libgomp.fortran/non-rectangular-loop-6.f90 | 196 7 files changed, 494 insertions(+), 661 deletions(-) diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc index c88ee3c7656..cf741cebf91 100644 --- a/gcc/fortran/trans-openmp.cc +++ b/gcc/fortran/trans-openmp.cc @@ -5374,10 +5374,10 @@ gfc_nonrect_loop_expr (stmtblock_t *pblock, gfc_se *sep, int loop_n, if
[patch] OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1 [PR107424]
Comments regarding the validity of the Fortran assumptions are welcome!
This patch now uses a 'simple' loop for OpenMP loops with
a constant loop-step size. Before, it only did so for step = ±1.
(Otherwise, a count variable is used from which the original
loop index variable is calculated from.)
For details, see the attached patch or
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424#c12
(comment 12 + 14 plus the email linked in comment 12).
Comments? Remarks? If there are none, I will relatively soonish
commit the attached patch to mainline, only.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP/Fortran: Non-rectangular loops with constant steps other than 1 or -1 [PR107424]
Before this commit, gfortran produced with OpenMP for 'do i = 1,10,2'
the code
for (count.0 = 0; count.0 < 5; count.0 = count.0 + 1)
i = count.0 * 2 + 1;
While such an inner loop can be collapsed, a non-rectangular could not.
With this commit and for all constant loop steps, a simple loop such
as 'for (i = 1; i <= 10; i = i + 2)' is created. (Before only for the
constant steps of 1 and -1.)
The constant step permits to know the direction (increasing/decreasing)
that is required for the loop condition.
The new code is only valid if one assumes no overflow of the loop variable.
However, the Fortran standard can be read that this must be ensured by
the user. Namely, the Fortran standard requires (F2023, 10.1.5.2.4):
"The execution of any numeric operation whose result is not defined by
the arithmetic used by the processor is prohibited."
And, for DO loops, F2023's "11.1.7.4.3 The execution cycle" has the
following: The number of loop iterations handled by an iteration count,
which would permit code like 'do i = huge(i)-5, huge(i),4'. However,
in step (3), this count is not only decremented by one but also:
"... The DO variable, if any, is incremented by the value of the
incrementation parameter m3."
And for the example above, 'i' would be 'huge(i)+3' in the last
execution cycle, which exceeds the largest model number and should
render the example as invalid.
PR fortran/107424
gcc/fortran/ChangeLog:
* trans-openmp.cc (gfc_nonrect_loop_expr): Accept all
constant loop steps.
(gfc_trans_omp_do): Likewise; use sign to determine
loop direction.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/non-rectangular-loop-1.f90: Enabled
commented tests.
* testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: Removed.
* testsuite/libgomp.fortran/non-rectangular-loop-5.f90: Change
testcase to use a non-constant step to retain the 'sorry' test.
* testsuite/libgomp.fortran/non-rectangular-loop-6.f90: New test.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/linear-2.f90: Update dump to remove
the additional count variable.
gcc/fortran/trans-openmp.cc| 12 +-
gcc/testsuite/gfortran.dg/gomp/linear-2.f90| 4 +-
.../libgomp.fortran/non-rectangular-loop-1.f90 | 537 ++---
.../libgomp.fortran/non-rectangular-loop-1a.f90| 374 --
.../libgomp.fortran/non-rectangular-loop-5.f90 | 10 +-
.../libgomp.fortran/non-rectangular-loop-6.f90 | 196
6 files changed, 477 insertions(+), 656 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index c88ee3c7656..e33b18fdada 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5374,10 +5374,10 @@ gfc_nonrect_loop_expr (stmtblock_t *pblock, gfc_se *sep, int loop_n,
if (!simple)
{
- /* FIXME: Handle non-unit iter steps, cf. PR fortran/107424. */
+ /* FIXME: Handle non-const iter steps, cf. PR fortran/107424. */
sorry_at (gfc_get_location (_loop_var->where),
- "non-rectangular loop nest with step other than constant 1 "
- "or -1 for %qs", curr_loop_var->symtree->n.sym->name);
+ "non-rectangular loop nest with non-constant step for %qs",
+ curr_loop_var->symtree->n.sym->name);
return false;
}
@@ -5578,10 +5578,8 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t *pblock,
gfc_add_block_to_block (pblock, );
step = gfc_evaluate_now (se.expr, pblock);
- if (integer_onep (step))
- simple = 1;
- else if (tree_int_cst_equal (step, integer_minus_one_node))
- simple = -1;
+ if (TREE_CODE (step) == INTEGER_CST)
+ simple = tree_int_cst_sgn (step);
gfc_init_se (, NULL);
if (!clauses->non_rectangular
diff --git a/gcc/testsuite/gfortran.dg/gomp/linear-2.f90 b/gcc/testsuite/gfortran.dg/gomp/linear-2.f90
index 05f007fd5c2..88df96e9b8f 100644
--- a/gcc/testsuite/gfortran.dg/gomp/linear-2.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/linear-2.f90
@@ -105,8 +105,8 @@ end module
! { dg-final { scan-tree-dump-times "#pragma omp for
