[Bug fortran/102043] New: Wrong array types used for negative stride accesses

[Bug fortran/102043] New: Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

            Bug ID: 102043
           Summary: Wrong array types used for negative stride accesses
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

gfortran.dg/vector_subscript_1.f90 fails with -O -fipa-pta because

When the frontend lowers idx (10:6:-2) it generates

      parm.22D.4083.dimD.4082[0].lboundD.3942 = 1;
      parm.22D.4083.dimD.4082[0].uboundD.3943 = 3;
      parm.22D.4083.dimD.4082[0].strideD.3941 = -2;
      parm.22D.4083.dataD.4078 = (voidD.27 *) &idxD.4029[9];
...
              D.4091 = (*(integer(kind=4)D.8[0:] *)
parm.22D.4083.dataD.4078)[parm.22D.4083.dimD.4082[0].strideD.3941 *
NON_LVALUE_EXPR <S.24D.4090>];

and that looks clearly wrong to me, we have an access to an array [0:] but
with negative index.  The middle-end here correctly concludes that this access
can only possibly access idx[9] and not any other element.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-08-25
     Ever confirmed|0                           |1
      Known to fail|                            |11.2.1, 12.0, 7.5.0
      Known to work|                            |4.3.5

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Testcase, FAILs at -O1, FRE concludes the idx(2:1:-1) load is always '2'.  At
-O2 we unroll the loop mitigating the issue.

program main
   integer, dimension (2) :: idx, a, b
   a = (/ 0, 0 /)
   idx = (/ 1, 2 /)
   a(idx(2:1:-1)) = idx
   if (a(1).ne.2) STOP 1
end program main

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
For this testcase the bogus ref is constructed by conv_array_index_offset:

          /* Read the vector to get an index into info->descriptor.  */
          data = build_fold_indirect_ref_loc (input_location,
                                          gfc_conv_array_data (desc));
          index = gfc_build_array_ref (data, index, NULL);

where ultimatively gfc_conv_descriptor_data_get is what builds the
base the ARRAY_REF is applied to and thus GFC_TYPE_ARRAY_DATAPTR_TYPE
what is "wrong".

This type is built in gfc_get_array_type_bounds as

  /* We define data as an array with the correct size if possible.
     Much better than doing pointer arithmetic.  */
  if (stride)
    rtype = build_range_type (gfc_array_index_type, gfc_index_zero_node,
                              int_const_binop (MINUS_EXPR, stride,
                                               build_int_cst (TREE_TYPE
(stride), 1)));
  else
    rtype = gfc_array_range_type;
  arraytype = build_array_type (etype, rtype);
  arraytype = build_pointer_type (arraytype);
  if (restricted)
    arraytype = build_qualified_type (arraytype, TYPE_QUAL_RESTRICT);
  GFC_TYPE_ARRAY_DATAPTR_TYPE (fat_type) = arraytype;

The issue might also be the source of "bogus" array bound warnings.

Note it might in the end be the issue that the middle-end lacks notion
of a stride for array accesses which the Fortran frontend emulates
by multiplying the index by the stride.  When the stride is negative
then the idea to base all arrays on [0:] no longer works out.  Note
that simply using an unknown TYPE_DOMAIN (aka NULL_TREE) doesn't
work either since we still assume that ARRAY_REFs can only have
positive indices.

Eric - does Ada have something like negative stride array accesses?

[Bug fortran/102043] Wrong array types used for negative stride accesses

[ebotcazou at gcc dot gnu.org]

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

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Eric - does Ada have something like negative stride array accesses?

No, Ada does not have negative strides, all array accesses are based on the
lower bounds and counted in increasing memory addresses.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
It would be much nicer if the array descriptor for idx(2:1:-1) would not
use a pointer to idx(2) as the data pointer but we'd instead still represent
it as a (1:2) array but with adjusted offset (2 instead of 1).  I think
then the middle-end would be happy.

Does the language always allow to compute this (from ubound - lbound
basically)?

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
So instead of doing *((T[0:] *)&a[ubound])[-idx] for accesses do
a[ubound - idx]?

[Bug fortran/102043] Wrong array types used for negative stride accesses

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gcc dot gnu.org

