[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #32 from Richard Biener --- But maybe I'm misunderstanding what you do - can you point to the respective hunk in the patch?
[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #31 from Richard Biener --- (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 [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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #30 from Mikael Morin --- *** Bug 103671 has been marked as a duplicate of this bug. ***
[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=80645 --- Comment #29 from Mikael Morin --- PR #80645 is possibly related to this.
[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #28 from Mikael Morin --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added Attachment #51891|0 |1 is obsolete|| --- Comment #27 from Mikael Morin --- Created attachment 51974 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51974=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added Attachment #51839|0 |1 is obsolete|| --- Comment #26 from Mikael Morin --- Created attachment 51891 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51891=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Jan Hubicka changed: What|Removed |Added CC||msebor at gcc dot gnu.org --- Comment #25 from Jan Hubicka --- *** Bug 103369 has been marked as a duplicate of this bug. ***
[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #24 from 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 --- > (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 + (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #23 from Mikael Morin --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added Attachment #51791|0 |1 is obsolete|| --- Comment #22 from Mikael Morin --- Created attachment 51839 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51839=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #21 from Richard Biener --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #20 from Thomas Koenig --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #19 from Mikael Morin --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added Attachment #51787|0 |1 is obsolete|| --- Comment #18 from Mikael Morin --- Created attachment 51791 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51791=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #17 from Bernhard Reutner-Fischer --- 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 (, parm, gfc_index_zero_node); else /* Point the data pointer at the 1st element in the section. */ gfc_get_dataptr_offset (, parm, desc, base, subref_array_target, expr); gfc_conv_descriptor_offset_set (, 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043
--- Comment #16 from Bernhard Reutner-Fischer ---
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043
Thomas Koenig changed:
What|Removed |Added
CC||sandra at codesourcery dot com
--- Comment #15 from Thomas Koenig ---
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #14 from Mikael Morin --- Created attachment 51787 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51787=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #13 from Thomas Koenig --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #12 from Mikael Morin --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #11 from Thomas Koenig --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #10 from Richard Biener --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Bernhard Reutner-Fischer changed: What|Removed |Added CC||aldot at gcc dot gnu.org --- Comment #9 from Bernhard Reutner-Fischer --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Mikael Morin changed: What|Removed |Added CC||mikael at gcc dot gnu.org --- Comment #8 from Mikael Morin --- It comes from gfc_conv_expr_descriptor: /* Point the data pointer at the 1st element in the section. */ gfc_get_dataptr_offset (, 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Jan Hubicka changed: What|Removed |Added CC||seurer at gcc dot gnu.org --- Comment #7 from Jan Hubicka --- *** Bug 103142 has been marked as a duplicate of this bug. ***
[Bug fortran/102043] Wrong array types used for negative stride accesses
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:] *)[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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #5 from Richard Biener --- So instead of doing *((T[0:] *)[ubound])[-idx] for accesses do a[ubound - idx]?
[Bug fortran/102043] Wrong array types used for negative stride accesses
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #4 from Richard Biener --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 --- Comment #3 from Eric Botcazou --- > 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Richard Biener changed: What|Removed |Added CC||ebotcazou at gcc dot gnu.org --- Comment #2 from Richard Biener --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102043 Richard Biener 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 --- 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
