public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397
@ 2022-10-26 18:37 gscfq@t-online.de
  2022-10-27  8:50 ` [Bug fortran/107424] " marxin at gcc dot gnu.org
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: gscfq@t-online.de @ 2022-10-26 18:37 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

            Bug ID: 107424
           Summary: [13 Regression] ICE in gfc_trans_omp_do, at
                    fortran/trans-openmp.cc:5397
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gscfq@t-online.de
  Target Milestone: ---

Started between 20220501 and 20220508 :


$ cat z1.f90
program p
   integer :: i, j
   !$omp do simd collapse(2)
   do i = 1, 9, 2
      do j = 1, i
      end do
   end do
end


$ gfortran-13-20220501 -c z1.f90 -fopenmp
z1.f90:5:17:

    5 |       do j = 1, i
      |                 1
Error: !$OMP DO SIMD collapsed loops don't form rectangular iteration space at
(1)


$ gfortran-13-20221023 -c z1.f90 -fopenmp
z1.f90:3:28:

    3 |    !$omp do simd collapse(2)
      |                            1
internal compiler error: in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397
0x953457 gfc_trans_omp_do
        ../../gcc/fortran/trans-openmp.cc:5397
0x9549a1 gfc_trans_omp_do_simd
        ../../gcc/fortran/trans-openmp.cc:6526
0x956cd2 gfc_trans_omp_directive(gfc_code*)
        ../../gcc/fortran/trans-openmp.cc:7554
0x8b7897 trans_code
        ../../gcc/fortran/trans.cc:2245
0x8ef2a9 gfc_generate_function_code(gfc_namespace*)
        ../../gcc/fortran/trans-decl.cc:7659
0x8608de translate_all_program_units
        ../../gcc/fortran/parse.cc:6696
0x8608de gfc_parse_file()
        ../../gcc/fortran/parse.cc:7002
0x8aec0f gfc_be_parse_file
        ../../gcc/fortran/f95-lang.cc:229

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
@ 2022-10-27  8:50 ` marxin at gcc dot gnu.org
  2022-10-28 11:35 ` rguenth at gcc dot gnu.org
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-10-27  8:50 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-10-27
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |sandra at codesourcery dot com

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r13-142-g705bcedf6eae2d7c.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
  2022-10-27  8:50 ` [Bug fortran/107424] " marxin at gcc dot gnu.org