--- Comment #6 from anlauf at gcc dot gnu.org ---
(In reply to Richard Biener from comment #5)
> So instead of doing *((T[0:] *)&a[ubound])[-idx] for accesses do
> a[ubound - idx]?

I think this assumption is correct.

Of course 'a' could have a non-trivial stride (i.e. being /= 1), which is
provided by the array descriptor.  That should hopefully been respected by
the suggested change.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |seurer at gcc dot gnu.org

--- Comment #7 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
*** Bug 103142 has been marked as a duplicate of this bug. ***

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #8 from Mikael Morin <mikael at gcc dot gnu.org> ---
It comes from gfc_conv_expr_descriptor:

        /* Point the data pointer at the 1st element in the section.  */
        gfc_get_dataptr_offset (&loop.pre, parm, desc, base,
                                subref_array_target, expr);

where base has been calculated as the index of the first element.

I’ve tried doing a mere "if (forward) start else end" in the calculation of
base, but then I needed to add an offset (i.e. array index offset) to vector
subscript accesses, and then a delta was missing (i.e. loop index offset) as
the descriptor is one-based, but I don’t know how to fix the latter without
impacting all the vector subscript handling which seems to rely on quite a few
assumptions (no delta, no offset, zero-based loop?, forward loop?).

[Bug fortran/102043] Wrong array types used for negative stride accesses

[aldot at gcc dot gnu.org]

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

Bernhard Reutner-Fischer <aldot at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aldot at gcc dot gnu.org

--- Comment #9 from Bernhard Reutner-Fischer <aldot at gcc dot gnu.org> ---
yes i've been looking into that too and it seems rather involved to switch the
addressing away from array_ref.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
So to clarify the ARRAY_REF constraints - there is currently no way to
construct a valid ARRAY_REF where an index does an access to memory before the
supplied
base object.  TREE_OPERAND (array_ref, 0) needs to always be the array,
it's address needs to be the address of the _first_ element.  For negative
strides gfortran seems to construct the array object in a way so its
address points to the _last_ element of the array.  That's not supported.

I realize the complication is that with array descriptors we do not know
statically whether the stride is positive or negative and the data
pointer is already set up "wrong" in there so we'd have to go and
undo the biasing which might or might not be easily possible but it will
be costly.

Is there any case where the frontend would make 'data' point into the
middle of the array and iteration over the array would end up accessing
elements on "both sides"?  Can somebody write a short testcase where that
would happen?

[Bug fortran/102043] Wrong array types used for negative stride accesses

[tkoenig at gcc dot gnu.org]

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

--- Comment #11 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #10)

> Is there any case where the frontend would make 'data' point into the
> middle of the array and iteration over the array would end up accessing
> elements on "both sides"?  Can somebody write a short testcase where that
> would happen?

I've tried, but I have been unable to find such a test case, and
I do not think this can (or should) happen.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #12 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Thomas Koenig from comment #11)
> (In reply to Richard Biener from comment #10)
> 
> > Is there any case where the frontend would make 'data' point into the
> > middle of the array and iteration over the array would end up accessing
> > elements on "both sides"?  Can somebody write a short testcase where that
> > would happen?
> 
> I've tried, but I have been unable to find such a test case, and
> I do not think this can (or should) happen.

I would say it can happen as things stand, when one dimension is accessed going
backward and another going forward.

program main
   implicit none
   integer, dimension :: a(4, 4)
   a = 0
   call s(a(4:1:-1,:))
   if (any(a /= 10)) stop 1
contains
  subroutine s(b)
    implicit none
    integer, dimension(:,:) :: b
    b = 10
  end subroutine s
end program main

[Bug fortran/102043] Wrong array types used for negative stride accesses

[tkoenig at gcc dot gnu.org]

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

--- Comment #13 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #12)
> (In reply to Thomas Koenig from comment #11)
> > (In reply to Richard Biener from comment #10)
> > 
> > > Is there any case where the frontend would make 'data' point into the
> > > middle of the array and iteration over the array would end up accessing
> > > elements on "both sides"?  Can somebody write a short testcase where that
> > > would happen?
> > 
> > I've tried, but I have been unable to find such a test case, and
> > I do not think this can (or should) happen.
> 
> I would say it can happen as things stand, when one dimension is accessed
> going backward and another going forward.
> 

You're right.

The test case is actually quite interesting, it shows a
discrepancy between scalarization and indexed access even
in earlier versions:

$ cat a.f90
program main
   implicit none
   integer :: a(4, 4)
   a = 0
   call s(a(4:1:-1,:))
   print '(4(I4))',a
   if (any(a /= 2)) stop 1
contains
  subroutine s(b)
    implicit none
    integer, dimension(:,:) :: b
    b = 2
  end subroutine s
end program main
$ gfortran -v
[...]
gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04) 
$ gfortran a.f90 
$ ./a.out
   2   2   2   2
   2   2   2   2
   2   2   2   2
   2   2   2   2
$ gfortran -O3 a.f90 
$ ./a.out
   2   2   2   2
   2   2   2   2
   2   2   2   2
   2   2   2   2
STOP 1

the same result I get with a fairly recent trunk.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #14 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 51787
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51787&action=edit
draft patch

This "fixes" the problem of negative index access, and adjusts vector subscript
handling, so that correct code is produced for comment #1.
It seems to break comment #13 even more, for reasons that I don’t see (yet).

[Bug fortran/102043] Wrong array types used for negative stride accesses

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at codesourcery dot com

--- Comment #15 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
One possibility would be to extend the patch Sandra posted at
https://gcc.gnu.org/pipermail/fortran/2021-January/055563.html
to scalarization.  With the patch,

the snippet

   integer :: i,j
   integer :: a(4, 4)
   do j=1,size(a,2)
     do i=1,size(a,1)
       a(i,j) = 0
     end do
   end do

is translated to

  while (1)
    {
      {
        logical(kind=4) D.4265;

        D.4265 = j > 4;
        if (D.4265) goto L.4;
        i = 1;
        while (1)
          {
            {
              logical(kind=4) D.4268;

              D.4268 = i > 4;
              if (D.4268) goto L.6;
                            typedef integer(kind=4) [1:4];
                            typedef integer(kind=4) [1:4][1:4];
              ((integer(kind=4)[1:4][1:4]) a)[(integer(kind=8)) j]{lb: 1 sz:
16}[(integer(kind=8)) i]{lb: 1 sz: 4} = 0;
              L.5:;
              i = i + 1;
            }
          }
        L.6:;
        L.3:;
        j = j + 1;
      }
    }

which looks good.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[aldot at gcc dot gnu.org]

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

--- Comment #16 from Bernhard Reutner-Fischer <aldot at gcc dot gnu.org> ---
In addition to comment #1
here's an excerpt of an existing test with just one dimension:

$ cat f_pr86389.f90
! PR 19239.  Check for various kinds of vector subscript.  In this test,
! all vector subscripts are indexing single-dimensional arrays.
! { dg-do run }
program main
  implicit none
  integer, parameter :: n = 10
  integer :: i
  integer, dimension (n) :: a, b, idx, id

  idx = (/ 3, 1, 5, 2, 4, 10, 8, 7, 6, 9 /)
  id = (/ (i, i = 1, n) /)
  b = (/ (i * 100, i = 1, n) /)

  a = 0
  a (idx (10:6:-2)) = b (idx (1:7:3))
write(*,*) "#i   =", id
write(*,*) "idx  =", idx
write(*,*) "now a=", a
write(*,*) "now b=", b
write(*,*) "a idx(10:6:-2)=", idx (10:6:-2)
write(*,*) "b idx(1:7:3)  =", idx (1:7:3)
  call test (idx (10:6:-2), idx (1:7:3))

contains
  subroutine test (lhs, rhs)
    integer, dimension (:) :: lhs, rhs
    integer :: i
!!!    if (size (lhs, 1) .ne. size (rhs, 1)) STOP 11
!    do i = 1, size (lhs, 1)
    do i = 1, 3 ! spare us the code for size(), fixed const
      if (a (lhs (i)) .ne. b (rhs (i))) STOP 12
    end do
!    a = 0
  end subroutine test
end program main

[Bug fortran/102043] Wrong array types used for negative stride accesses

[aldot at gcc dot gnu.org]

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

--- Comment #17 from Bernhard Reutner-Fischer <aldot at gcc dot gnu.org> ---
Do we want to address arrays always at position 0 (maybe to help graphite ?) or
would it be sufficient to just not dereference the array "before" the first
position like Mikael suggests in comment #14?

gfc_conv_expr_descriptor has this
      if (se->data_not_needed)
        gfc_conv_descriptor_data_set (&loop.pre, parm,
                                      gfc_index_zero_node);
      else
        /* Point the data pointer at the 1st element in the section.  */
        gfc_get_dataptr_offset (&loop.pre, parm, desc, base,
                                subref_array_target, expr);

      gfc_conv_descriptor_offset_set (&loop.pre, parm, offset);

where base is (for "section"-addressing) 9 for the testcase in comment #16 as
can be seen in Richard's initial report.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #18 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 51791
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51791&action=edit
new draft patch

This fixes comment #13 as well, but it breaks printing of the dummy array from
the contained subroutine s. :-(

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #19 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Thomas Koenig from comment #15)
> One possibility would be to extend the patch Sandra posted at
> https://gcc.gnu.org/pipermail/fortran/2021-January/055563.html
> to scalarization.

Probably nice to have, but it relies on casts which I see as fragile, and I
don’t trust the middle end to not mishandle corner cases here and there.

(In reply to Bernhard Reutner-Fischer from comment #17)
> Do we want to address arrays always at position 0 (maybe to help graphite ?)
> or would it be sufficient to just not dereference the array "before" the
> first position 
> 
I’m not sure I understand the difference.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[tkoenig at gcc dot gnu.org]

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

--- Comment #20 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #19)
> (In reply to Thomas Koenig from comment #15)
> > One possibility would be to extend the patch Sandra posted at
> > https://gcc.gnu.org/pipermail/fortran/2021-January/055563.html
> > to scalarization.
> 
> Probably nice to have, but it relies on casts which I see as fragile, and I
> don’t trust the middle end to not mishandle corner cases here and there.

Is it indeed fragile?  And if we think up enough test cases to
throw into the testsuite, I think we can at least make sure that
it does not break quietly :-)

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Bernhard Reutner-Fischer from comment #17)
> Do we want to address arrays always at position 0 (maybe to help graphite ?)

Helping graphite (and other loop optimizers) would be to not lower
multi-dimensional accesses to a single dimension (I think that's what
Sandras patches try to do).  The lower bound doesn't really matter here and
is well-handled by all code.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #22 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 51839
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51839&action=edit
third try

Yet another patch.
It works with comment #1 and comment #13.
However, with the testsuite it is a disaster.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #23 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #21)
> (In reply to Bernhard Reutner-Fischer from comment #17)
> > Do we want to address arrays always at position 0 (maybe to help graphite ?)
> 
> Helping graphite (and other loop optimizers) would be to not lower
> multi-dimensional accesses to a single dimension (I think that's what
> Sandras patches try to do). 

Or maybe graphite can be taught to handle flattened array access?

Anyway, does the middle-end support out-of-order array access?
Namely for an array arr(4, 5, 6), arr(:, 1, :) is an array of size (4, 6).
Does the middle-end type system support this?

In any case, it’s not for gcc 12.

> The lower bound doesn't really matter here and
> is well-handled by all code.

Well, unless the lower bound is negative. ;-)

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenther at suse dot de]

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

--- Comment #24 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 19 Nov 2021, mikael at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043
> 
> --- Comment #23 from Mikael Morin <mikael at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #21)
> > (In reply to Bernhard Reutner-Fischer from comment #17)
> > > Do we want to address arrays always at position 0 (maybe to help graphite ?)
> > 
> > Helping graphite (and other loop optimizers) would be to not lower
> > multi-dimensional accesses to a single dimension (I think that's what
> > Sandras patches try to do). 
> 
> Or maybe graphite can be taught to handle flattened array access?
> 
> Anyway, does the middle-end support out-of-order array access?
> Namely for an array arr(4, 5, 6), arr(:, 1, :) is an array of size (4, 6).
> Does the middle-end type system support this?

Support as in not miscompile - yes.  Dependence analysis is only
helped if the shapes of the accesses to the same storage are
compatible - a two dimensional and a three dimensional access are not.

> In any case, it’s not for gcc 12.

Agreed.

> > The lower bound doesn't really matter here and
> > is well-handled by all code.
> 
> Well, unless the lower bound is negative. ;-)

Even a negative lower bound is OK.  Problematic is only if you
provide an index that accesses the array below the (negative)
lower bound ;)  But note!  In GCC  a[i] with a having a lower
bound of say -100 will access storage at &a + (i - -100),
the lower bound is just to make the effective index >= 0 again,
so it can't be used (on its own) to solve the present issue
as far as I understand since you _do_ need to access the
storage at an effective index < 0.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #25 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
*** Bug 103369 has been marked as a duplicate of this bug. ***

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #26 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 51891
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51891&action=edit
latest patch

Here is the patch I have arrived to so far.
I have managed to reduce the number of regressions to 24.
The ones that remain are more and more difficult to analyze and fix, with some
iterating code for internal io that seems mishandled, and a few bind(c) stuff.
There are also middle-end regressions (loop versioning) that I haven’t looked
at.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #27 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 51974
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51974&action=edit
latest patch

This is an almost complete fix.
Almost because one regression remains on ISO_Fortran_binding_1.f90
Indeed this patch doesn’t handle c-fortran interface (aka CFI) descriptors.
Those don’t have any offset field, so it should be fixed differently for them.

In fact, I think the patch, as it is, is not worth it.  It complicates bounds
calculations, and I doubt the benefit out-weights the added complexity.
I’m going to see how to just remove the lower bound information from the
descriptor.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #28 from Mikael Morin <mikael at gcc dot gnu.org> ---
I’m reading the previous comments again:

(In reply to Richard Biener from comment #10)
> So to clarify the ARRAY_REF constraints - there is currently no way to
> construct a valid ARRAY_REF where an index does an access to memory before
> the supplied
> base object.  TREE_OPERAND (array_ref, 0) needs to always be the array,
> it's address needs to be the address of the _first_ element.  For negative
> strides gfortran seems to construct the array object in a way so its
> address points to the _last_ element of the array.  That's not supported.
> 
does that means that clearing the lower bound information from the type as I
suggested in my last comment would not work?

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=80645

--- Comment #29 from Mikael Morin <mikael at gcc dot gnu.org> ---
PR #80645 is possibly related to this.

[Bug fortran/102043] Wrong array types used for negative stride accesses

[mikael at gcc dot gnu.org]

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

--- Comment #30 from Mikael Morin <mikael at gcc dot gnu.org> ---
*** Bug 103671 has been marked as a duplicate of this bug. ***

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #31 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #28)
> I’m reading the previous comments again:
> 
> (In reply to Richard Biener from comment #10)
> > So to clarify the ARRAY_REF constraints - there is currently no way to
> > construct a valid ARRAY_REF where an index does an access to memory before
> > the supplied
> > base object.  TREE_OPERAND (array_ref, 0) needs to always be the array,
> > it's address needs to be the address of the _first_ element.  For negative
> > strides gfortran seems to construct the array object in a way so its
> > address points to the _last_ element of the array.  That's not supported.
> > 
> does that means that clearing the lower bound information from the type as I
> suggested in my last comment would not work?

Correct - the lower bound is used to bias 'i' in 'array[i]', the result still
has to be >= 0.

As said the middle-end lacks support for (negative) stride arrays.
For a negative stride array you'd probably expect array[i] to compute
&array[ubound - S * i]?

That said, the middle-end doesn't even handle positive strided accesses - the
stride needs to be manually reflected to the index which means that the array
type domain does not really reflect the "true" shape of the array.  It's really
tied to C language array support (where array accesses are just fancy ways
of pointer arithmetic).

[Bug fortran/102043] Wrong array types used for negative stride accesses

[rguenth at gcc dot gnu.org]

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

--- Comment #32 from Richard Biener <rguenth at gcc dot gnu.org> ---
But maybe I'm misunderstanding what you do - can you point to the respective
hunk in the patch?

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.5
            Summary|Wrong array types used for  |[9/10/11/12 Regression]
                   |negative stride accesses    |Wrong array types used for
                   |                            |negative stride accesses,
                   |                            |gfortran.dg/vector_subscrip
                   |                            |t_1.f90  FAILs

--- Comment #33 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, for GCC 12 this issue is now visible through IPA modref and results in

FAIL: gfortran.dg/vector_subscript_1.f90   -O1  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O2  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -Os  execution test

on at least x86_64-unknown-linux-gnu.

The testcase in comment#1 still shows the issue is older.  Since gfortran 4.3
works this is still a regression (I guess it became appearant with the alias
oracle introduction).

I'll note the OpenACC folks were working on making gfortran preserve
multi-dimensional array accesses, not sure how they address this issue.  With
a single dimension there's not much value in using ARRAY_REF over
pointer arithmetic and dereference.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[tschwinge at gcc dot gnu.org]

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

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org,
                   |                            |frederik at gcc dot gnu.org,
                   |                            |sandra at gcc dot gnu.org,
                   |                            |tschwinge at gcc dot gnu.org

--- Comment #34 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
I'm totally lacking all context here ;-) -- but did see "OpenACC" mentioned,
so:

(In reply to Richard Biener from comment #33)
> I'll note the OpenACC folks were working on making gfortran preserve
> multi-dimensional array accesses

See Frederik's submission "Fortran: Delinearize array accesses",
<http://mid.mail-archive.com/20211215155447.19379-13-frederik@codesourcery.com>,
primarily developed by Sandra Loosemore, with help from Tobias Burnus (from
what I remember).  Intended for discussion for GCC 13.

> not sure how they address this issue.  With
> a single dimension there's not much value in using ARRAY_REF over
> pointer arithmetic and dereference.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[mikael at gcc dot gnu.org]

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

--- Comment #35 from Mikael Morin <mikael at gcc dot gnu.org> ---
A little status update.

I have pushed the latest patch attached to this PR a little further, but not
far enough to reduce the number of testsuite regressions to 0.

I plan to submit it for gcc-13, but it won’t make it for gcc-12.

(In reply to Richard Biener from comment #33)
> With
> a single dimension there's not much value in using ARRAY_REF over
> pointer arithmetic and dereference.

I have tried to fix this PR using pointer arithmetic too.
But there are so many places in the frontend where we expect to have an array
type when dereferencing a descriptor pointer, that it’s not as simple as it
seems and I finally convinced myself that the patch attached to this PR was the
best way to go.

For gcc-12, is there a way to add a middle-end workaround using annotations on
descriptor types (a lang flag or something) ?

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenther at suse dot de]

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

--- Comment #36 from rguenther at suse dot de <rguenther at suse dot de> ---
On Sun, 27 Mar 2022, mikael at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043
> 
> --- Comment #35 from Mikael Morin <mikael at gcc dot gnu.org> ---
> A little status update.
> 
> I have pushed the latest patch attached to this PR a little further, but not
> far enough to reduce the number of testsuite regressions to 0.
> 
> I plan to submit it for gcc-13, but it won’t make it for gcc-12.
> 
> (In reply to Richard Biener from comment #33)
> > With
> > a single dimension there's not much value in using ARRAY_REF over
> > pointer arithmetic and dereference.
> 
> I have tried to fix this PR using pointer arithmetic too.
> But there are so many places in the frontend where we expect to have an array
> type when dereferencing a descriptor pointer, that it’s not as simple as it
> seems and I finally convinced myself that the patch attached to this PR was the
> best way to go.

OK.

> For gcc-12, is there a way to add a middle-end workaround using annotations on
> descriptor types (a lang flag or something) ?

I will think of what options we have to work around this in the
middle-end (maybe with help of the frontend during gimplification).

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

--- Comment #37 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #36)
> On Sun, 27 Mar 2022, mikael at gcc dot gnu.org wrote:
> 
> > For gcc-12, is there a way to add a middle-end workaround using annotations on
> > descriptor types (a lang flag or something) ?
> 
> I will think of what options we have to work around this in the
> middle-end (maybe with help of the frontend during gimplification).

So the difficult issue is that the intended effect of the ARRAY_REF is to
apply a negative offset to its base.  That's something fundamentally not
supported for any handled component reference, especially when it is a
non-constant negative offset.  An access is currently constrained to
[offset, offset + max_size] where max_size can be unknown (we use a special
value of -1 for that).  In particular 'offset' cannot be unknown, but when
we split the ultimate base object we do support a constant negative offset
from the real base object.  So we consider a get_base_address() base
plus [0, 0 + unknown] to be a valid conservative answer - but for the
fortran ARRAY_REF indexing it would not be!

Now - actual miscompilations of course require more specific setups, the
ones cited here take advantage of knowing the size of the underlying object
and together with the offset >= 0 constraint constraining the variable index
to exactly the last element of the array.  We already have
-funconstrained-commons to catch a similar case (but with trailing arrays
here).

On the middle-end side it's hard to tell whether the actual array
reference is from a "safe" one (not via an array descriptor and not with
negative stride), so truly fixing it there with lowering the ARRAY_REF
to pointer arithmetic would be a mass-rewrite of even safe accesses.

Another effect besides the wrong constant propagation would be
disambiguation of accesses.  Like if we had

  integer, dimension (10) :: a
  do i = 1, 10
    reshape(a, (/10:1:-1/))(i) = a(i)
  end do

(sorry for my likely invalid fortran, but you get the idea), then
the middle-end would disambiguate all a(i) besides a(10) against the
store.

To sum up, I don't see a good way to workaround this in the middle-end
(without a really big hammer that would do more harm than good).  I also
do not see a way to teach the middle-end negative offsetting ARRAY_REFs,
even if you'd call them differently.

The issue itself is long latent so we probably have to live with GCC 12
exposing slightly more cases of it in the real world (I have yet to see
one there).

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[burnus at gcc dot gnu.org]

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

--- Comment #38 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Thomas Koenig from comment #15)
> One possibility would be to extend the patch Sandra posted at
> https://gcc.gnu.org/pipermail/fortran/2021-January/055563.html
> to scalarization.

As mentioned by Thomas, a re-based patch is
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584716.html

(I did not quickly see whether Mikael's patch c, attachment 51974) is
completely orthogonal, goes in the same direction or a different one.)


Mikael Morin wrote in (comment #35)
> I have tried to fix this PR using pointer arithmetic too.
> But there are so many places in the frontend where we expect to have an
> array type when dereferencing a descriptor pointer

Do you see a special issue here or not?

In that area, I realized when working on my OpenMP deep-mapping patch that
there were issues related to coarrays and CLASS, in particular:
* 'select type' always adds 'attr.pointer' which both permits wrong code (e.g.
deallocate, pointer assignment), but also sets the GFC_ARRAY_POINTER - but
fixing it then caused issues with coarrays.
(GFC_TYPE_ARRAY_AKIND also does not distinguish allocatable/pointer for assumed
rank, but that should be only/mostly a problem for my OpenMP patch.)


I have a continuously growing to-do list, but still: is there something I can
do here?

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[tkoenig at gcc dot gnu.org]

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

--- Comment #39 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #37)

> The issue itself is long latent so we probably have to live with GCC 12
> exposing slightly more cases of it in the real world (I have yet to see
> one there).

Silent introduction of wrong code is always a bad idea.

Should we try to generate an ICE for the test case, so people are at
least warned?

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenther at suse dot de]

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

--- Comment #40 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 30 Mar 2022, tkoenig at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043
> 
> --- Comment #39 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #37)
> 
> > The issue itself is long latent so we probably have to live with GCC 12
> > exposing slightly more cases of it in the real world (I have yet to see
> > one there).
> 
> Silent introduction of wrong code is always a bad idea.
> 
> Should we try to generate an ICE for the test case, so people are at
> least warned?

An ICE for every (possibly!) negative stride array in Fortran?!

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

--- Comment #41 from Richard Biener <rguenth at gcc dot gnu.org> ---
-- random ideas dumping below --

so reading about ISO_Fortran_binding.h it seems to be that a base of the object
isn't readily available and making it available would require quite some
computation.

staying with the current scheme of simply piggy-backing on C [] operator
semantics which is, when facing a pointer that is being indexed, equivalent
to pointer arithmetic which _can_ advance to before the indexed pointer,
looks reasonable.

that rules out using ARRAY_REF for all accesses through a descriptor
(unless all strides are positive - not sure if it's worth special casing that
though).

it might be helpful to have a tree code providing CFI_address in an expanded
form with explicitely specified index, [low-bound,] stride per indexed
dimension and with the guarantee of CFI that the dimensions are independent
(non-overlapping elements).  I'd call it ELEMENT_SELECT_EXPR here, it would
not be a tcc_reference since it only computes an address that would need
to be dereferenced with a MEM_REF.  Since it would be variable-length it
doesn't nicely map to GIMPLE.  Lowering during gimplification would be
possible.  Note we already have a vehicle that should be usable for
the 1-dimensional case, namely TARGET_MEM_REF which allows variable
pointer offsetting with a scaled index.  Of course it would be somewhat
abusing this and dependence analysis doesn't play nicely with it either
AFAIK.  Going for selected ARRAY_REF -> POINTER_PLUS_EXPR + MEM_REF
lowering sounds like the way of least resistance, but then that's probably
backwards of what the OpenACC folks want to do.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

--- Comment #42 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 52717
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52717&action=edit
hack to rewrite all ARRA_REFs

This shows a simple hack emitting *(&array + offset) from gfc_build_array_ref. 
That covers too many cases (as said, covering cases from array descriptor
accesses should be enough).  Appended is also testresults which show besides a
large number of original tree dump scanning fails a few execute FAILs and a few
ICEs where the gfortran frontend doesn't expect anything but an ARRAY_REF to be
fed into select places (some already deal with an INDIRECT_REF as I found out).
One such place is gfc_convert_array_to_string.

As said, all this is probably backwards as to what the OpenACC folks desire.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mikael at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #43 from Mikael Morin <mikael at gcc dot gnu.org> ---
I’m working on it.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #44 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

https://gcc.gnu.org/g:89ca0fffa48b799b228beee48a16e26e24d8e199

commit r12-8227-g89ca0fffa48b799b228beee48a16e26e24d8e199
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Fri Apr 22 22:52:12 2022 +0200

    fortran: Pre-evaluate string pointers. [PR102043]

    This avoids a regression on deferred_character_23.f90 later in the
    patch series when array references are rewritten to use pointer
    arithmetic.

    The problem is a SAVE_EXPR tree as TYPE_SIZE_UNIT of one array element
    type, which is used by the pointer arithmetic expressions.  As these
    expressions appear in both branches of an if-then-else block, the tree
    is lowered to a variable in one of the branches but itâs used in both
    branches, which is invalid middle-end code.

    This change pre-evaluates the array references or pointer arithmetics
    to variables before the if-then-else block, so that the SAVE_EXPR are
    expanded to variables in the parent scope of the if-then-else block,
    and expressions referencing the variables remain valid in both
    branches.

            PR fortran/102043

    gcc/fortran/ChangeLog:
            * trans-expr.cc: Pre-evaluate src and dest to variables
            before using them.

    gcc/testsuite/ChangeLog:
            * gfortran.dg/dependency_49.f90: Update variable occurence
            count.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #45 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

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

commit r12-8228-ge72fbb6915c1dd1a52ecef55e10329e353cc3072
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Fri Apr 22 22:52:26 2022 +0200

    fortran: Update index extraction code. [PR102043]

    This avoids a regression on hollerith4.f90 and hollerith6.f90 later in
    the patch series when code generation for array references is changed
    to use pointer arithmetic.

    The problem comes from the extraction of the array index from an
    ARRAY_REF tree, which doesnât work if the tree is not an ARRAY_REF
    any more.

    This updates the code generated for remaining size evaluation to work
    with a source tree that uses either array indexing or pointer
    arithmetic.

            PR fortran/102043

    gcc/fortran/ChangeLog:

            * trans-io.cc: Add handling for the case where the array
            is referenced using pointer arithmetic.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #46 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

https://gcc.gnu.org/g:761dda57482295f9c41fcf87e5defa2ac1959f03

commit r12-8229-g761dda57482295f9c41fcf87e5defa2ac1959f03
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Fri Apr 22 22:52:38 2022 +0200

    fortran: Generate an array temporary reference [PR102043]

    This avoids regressing on char_cast_1.f90 and char_cast_2.f90 later in
    the patch series when the code generation for array references is
    changed to use pointer arithmetic.

    The regressing testcases match part of an array reference in the
    generated tree dump and itâs not clear how the pattern should be
    rewritten to match the equivalent with pointer arithmetic.

    This change uses a method specific to array temporaries to generate
    array-references, so that these array references are flagged as safe
    for array indexing and will not be updated to use pointer arithmetic.

            PR fortran/102043

    gcc/fortran/ChangeLog:
            * trans-array.cc (gfc_conv_expr_descriptor): Use
            gfc_conv_tmp_array_ref.

[Bug fortran/102043] [9/10/11/12 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #47 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

https://gcc.gnu.org/g:7964ab6c364c410c34efe7ca2eba797d36525349

commit r12-8230-g7964ab6c364c410c34efe7ca2eba797d36525349
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Fri Apr 22 22:52:50 2022 +0200

    fortran: Use pointer arithmetic to index arrays [PR102043]

    The code generated for array references used to be ARRAY_REF trees as
    could be expected.  However, the middle-end may conclude from those
    trees that the indexes used are non-negative (more precisely not below
    the lower bound), which is a wrong assumption in the case of "reversed-
    order" arrays.

    The problematic arrays are those with a descriptor and having a negative
    stride for at least one dimension.  The descriptor data points to the
    first element in array order (which is not the first in memory order in
    that case), and the negative stride(s) makes walking the array backwards
    (towards lower memory addresses), and we can access elements with
    negative index wrt data pointer.

    With this change, pointer arithmetic is generated by default for array
    references, unless we are in a case where negative indexes canât happen
    (array descriptorâs dim element, substrings, explicit shape,
    allocatable, or assumed shape contiguous).  A new flag is added to
    choose between array indexing and pointer arithmetic, and itâs set
    if the context can tell array indexing is safe (descriptor dim
    element, substring, temporary array), or a new method is called
    to decide on whether the flag should be set for one given array
    expression.

            PR fortran/102043

    gcc/fortran/ChangeLog:

            * trans.h (gfc_build_array_ref): Add non_negative_offset
            argument.
            * trans.cc (gfc_build_array_ref): Ditto. Use pointer arithmetic
            if non_negative_offset is false.
            * trans-expr.cc (gfc_conv_substring): Set flag in the call to
            gfc_build_array_ref.
            * trans-array.cc (gfc_get_cfi_dim_item,
            gfc_conv_descriptor_dimension): Same.
            (build_array_ref): Decide on whether to set the flag and update
            the call.
            (gfc_conv_scalarized_array_ref): Same.  New argument tmp_array.
            (gfc_conv_tmp_array_ref): Update call to
            gfc_conv_scalarized_ref.
            (non_negative_strides_array_p): New function.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/array_reference_3.f90: New.
            * gfortran.dg/negative_stride_1.f90: New.
            * gfortran.dg/vector_subscript_8.f90: New.
            * gfortran.dg/vector_subscript_9.f90: New.
            * gfortran.dg/c_loc_test_22.f90: Update dump patterns.
            * gfortran.dg/finalize_10.f90: Same.

    Co-Authored-By: Richard Biener <rguenther@suse.de>

[Bug fortran/102043] [9/10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |12.0
            Summary|[9/10/11/12 Regression]     |[9/10/11 Regression] Wrong
                   |Wrong array types used for  |array types used for
                   |negative stride accesses,   |negative stride accesses,
                   |gfortran.dg/vector_subscrip |gfortran.dg/vector_subscrip
                   |t_1.f90  FAILs              |t_1.f90  FAILs
      Known to fail|12.0                        |
           Priority|P3                          |P4

--- Comment #48 from Richard Biener <rguenth at gcc dot gnu.org> ---
Thanks Mikael!

[Bug fortran/102043] [9/10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #49 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

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

commit r12-8280-g3e0c9fdfd00b5b5cbff1a0bd6ac012a10fe81348
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Wed Apr 27 11:36:00 2022 +0200

    fortran: Avoid infinite self-recursion [PR105381]

    Dummy array decls are local decls different from the argument decl
    accessible through GFC_DECL_SAVED_DESCRIPTOR.  If the argument decl has
    a DECL_LANG_SPECIFIC set, it is copied over to the local decl at the
    time the latter is created, so that the DECL_LANG_SPECIFIC object is
    shared between local dummy decl and argument decl, and thus the
    GFC_DECL_SAVED_DESCRIPTOR of the argument decl is the argument decl
    itself.

    The r12-8230-g7964ab6c364c410c34efe7ca2eba797d36525349 change introduced
    the non_negative_strides_array_p predicate which recurses through
    GFC_DECL_SAVED_DESCRIPTOR to avoid seeing dummy decls as purely local
    decls.  As the GFC_DECL_SAVED_DESCRIPTOR of the argument decl is itself,
    this can cause infinite recursion.

    This change adds a check to avoid infinite recursion.

            PR fortran/102043
            PR fortran/105381

    gcc/fortran/ChangeLog:

            * trans-array.cc (non_negative_strides_array_p): Inline variable
            orig_decl and merge nested if conditions.  Add condition to not
            recurse if the next argument is the same as the current.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/character_array_dummy_1.f90: New test.

[Bug fortran/102043] [10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #50 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug fortran/102043] [10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #51 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug fortran/102043] [10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[mikael at gcc dot gnu.org]

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

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|mikael at gcc dot gnu.org          |unassigned at gcc dot gnu.org
             Status|ASSIGNED                    |NEW

--- Comment #52 from Mikael Morin <mikael at gcc dot gnu.org> ---
I posted a mostly-finished patch at [1] to restore array indexing of descriptor
arrays.
I got an answer suggesting to do the array descriptor reform (aka PR37577) by
the way, if the ABI had to be broken anyway.
This made me realize that the patch was going in the opposite direction as the
long term one the fortran front-end is supposed to take, that is move to an
array descriptor that would always require pointer arithmetics.
So, I’m not working on introducing more array indexing any more.
Unassigning.

[Bug fortran/102043] [10/11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[mikael at gcc dot gnu.org]

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

--- Comment #53 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #52)
> I posted a mostly-finished patch at [1] to restore array indexing of
> descriptor arrays.

I forgot to add a link for [1]:
https://gcc.gnu.org/pipermail/fortran/2022-July/057980.html

[Bug fortran/102043] [11 Regression] Wrong array types used for negative stride accesses, gfortran.dg/vector_subscript_1.f90 FAILs

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #54 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.