[Bug tree-optimization/113808] New: [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90

[Bug tree-optimization/113808] New: [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90

[rguenth at gcc dot gnu.org]

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

            Bug ID: 113808
           Summary: [14 Regression] FAIL:
                    libgomp.fortran/non-rectangular-loop-1.f90
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

The following reduced testcase from libgomp.fortran/non-rectangular-loop-1.f90
fails execution:

program main
  integer :: n,m,p, i,j,k,ll
  n = 11
  m = 23
  p = 27
  !$omp simd collapse(3) lastprivate(k)
  do i = 1, n,2
    do j = 1, m
      do k = 1, i + 41
        if (k > 11 + 41 .or. k < 1) error stop
      end do
    end do
  end do
  if (k /= 53) then
    print *, k, 53
    error stop
  endif
end

when built with -O -msse4.1 -fopenmp-simd

> ./a.out 
          50          53
ERROR STOP 

Error termination. Backtrace:
#0  0x4008ec in ???
#1  0x400909 in ???
#2  0x7f873306f24c in ???
#3  0x400679 in _start
        at ../sysdeps/x86_64/start.S:120
#4  0xffffffffffffffff in ???

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
           Keywords|                            |wrong-code
                 CC|                            |tnfchris at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
The error must be for the continuation of 'k' to the scalar loop where we have

  <bb 5> [local count: 829590381]:
  MEM <vector(4) integer(kind=4)> [(integer(kind=4) *)&D.4344] =
vect_vec_iv_.27_95;
  vect_k.32_118 = vect_vec_iv_.27_95 + { 1, 1, 1, 1 };
  k.4_23 = k.4_55 + 1; 
  ivtmp_120 = ivtmp_119 + 1;
  if (ivtmp_120 < bnd.23_89)
    goto <bb 15>; [85.44%]
  else
    goto <bb 18>; [14.56%]

  <bb 18> [local count: 136777259]:
  # k.4_45 = PHI <k.4_23(5)>
  # ivtmp_76 = PHI <ivtmp_14(5)>
  # vect_vec_iv_.27_99 = PHI <vect_vec_iv_.27_95(5)>
  # vect__19.29_108 = PHI <{ 0, 1, 2, 3 }(5)>
  _109 = BIT_FIELD_REF <vect__19.29_108, 32, 96>;
  _48 = _109;
  _100 = BIT_FIELD_REF <vect_vec_iv_.27_99, 32, 0>;
  k.4_43 = _100;
  niters_vector_mult_vf.24_90 = bnd.23_89 << 2;
  tmp.26_93 = 53 - niters_vector_mult_vf.24_90;
  _92 = (integer(kind=4)) niters_vector_mult_vf.24_90;
  tmp.25_91 = _92 + 1;
  if (niters.22_12 == niters_vector_mult_vf.24_90)
    goto <bb 6>; [25.00%]
  else
    goto <bb 22>; [75.00%]

  <bb 22> [local count: 136777259]:
  # k.4_74 = PHI <tmp.25_91(18), k.4_78(23)>
  # ivtmp_77 = PHI <tmp.26_93(18), ivtmp_79(23)>

but I can't really see anything wrong here (besides redundant code).

It's possible to elide the middle loop, but I failed to emulate the
inner loop how it's presented without -fopenmp-simd.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90

[tnfchris at gcc dot gnu.org]

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

--- Comment #2 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
I guess whether that code is correct depends on which exit was picked though.

I'll look at dump too.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-02-07
            Summary|[14 Regression] FAIL:       |[14 Regression] FAIL:
                   |libgomp.fortran/non-rectang |libgomp.fortran/non-rectang
                   |ular-loop-1.f90             |ular-loop-1.f90 since
                   |                            |r14-8768
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r14-8768-g85094e2aa6dba7908f053046f02dd443e8f65d72
The regression status is unclear because we emitted sorry on this
before r14-2634-g85da0b40538fb0d17d89de1e7905984668e3dfef

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Reduced a bit, w/o collapse:

program main
  integer :: n, i,k
  n = 11
  do i = 1, n,2
    !$omp simd lastprivate(k)
    do k = 1, i + 41
      if (k > 11 + 41 .or. k < 1) error stop
    end do
  end do
  if (k /= 53) then
    print *, k, 53
    error stop
  endif
end

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Started with r14-8768-g85094e2aa6dba7908f053046f02dd443e8f65d72
> The regression status is unclear because we emitted sorry on this
> before r14-2634-g85da0b40538fb0d17d89de1e7905984668e3dfef

I think r14-8768 just exposed this.

We are picking the last exit in the loop, it's not a PEELED case.
It's the exit towards the if (k/=53) not towards STOP.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
With the following I don't see things going wrong, but we end up with the loop
having the STOP exit last instead and thus a PEELED case.

function bar (n) result (k)
  integer :: n, k
  !$omp simd lastprivate(k)
  do k = 1, n + 41
    if (k > 11 + 41 .or. k < 1) error stop
  end do
end

program main
  integer :: n, i,k
  integer :: bar
  n = 11
  k = bar (n)
  if (k /= 53) then
    print *, k, 53
    error stop
  endif
end

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> (In reply to Jakub Jelinek from comment #3)
> > Started with r14-8768-g85094e2aa6dba7908f053046f02dd443e8f65d72
> > The regression status is unclear because we emitted sorry on this
> > before r14-2634-g85da0b40538fb0d17d89de1e7905984668e3dfef
> 
> I think r14-8768 just exposed this.

That is possible.  But in gcc 13 we'd never vectorize this due to the early
exit, right?
Still, even the #c4 testcase started at r14-8768 and that one is a real
regression from 11/12/13.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's surely a bug in the vectorizer early exit handling.  I just don't know
what exactly is wrong right now ;)

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |tnfchris at gcc dot gnu.org

--- Comment #9 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> With the following I don't see things going wrong, but we end up with the
> loop
> having the STOP exit last instead and thus a PEELED case.

If it's not a PEELED case than the code is wrong indeed.

  _100 = BIT_FIELD_REF <vect_vec_iv_.27_99, 32, 0>;
  k.4_43 = _100;

is wrong since for a normal case the primary exit needs to do a last reduction
rather than a first.

  _109 = BIT_FIELD_REF <vect__19.29_108, 32, 96>;
  _48 = _109;
  _100 = BIT_FIELD_REF <vect_vec_iv_.27_99, 32, 0>;
  k.4_43 = _100;

these two reduction orders should never be different.

The bug seems to be in vectorizable_live_operations where we determine if the
index needs to be a first or last reduction.

There's a boolean there

restart_loop = restart_loop || !main_exit_edge;

and we initially set it to

bool restart_loop = LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo);

outside the USE/DEF loop.

The problem is this depends on seeing the uses for the LOOP_VINFO_IV_EXIT
before seeing that of the early exits.

The code goes wrong because we see the early exit first and then see the main
exit, but once true the boolean can't become false again.

it's a silly bug, the boolean shouldn't be cached between loop iters.

quick hack:

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 190df9ec774..109a7e16abb 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -10966,7 +10966,7 @@ vectorizable_live_operation (vec_info *vinfo,
stmt_vec_info stmt_info,
              /* For early exit where the exit is not in the BB that leads
                 to the latch then we're restarting the iteration in the
                 scalar loop.  So get the first live value.  */
-             restart_loop = restart_loop || !main_exit_edge;
+             restart_loop = !main_exit_edge;
              if (restart_loop
                  && STMT_VINFO_DEF_TYPE (stmt_info) == vect_induction_def)
                {

works but will revisit this and fix properly now.

Thanks for the reduction.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:3f69db1812106cb5bab203e17a60300ac51cdc68

commit r14-8880-g3f69db1812106cb5bab203e17a60300ac51cdc68
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Feb 8 10:43:13 2024 +0000

    middle-end: don't cache restart_loop in vectorizable_live_operations
[PR113808]

    There's a bug in vectorizable_live_operation that restart_loop is defined
    outside the loop.

    This variable is supposed to indicate whether we are doing a first or last
    index reduction.  The problem is that by defining it outside the loop it
becomes
    dependent on the order we visit the USE/DEFs.

    In the given example, the loop isn't PEELED, but we visit the early exit
uses
    first.  This then sets the boolean to true and it can't get to false again.

    So when we visit the main exit we still treat it as an early exit for that
    SSA name.

    This cleans it up and renames the variables to something that's hopefully
    clearer to their intention.

    gcc/ChangeLog:

            PR tree-optimization/113808
            * tree-vect-loop.cc (vectorizable_live_operation): Don't cache the
            value cross iterations.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/113808
            * gfortran.dg/vect/vect-early-break_1-PR113808.f90: New test.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

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

--- Comment #11 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Fixed thanks.

[Bug tree-optimization/113808] [14 Regression] FAIL: libgomp.fortran/non-rectangular-loop-1.f90 since r14-8768

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:70b9dd8b388668df96f457b3c6ee33fc5b35f94c

commit r14-8882-g70b9dd8b388668df96f457b3c6ee33fc5b35f94c
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Feb 8 11:03:31 2024 +0000

    testsuite: address Jakub's comments on vect-early-break_1-PR113808.f90

    Rename pr to lowercase and drop lastprivate.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/113808
            * gfortran.dg/vect/vect-early-break_1-PR113808.f90: Moved to...
            * gfortran.dg/vect/vect-early-break_1-pr113808.f90: ...here.