@ 2022-10-28 11:35 ` rguenth at gcc dot gnu.org
  2023-01-11 15:30 ` [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops burnus at gcc dot gnu.org
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-10-28 11:35 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
   Target Milestone|---                         |13.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
  2022-10-27  8:50 ` [Bug fortran/107424] " marxin at gcc dot gnu.org
  2022-10-28 11:35 ` rguenth at gcc dot gnu.org
@ 2023-01-11 15:30 ` burnus at gcc dot gnu.org
  2023-01-11 15:59 ` burnus at gcc dot gnu.org
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-11 15:30 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org
           Keywords|                            |ice-on-valid-code,
                   |                            |wrong-code
            Summary|[13 Regression] ICE in      |[13 Regression] ICE in
                   |gfc_trans_omp_do, at        |gfc_trans_omp_do, at
                   |fortran/trans-openmp.cc:539 |fortran/trans-openmp.cc:539
                   |7                           |7 - and wrong code - with
                   |                            |non-rectangular loops

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
The following program does not ICE but it shows with -Wall:
  Warning: ‘i’ is used uninitialized
the code is as above (comment 0) except for:

   do i = 1, 9, 1   !  <<< only change: Stride == 1; ICE has stride == 2.
      do j = 1, i

Looking at the dump:

    integer(kind=4) D.4264;

    D.4264 = i;  // BAD: Uninitialized variable
    #pragma omp for collapse(2)
      {
        {
          #pragma omp simd collapse(2)
          for (i = 1; i <= 9; i = i + 1)
            for (j = 1; j <= D.4264; j = j + 1)  // Wrong: use loop war 'i' not
'D.4264'!
              {
                L.1:;
              }

--------------

Draft patch for this issue – but not for the original issue (the ICE):

--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5224 +5224 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t
*pblock,
-      from = gfc_evaluate_now (se.expr, pblock);
+      from = DECL_P (se.expr) ? se.expr : gfc_evaluate_now (se.expr, pblock);
@@ -5229 +5229 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t
*pblock,
-      to = gfc_evaluate_now (se.expr, pblock);
+      to = DECL_P (se.expr) ? se.expr : gfc_evaluate_now (se.expr, pblock);
@@ -5234 +5234 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t
*pblock,
-      step = gfc_evaluate_now (se.expr, pblock);
+      step = DECL_P (se.expr) ? se.expr : gfc_evaluate_now (se.expr, pblock);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (2 preceding siblings ...)
  2023-01-11 15:30 ` [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops burnus at gcc dot gnu.org
@ 2023-01-11 15:59 ` burnus at gcc dot gnu.org
  2023-01-13 10:46 ` burnus at gcc dot gnu.org
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-11 15:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Commenting out the 'gcc_assert' of comment 0, it compiles and produces the
following dump.
I don't understand why there is a 'lastprivate' – and 'i' in the bounds are
wrong: for the first iteration, it is undefined and otherwise, it lags always
behind.

          #pragma omp simd lastprivate(count.0) collapse(2)
          for (count.0 = 0; count.0 < 5; count.0 = count.0 + 1)
            for (j = 1; j <= i; j = j + 1)
              {
                i = count.0 * 2 + 1;
                L.1:;
              }

And yet another variant:
   !$omp do simd collapse(2)
   do i = 1, 9, 2
      do j = 1, i, 2
i.e. both with non-unit strides. Then the result is still an ICE; commenting
the assert, the result is:

    D.4265 = (i + 1) / 2;  // Ups! This should use 'count.1' and shall not be
hoisted!
    #pragma omp for collapse(2)
      {
        {
          #pragma omp simd lastprivate(count.1) lastprivate(count.0)
collapse(2)
          for (count.0 = 0; count.0 < 5; count.0 = count.0 + 1)
            for (count.1 = 0; count.1 < D.4265; count.1 = count.1 + 1)
              {
                i = count.0 * 2 + 1;
                j = count.1 * 2 + 1;
                L.1:;
              }

Here, COUNT is used in the inner loop - that would be also the option for the
stride==1 case, but as the expression needs to be in the condition already, it
might be better to have for inner stride == 1:
            for (j = 1; j <= count.0 * 2 + 1; j = j + 1)
and for inner stride == 2:
            for (j = 1; j <= (count.0 * 2 + 1 + 1) / 2; j = j + 1)

We probably need to check whether any of lb,ub,stride contains a parent loop
var.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (3 preceding siblings ...)
  2023-01-11 15:59 ` burnus at gcc dot gnu.org
@ 2023-01-13 10:46 ` burnus at gcc dot gnu.org
  2023-01-13 10:53 ` burnus at gcc dot gnu.org
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-13 10:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 54265
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54265&action=edit
WIP patch (with one WIP testcase)

This patch should address the issues of this PR reported so far.

But there two issues: (A) I am not sure whether the implicit 'lastprivate'
should be added for SIMD or not. (B) there is an odd issue with lastprivate
after gimplify, before it looks fine.

(A) 'lastprivate' or not (refers to the patch itself):
The SIMD/!simple change is because of:
  If there is no unit stride, 'count' is created (+ 'lastprivate(count)').
  For collapse == 1, 'linear' is added and later due to the 'dovar_found = 3',
    OMP_CLAUSE_LINEAR_STMT (c) = ...
  For collapse > 1, nothing was added – such that the code trying to add
  a ..._STMT failed to find a clause + and the 'c != NULL' assert then failed.

  The current patch adds 'lastprivate' in this case – an alternative would be
  to move the 'dovar_found = 3' into the 'if'.
  (The lastprivate(count) would still be added.)

  It is not quite clear to me what OpenMP requires - lastprivate or nothing
  (TODO: check).


(B) The following code goes wrong; I converted it as closely as possible to C
and the C program works fine.

  integer :: n,m,p, i,j,k
  n = 11; m = 23; p = 27
  !$omp simd collapse(3) lastprivate(k)
  do i = 1, n
    do j = 1, m
      do k = j - 41, p
        if (k < 1 - 41 .or. k > p) error stop
      end do
    end do
  end do
  end

First, without the patch, the 'j - 41' is evaluated before the loop, which is
obviously wrong:
    D.4268 = j + -41;
    #pragma omp simd lastprivate(k) collapse(3)
      ...
        for (k = D.4268; k <= p; k = k + 1)

But with the patch, it seems to be fine:

  #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
  for (i = 1; i <= n; i = i + 1)
    for (j = 1; j <= m; j = j + 1)
      for (k = j + -41; k <= p; k = k + 1)
            if (k < -40 || k > p)

However, the gimple dump has the following - note the 'k = D.4287' where the
RHS is uninitialized!

      n = 11;
      m = 23;
      p = 27;
      #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
private(k.9) private(j.6) private(i.3) private(dt_parm.0)
      for (i.3 = 1; i.3 <= n; i.3 = i.3 + 1)
        for (j.6 = 1; j.6 <= m; j.6 = j.6 + 1)
          for (k.9 = D.4287; k.9 <= p; k.9 = k.9 + 1)


BTW: The attached patch contains a testcase from which (B) has been extracted
as reduced fail.

TODO: Besides fixing issue (B) and checking (A), some more testcases are needed
+ fixing issues popping up when doing so.

(PS: Currently, the start/end/step expressions are put into the loop even if
they do not depend on another loop variable - this could be optimized, but my
feeling is that this is not worthwhile.)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (4 preceding siblings ...)
  2023-01-13 10:46 ` burnus at gcc dot gnu.org
@ 2023-01-13 10:53 ` burnus at gcc dot gnu.org
  2023-01-13 11:07 ` jakub at gcc dot gnu.org
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-13 10:53 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
For completeness, the C testcase to previous comment (comment 3) is as follows.
Contrary to Fortran, I put a 'lastprivate' to all variables – to match the
current
patch (→ see (A) in comment 3).

  int n,m,p, i,j,k;
  n = 11; m = 23; p = 27;
  #pragma omp simd collapse(3) lastprivate(k) lastprivate(i) lastprivate(j)
  for (i = 1; i <= n; i++)
    for (j = 1; j <= m; j++)
      for (k = j - 41; k <= p; k++)
        if (k < 1 - 41 || k > p)
          __builtin_printf ("%d %d %d\n", i, j, k);
  if (i != n + 1) __builtin_abort ();
  if (j != m + 1) __builtin_abort ();
  if (k != p + 1) __builtin_abort ();

The code works fine. The original dump is also the same, except for the '* 1':
  #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
     ...
      for (k = j * 1 + -41; k <= p; k++ )

but in the gimple dump, the result is:
    #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3) \
                     private(k.2) private(j.1) private(i.0)
    for (i.0 = 1; i.0 <= n; i.0 = i.0 + 1)
      for (j.1 = 1; j.1 <= m; j.1 = j.1 + 1)
        for (k.2 = j.1 * 1 + -41; k.2 <= p; k.2 = k.2 + 1)

where 'j' has been replaced by 'j.1' and not been hosted before the loop.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (5 preceding siblings ...)
  2023-01-13 10:53 ` burnus at gcc dot gnu.org
@ 2023-01-13 11:07 ` jakub at gcc dot gnu.org
  2023-01-13 11:46 ` burnus at gcc dot gnu.org
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-13 11:07 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think there are multiple issues.  For the do i = 1, 9, 1 case, the bug is
that we:
      gfc_init_se (&se, NULL);
      gfc_conv_expr_val (&se, code->ext.iterator->end);
      gfc_add_block_to_block (pblock, &se.pre);
      to = gfc_evaluate_now (se.expr, pblock);
and therefore evaluate the upper expression (but ditto the lower expression)
before the loop nest into a temporary rather than keeping it directly in the
expression.
That is fine for the cases where the expressions are loop invariant, which was
always the case before non-rectangular loops were introduced, but is not ok for
non-rectangular loops.  I think the above patch deals with that somehow, but
haven't studied yet exactly how.  Don't know about the step2 case on the outer
loop, but guess it is similar to the below thing too.
Another problem is that for steps other than -1 and 1, we use the non-simple
expansion:
          /* STEP is not 1 or -1.  Use:
             for (count = 0; count < (to + step - from) / step; count++)
               {
                 dovar = from + count * step;
                 body;
               cycle_label:;
               }  */
If the inner loop of non-rectangular loop nest (well, in particular whenever
from or to
uses some outer loop iterator and is one of the allowed non-rectangular forms):
var-outer
var-outer + a2
a2 + var-outer
var-outer - a2
a2 - var-outer
a1 * var-outer
a1 * var-outer + a2
a2 + a1 * var-outer
a1 * var-outer - a2
a2 - a1 * var-outer
var-outer * a1
var-outer * a1 + a2
a2 + var-outer * a1
var-outer * a1 - a2
a2 - var-outer * a1
then using the non-simple expansion just can't work, the middle-end expects
the init and cond expressions to be literally something from the above list,
if that is turned into something divided by step, we've lost.
Now, the reason I've added the above expansion is because the middle-end
representation was chosen to be of the C/C++ loops and I wasn't sure if the
Fortran DO loop semantics
can be easily mapped to it.  For the non-rectangular loops, I'm afraid we have
to,
or have some flag on each of the OMP_FOR/GOMP_FOR loops to request Fortran
semantics.
If we do the latter, we'd need to adjust the middle-end (omp_extract_for_data,
everything that computes number of iterations, omp-expand loop expansion) to
handle that.
And I believe the non-simple outer loop is the same thing, for the
non-rectangular loops
the middle-end relies on the inner loop expressions that refer to var-outer to
be actually the outer loop's iterator.
So if we use the non-simple expansion for the outer loop:
             for (count = 0; count < (to + step - from) / step; count++)
               {
                 dovar = from + count * step;
                 body;
that would mean remapping the var_outer (aka dovar) in those expressions to
count based computations.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (6 preceding siblings ...)
  2023-01-13 11:07 ` jakub at gcc dot gnu.org
@ 2023-01-13 11:46 ` burnus at gcc dot gnu.org
  2023-01-13 11:51 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-13 11:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |accepts-invalid

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> I think there are multiple issues.  For the do i = 1, 9, 1 case, the bug is
> that we:
>       to = gfc_evaluate_now (se.expr, pblock);

The patch simply skips this if se.expr is already a DECL_P (as it is then IMHO
pointless)  or if there is a non-rectangular loop nest. — In the latter case,
it additionally replaces
 'inner = outer + a1' by  'inner = (count.outer + lb.outer) + a1'

Thus, semantically, that should be fine but breaks with:
...

> If the inner loop of non-rectangular loop nest (well, in particular whenever
> from or to
> uses some outer loop iterator and is one of the allowed non-rectangular
> forms):
...
> var-outer + a2
...
> var-outer * a1 + a2
> then using the non-simple expansion just can't work, the middle-end expects
> the init and cond expressions to be literally something from the above list,

In the testcase I have - which does not use 'count' - the original dump for
Fortran is:
      for (k = j + -41; k <= p; k = k + 1)
but for C it is
      for (k = j * 1 + -41; k <= p; k++ )

Both are valid according to the list, but possibly the gimplifier does not
handle
  'outer + a2' but only 'outer * a1 + a2'
which would explain the issue I see. From the code, it also looks as if a very
specific order is required – and a TREE_VEC and not a normal expression:

      if (OMP_FOR_NON_RECTANGULAR (for_stmt) && var != decl)
        for (int j = i + 1; j < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); j++)
          {
            t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), j);
            gcc_assert (TREE_CODE (t) == MODIFY_EXPR);
            if (TREE_CODE (TREE_OPERAND (t, 1)) == TREE_VEC
                && TREE_VEC_ELT (TREE_OPERAND (t, 1), 0) == decl)
              TREE_VEC_ELT (TREE_OPERAND (t, 1), 0) = var;

 * * *

With regards to 'count', I think it would still work - but the replacement
    ll * 3 + a1
cannot be: (count.0 + lb) * 3 + a1
but has to be:  count.0 * 3 + (lb*3 + a1)

Given that already quite some specific format is required (see above), it seems
as if
just continuing to use 'count' would work.
It also would permit to add some additional checks, ensuring that the format of
the
loop is correct.

It also seems to be simpler than:

> Now, the reason I've added the above expansion is because the middle-end representation
> was chosen to be of the C/C++ loops and I wasn't sure if the Fortran DO loop semantics
> can be easily mapped to it.  For the non-rectangular loops, I'm afraid we have to,
> or have some flag on each of the OMP_FOR/GOMP_FOR loops to request Fortran semantics.


Note regarding:
> if that is turned into something divided by step, we've lost.

I note that for Fortran, the OpenMP spec (I looked at 5.2) has:

   lb, ub   One of the following:
        ....
   a1, a2, incr   Integer expressions that are loop invariant with respect
                  to the outermost loop of the10 loop nest.

Thus, as soon as a DO loop has an increment expression (even if it evaluates to
'1'),
neither a1 nor a2 nor incr  can depend on any outer loop variable.

Thus, it seems to be doable in the Fortran FE itself.

Regarding the latter, gfortran accepts:
      do k = i - 41, p, p *4
which I think is wrong ('i - 41' does not fulfill this requirement)'. Likewise
wrong
is
      do k = i - 41, p, 1
but I am not sure whether the ',1' vs. absent is trivially detectable or not.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (7 preceding siblings ...)
  2023-01-13 11:46 ` burnus at gcc dot gnu.org
@ 2023-01-13 11:51 ` jakub at gcc dot gnu.org
  2023-01-18 22:24 ` burnus at gcc dot gnu.org
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-13 11:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For the non-simple loops, I guess the questions are if
do i = x, y, 10000
is equivalent to
for (i = x; i <= y; i += 10000)
or not (especially regarding the in C UB cases of y being above INT_MAX -
10000),
and also what to do about
do i = x, y, z
where the step is variable - in C/C++, we always know if the iterator is
increasing or decreasing from the condition, except for != condition but in
that case OpenMP requires
the step to be compile time known 1 or -1.  I'm afraid for Fortran we have no
idea.
And for non-rectangular loops like:
!$omp do collapse(2)
do i = x, y, s1
  do j = z, 2 * i + w, s2
that is quite important...

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (8 preceding siblings ...)
  2023-01-13 11:51 ` jakub at gcc dot gnu.org
@ 2023-01-18 22:24 ` burnus at gcc dot gnu.org
  2023-01-25 14:50 ` burnus at gcc dot gnu.org
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-18 22:24 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54265|0                           |1
        is obsolete|                            |

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 54300
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54300&action=edit
Patch handling some nonrect loops + sorry for others, but being blocked by
PR108459

Next attempt for a patch, which handles non-rectangular loops except when a
'count' variable is introduced. For that case, see comment 6 and comment 8.

In principle, it is ready for submission, except that now the testcase
  gcc/testsuite/gfortran.dg/gomp/canonical-loop-1.f90
fails with an ICE (segfault).

That seems to be a related but independent issue, also affecting C/C++;
now filed as PR middle-end/108459. While independent, it blocks this patch.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (9 preceding siblings ...)
  2023-01-18 22:24 ` burnus at gcc dot gnu.org
@ 2023-01-25 14:50 ` burnus at gcc dot gnu.org
  2023-02-09 14:51 ` cvs-commit at gcc dot gnu.org
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-01-25 14:50 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #10 from Tobias Burnus <burnus at gcc dot gnu.org> ---
> now filed as PR middle-end/108459.

This has been fixed to mainline. Cross ref: also about collapsed loops but
otherwise unrelated: PR middle-end/108435 (loop-var with function nesting
issue)

 * * *

Patch for this PR, sorry-ing for non-unit loop steps, but otherwise working:

  https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610584.html

Follow-up work needed (once accepted): Handling some/all cases of non-unit
steps; for some thoughts on handling this, see:

  https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610351.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (10 preceding siblings ...)
  2023-01-25 14:50 ` burnus at gcc dot gnu.org
@ 2023-02-09 14:51 ` cvs-commit at gcc dot gnu.org
  2023-02-09 14:54 ` burnus at gcc dot gnu.org
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-02-09 14:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:ac2949574da9a668daad421d7edb79f172f73c6f

commit r13-5756-gac2949574da9a668daad421d7edb79f172f73c6f
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Thu Feb 9 15:51:13 2023 +0100

    OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]

    This patch ensures that loop bounds depending on outer loop vars use the
    proper TREE_VEC format. It additionally gives a sorry if such an outer
    var has a non-one/non-minus-one increment as currently a count variable
    is used in this case (see PR).

    Finally, it avoids 'count' and just uses a local loop variable if the
    step increment is +/-1.

            PR fortran/107424

    gcc/fortran/ChangeLog:

            * trans-openmp.cc (struct dovar_init_d): Add 'sym' and
            'non_unit_incr' members.
            (gfc_nonrect_loop_expr): New.
            (gfc_trans_omp_do): Call it; use normal loop bounds
            for unit stride - and only create local loop var.

    libgomp/ChangeLog:

            * testsuite/libgomp.fortran/non-rectangular-loop-1.f90: New test.
            * testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: New test.
            * testsuite/libgomp.fortran/non-rectangular-loop-2.f90: New test.
            * testsuite/libgomp.fortran/non-rectangular-loop-3.f90: New test.
            * testsuite/libgomp.fortran/non-rectangular-loop-4.f90: New test.
            * testsuite/libgomp.fortran/non-rectangular-loop-5.f90: New test.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/goacc/privatization-1-compute-loop.f90: Update
