* PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish
@ 2021-01-13 19:10 Harris Snyder
2021-01-27 19:38 ` Harris Snyder
0 siblings, 1 reply; 4+ messages in thread
From: Harris Snyder @ 2021-01-13 19:10 UTC (permalink / raw)
To: Tobias Burnus, fortran
Hi Tobias / all,
Further related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93524
`sm` is being incorrectly computed in CFI_establish. Take a look at
the diff below - we are currently only using the extent of the
previous rank to assign `sm`, instead of all previous ranks. Have I
got this right, or am I missing something / does this need to be
handled differently? I can offer some test cases and submit a proper
patch if we think this solution is OK...
Thanks,
Harris
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
b/libgfortran/runtime/ISO_Fortran_binding.c
index 3746ec1c681..20833ad2025 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -391,7 +391,12 @@ int CFI_establish (CFI_cdesc_t *dv, void
*base_addr, CFI_attribute_t attribute,
if (i == 0)
dv->dim[i].sm = dv->elem_len;
else
- dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents[i - 1]);
+ {
+ CFI_index_t extents_product = 1;
+ for (int j = 0; j < i; j++)
+ extents_product *= extents[j];
+ dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents_product);
+ }
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish
2021-01-13 19:10 PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish Harris Snyder
@ 2021-01-27 19:38 ` Harris Snyder
2021-01-27 21:21 ` [PATCH Fortran] " Harris Snyder
0 siblings, 1 reply; 4+ messages in thread
From: Harris Snyder @ 2021-01-27 19:38 UTC (permalink / raw)
To: Tobias Burnus, fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
Hi all,
Now that my copyright assignment is complete, I'm submitting this fix.
Test cases are included.
OK for master? I do not have write access, so someone will need to
commit this for me.
Regards,
Harris
libgfortran/ChangeLog:
* runtime/ISO_Fortran_binding.c (CFI_establish): fixed strides
for rank >2 arrays
gcc/testsuite/ChangeLog:
* gfortran.dg/ISO_Fortran_binding_18.c: New test.
* gfortran.dg/ISO_Fortran_binding_18.f90: New test.
On Wed, Jan 13, 2021 at 2:10 PM Harris Snyder <hsnyder@structura.bio> wrote:
>
> Hi Tobias / all,
>
> Further related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93524
> `sm` is being incorrectly computed in CFI_establish. Take a look at
> the diff below - we are currently only using the extent of the
> previous rank to assign `sm`, instead of all previous ranks. Have I
> got this right, or am I missing something / does this need to be
> handled differently? I can offer some test cases and submit a proper
> patch if we think this solution is OK...
>
> Thanks,
> Harris
>
> diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
> b/libgfortran/runtime/ISO_Fortran_binding.c
> index 3746ec1c681..20833ad2025 100644
> --- a/libgfortran/runtime/ISO_Fortran_binding.c
> +++ b/libgfortran/runtime/ISO_Fortran_binding.c
> @@ -391,7 +391,12 @@ int CFI_establish (CFI_cdesc_t *dv, void
> *base_addr, CFI_attribute_t attribute,
> if (i == 0)
> dv->dim[i].sm = dv->elem_len;
> else
> - dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents[i - 1]);
> + {
> + CFI_index_t extents_product = 1;
> + for (int j = 0; j < i; j++)
> + extents_product *= extents[j];
> + dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents_product);
> + }
> }
> }
[-- Attachment #2: hsnyder_iso_fortran_binding2.patch --]
[-- Type: application/x-patch, Size: 2662 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH Fortran] Re: PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish
2021-01-27 19:38 ` Harris Snyder
@ 2021-01-27 21:21 ` Harris Snyder
2021-01-27 21:59 ` Thomas Koenig
0 siblings, 1 reply; 4+ messages in thread
From: Harris Snyder @ 2021-01-27 21:21 UTC (permalink / raw)
To: Tobias Burnus, fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]
(re-sending with subject line tags)
Hi all,
Now that my copyright assignment is complete, I'm submitting this fix.
Test cases are included.
OK for master? I do not have write access, so someone will need to
commit this for me.
Regards,
Harris
libgfortran/ChangeLog:
* runtime/ISO_Fortran_binding.c (CFI_establish): fixed strides
for rank >2 arrays
gcc/testsuite/ChangeLog:
* gfortran.dg/ISO_Fortran_binding_18.c: New test.
* gfortran.dg/ISO_Fortran_binding_18.f90: New test.
> On Wed, Jan 13, 2021 at 2:10 PM Harris Snyder <hsnyder@structura.bio> wrote:
> >
> > Hi Tobias / all,
> >
> > Further related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93524
> > `sm` is being incorrectly computed in CFI_establish. Take a look at
> > the diff below - we are currently only using the extent of the
> > previous rank to assign `sm`, instead of all previous ranks. Have I
> > got this right, or am I missing something / does this need to be
> > handled differently? I can offer some test cases and submit a proper
> > patch if we think this solution is OK...
> >
> > Thanks,
> > Harris
> >
> > diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
> > b/libgfortran/runtime/ISO_Fortran_binding.c
> > index 3746ec1c681..20833ad2025 100644
> > --- a/libgfortran/runtime/ISO_Fortran_binding.c
> > +++ b/libgfortran/runtime/ISO_Fortran_binding.c
> > @@ -391,7 +391,12 @@ int CFI_establish (CFI_cdesc_t *dv, void
> > *base_addr, CFI_attribute_t attribute,
> > if (i == 0)
> > dv->dim[i].sm = dv->elem_len;
> > else
> > - dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents[i - 1]);
> > + {
> > + CFI_index_t extents_product = 1;
> > + for (int j = 0; j < i; j++)
> > + extents_product *= extents[j];
> > + dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents_product);
> > + }
> > }
> > }
[-- Attachment #2: hsnyder_iso_fortran_binding2.patch --]
[-- Type: text/x-patch, Size: 2662 bytes --]
commit 451bd40aca006ebdba52553de2392fcb5b1ff42f
Author: Harris M. Snyder <harris.snyder@gmail.com>
Date: Tue Jan 26 23:29:24 2021 -0500
Partial fix for PR fortran/93524
diff --git a/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.c b/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.c
new file mode 100644
index 00000000000..4d1c4ecbd72
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.c
@@ -0,0 +1,29 @@
+#include <ISO_Fortran_binding.h>
+
+#include <stdlib.h>
+#include <string.h>
+
+
+
+extern int do_loop(CFI_cdesc_t* array);
+
+int main(int argc, char ** argv)
+{
+ int nx = 9;
+ int ny = 10;
+ int nz = 2;
+
+ int arr[nx*ny*nz];
+ memset(arr,0,sizeof(int)*nx*ny*nz);
+ CFI_index_t shape[3];
+ shape[0] = nz;
+ shape[1] = ny;
+ shape[2] = nx;
+
+ CFI_CDESC_T(3) farr;
+ int rc = CFI_establish((CFI_cdesc_t*)&farr, arr, CFI_attribute_other, CFI_type_int, 0, (CFI_rank_t)3, (const CFI_index_t *)shape);
+ if (rc != CFI_SUCCESS) abort();
+ int result = do_loop((CFI_cdesc_t*)&farr);
+ if (result != nx*ny*nz) abort();
+ return 0;
+}
diff --git a/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.f90 b/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.f90
new file mode 100644
index 00000000000..76be51d22fb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_18.f90
@@ -0,0 +1,28 @@
+! { dg-do run }
+! { dg-additional-sources ISO_Fortran_binding_18.c }
+
+module fortran_binding_test_18
+ use iso_c_binding
+ implicit none
+contains
+
+ subroutine test(array)
+ integer(c_int) :: array(:)
+ array = 1
+ end subroutine
+
+ function do_loop(array) result(the_sum) bind(c)
+ integer(c_int), intent(in out) :: array(:,:,:)
+ integer(c_int) :: the_sum, i, j
+
+ the_sum = 0
+ array = 0
+ do i=1,size(array,3)
+ do j=1,size(array,2)
+ call test(array(:,j,i))
+ end do
+ end do
+ the_sum = sum(array)
+ end function
+
+end module
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 3746ec1c681..20833ad2025 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -391,7 +391,12 @@ int CFI_establish (CFI_cdesc_t *dv, void *base_addr, CFI_attribute_t attribute,
if (i == 0)
dv->dim[i].sm = dv->elem_len;
else
- dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents[i - 1]);
+ {
+ CFI_index_t extents_product = 1;
+ for (int j = 0; j < i; j++)
+ extents_product *= extents[j];
+ dv->dim[i].sm = (CFI_index_t)(dv->elem_len * extents_product);
+ }
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH Fortran] Re: PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish
2021-01-27 21:21 ` [PATCH Fortran] " Harris Snyder
@ 2021-01-27 21:59 ` Thomas Koenig
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Koenig @ 2021-01-27 21:59 UTC (permalink / raw)
To: Harris Snyder, Tobias Burnus, fortran, gcc-patches
Hi Harris!
> OK for master? I do not have write access, so someone will need to
> commit this for me.
Reviewed, regression-tested and committed as
https://gcc.gnu.org/g:1cdca4261e88f4dc9c3293c6b3c2fff3071ca32b
Thanks for your patch, and welcome aboard!
Best regards
Thomas
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-27 21:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 19:10 PR fortran/93524 - rank >= 3 array stride incorrectly set in CFI_establish Harris Snyder
2021-01-27 19:38 ` Harris Snyder
2021-01-27 21:21 ` [PATCH Fortran] " Harris Snyder
2021-01-27 21:59 ` Thomas Koenig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).