dg-note.
            * gfortran.dg/goacc/privatization-1-routine_gang-loop.f90:
Likewise.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (11 preceding siblings ...)
  2023-02-09 14:51 ` cvs-commit at gcc dot gnu.org
@ 2023-02-09 14:54 ` burnus at gcc dot gnu.org
  2023-04-26  6:56 ` [Bug fortran/107424] [13/14 " rguenth at gcc dot gnu.org
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-02-09 14:54 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #12 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Still to be done:

Handle loop steps other than ±1. For a suggestion how it could be handled, see
thread ending with the following email, which is possibly sufficient

  https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610351.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13/14 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (12 preceding siblings ...)
  2023-02-09 14:54 ` burnus at gcc dot gnu.org
@ 2023-04-26  6:56 ` rguenth at gcc dot gnu.org
  2023-07-17 11:15 ` burnus at gcc dot gnu.org
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-04-26  6:56 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.0                        |13.2

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 13.1 is being released, retargeting bugs to GCC 13.2.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13/14 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (13 preceding siblings ...)
  2023-04-26  6:56 ` [Bug fortran/107424] [13/14 " rguenth at gcc dot gnu.org
@ 2023-07-17 11:15 ` burnus at gcc dot gnu.org
  2023-07-19  8:19 ` cvs-commit at gcc dot gnu.org
  2023-07-19  8:27 ` burnus at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-07-17 11:15 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #14 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #12)
> Handle loop steps other than ±1.

Fortran (here F2023) has under "11.1.7.4.3 The execution cycle" (for "DO"):
"... consists of the following steps ..."
"(1) The iteration count, if any, is tested. If it is zero, the loop terminates
...
"(2) The block of the loop is executed.28
"(3) The iteration count, if any, is decremented by one. The DO variable, if
any, is incremented by the value of the incrementation parameter m3."

And Fortran states: "The execution of any numeric operation whose result is not
defined by the arithmetic used by the processor is prohibited." (10.1.5.2.4
Evaluation of numeric intrinsic operations).

* * *

Thus (cf. email linked in comment 12), simply extending this to any constant
step should be possible. (For the other cases, see email.)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13/14 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (14 preceding siblings ...)
  2023-07-17 11:15 ` burnus at gcc dot gnu.org
@ 2023-07-19  8:19 ` cvs-commit at gcc dot gnu.org
  2023-07-19  8:27 ` burnus at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-19  8:19 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:85da0b40538fb0d17d89de1e7905984668e3dfef

commit r14-2634-g85da0b40538fb0d17d89de1e7905984668e3dfef
Author: Tobias Burnus <tobias@codesourcery.com>
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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug fortran/107424] [13/14 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
  2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
                   ` (15 preceding siblings ...)
  2023-07-19  8:19 ` cvs-commit at gcc dot gnu.org
@ 2023-07-19  8:27 ` burnus at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-07-19  8:27 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #16 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Close as FIXED - see also PR110735

The original issue (the ICE) and the wrong-code issue (comment 2) were fixed in
r13-5756 when it was still mainline. Thus, the regression is fixed and there is
no need to backport anything.

As part of that commit, a "sorry, unimplemented" was introduced.
- A simpler and possibly more common case was implemented in GCC 14 mainline
via commit r14-2634.
- The remaining issues are now tracked in PR110735.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-07-19  8:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
2022-10-27  8:50 ` [Bug fortran/107424] " marxin at gcc dot gnu.org
2022-10-28 11:35 ` rguenth at gcc dot gnu.org
2023-01-11 15:30 ` [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops burnus at gcc dot gnu.org
2023-01-11 15:59 ` burnus at gcc dot gnu.org
2023-01-13 10:46 ` burnus at gcc dot gnu.org
2023-01-13 10:53 ` burnus at gcc dot gnu.org
2023-01-13 11:07 ` jakub at gcc dot gnu.org
2023-01-13 11:46 ` burnus at gcc dot gnu.org
2023-01-13 11:51 ` jakub at gcc dot gnu.org
2023-01-18 22:24 ` burnus at gcc dot gnu.org
2023-01-25 14:50 ` burnus at gcc dot gnu.org
2023-02-09 14:51 ` cvs-commit at gcc dot gnu.org
2023-02-09 14:54 ` burnus at gcc dot gnu.org
2023-04-26  6:56 ` [Bug fortran/107424] [13/14 " rguenth at gcc dot gnu.org
2023-07-17 11:15 ` burnus at gcc dot gnu.org
2023-07-19  8:19 ` cvs-commit at gcc dot gnu.org
2023-07-19  8:27 ` burnus at gcc dot gnu.org

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).