* [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) @ 2023-10-20 14:02 Paul-Antoine Arras 2023-10-24 16:12 ` Tobias Burnus 0 siblings, 1 reply; 8+ messages in thread From: Paul-Antoine Arras @ 2023-10-20 14:02 UTC (permalink / raw) To: gcc-patches, fortran [-- Attachment #1: Type: text/plain, Size: 540 bytes --] Hi all, The attached patch fixes a bug that causes valid OpenMP declare variant directive and functions to be rejected with the following error (see testcase): c_ptr.f90:41:37: 41 | !$omp declare variant(foo_variant) & | 1 Error: variant ‘foo_variant’ and base ‘foo’ at (1) have incompatible types: Type mismatch in argument 'c_bv' (INTEGER(8)/TYPE(c_ptr)) The fix consists in special-casing this situation in gfc_compare_types(). OK for mainline? Thanks, -- PA [-- Attachment #2: 0001-Fortran-Fix-incompatible-types-between-INTEGER-8-and.patch --] [-- Type: text/x-patch, Size: 5060 bytes --] From 8e5fa4828678a1388e75795de2a1f253d9f0ec95 Mon Sep 17 00:00:00 2001 From: Paul-Antoine Arras <pa@codesourcery.com> Date: Fri, 20 Oct 2023 12:42:49 +0200 Subject: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) In the context of an OpenMP declare variant directive, arguments of type C_PTR are sometimes recognised as C_PTR in the base function and as INTEGER(8) in the variant - or the other way around, depending on the parsing order. This patch prevents such situation from turning into a compile error. 2023-10-20 Paul-Antoine Arras <pa@codesourcery.com> Tobias Burnus <tobias@codesourcery.com> gcc/fortran/ChangeLog: * interface.cc (gfc_compare_types): Return true in this situation. gcc/testsuite/ChangeLog: * gfortran.dg/c_ptr_tests_20.f90: New test. --- gcc/fortran/ChangeLog.omp | 5 ++ gcc/fortran/interface.cc | 17 +++--- gcc/testsuite/ChangeLog.omp | 4 ++ gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 | 56 ++++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 diff --git a/gcc/fortran/ChangeLog.omp b/gcc/fortran/ChangeLog.omp index 62a33475ee5..299223ceaa7 100644 --- a/gcc/fortran/ChangeLog.omp +++ b/gcc/fortran/ChangeLog.omp @@ -1,3 +1,8 @@ +2023-10-20 Paul-Antoine Arras <pa@codesourcery.com> + Tobias Burnus <tobias@codesourcery.com> + + * interface.cc (gfc_compare_types): Return true in this situation. + 2023-09-19 Tobias Burnus <tobias@codesourcery.com> Backported from master: diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index e9843e9549c..8bd35fd6d22 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -705,12 +705,17 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec *ts2) /* Special case for our C interop types. FIXME: There should be a better way of doing this. When ISO C binding is cleared up, - this can probably be removed. See PR 57048. */ - - if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) - || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) - && ts1->u.derived && ts2->u.derived - && ts1->u.derived == ts2->u.derived) + this can probably be removed. See PR 57048. + Note that this does not distinguish between c_ptr and c_funptr. */ + + if ((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED + && ts1->f90_type == BT_VOID + && ts2->u.derived->ts.is_iso_c + && ts2->u.derived->ts.u.derived->ts.f90_type == BT_VOID) + || (ts2->type == BT_INTEGER && ts1->type == BT_DERIVED + && ts2->f90_type == BT_VOID + && ts1->u.derived->ts.is_iso_c + && ts1->u.derived->ts.u.derived->ts.f90_type == BT_VOID)) return true; /* The _data component is not always present, therefore check for its diff --git a/gcc/testsuite/ChangeLog.omp b/gcc/testsuite/ChangeLog.omp index e9f4d1c63e6..1fc9b0606dc 100644 --- a/gcc/testsuite/ChangeLog.omp +++ b/gcc/testsuite/ChangeLog.omp @@ -1,3 +1,7 @@ +2023-10-20 Paul-Antoine Arras <pa@codesourcery.com> + + * gfortran.dg/c_ptr_tests_20.f90: New test. + 2023-09-20 Tobias Burnus <tobias@codesourcery.com> Backported from master: diff --git a/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 new file mode 100644 index 00000000000..777181cece0 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 @@ -0,0 +1,56 @@ +! { dg-do compile } +! +! This failed to compile the declare variant directive due to the C_PTR +! arguments to foo being recognised as INTEGER(8) + +program adjust_args + use iso_c_binding, only: c_loc + implicit none + + integer, parameter :: N = 1024 + real, allocatable, target :: av(:), bv(:), cv(:) + + call foo(c_loc(bv), c_loc(av), N) + + !$omp target data map(to: av(:N)) map(from: cv(:N)) + !$omp parallel + call foo(c_loc(cv), c_loc(av), N) + !$omp end parallel + !$omp end target data + +contains + subroutine foo_variant(c_d_bv, c_d_av, n) + use iso_c_binding, only: c_ptr, c_f_pointer + type(c_ptr), intent(in) :: c_d_bv, c_d_av + integer, intent(in) :: n + real, pointer :: f_d_bv(:) + real, pointer :: f_d_av(:) + integer :: i + + call c_f_pointer(c_d_bv, f_d_bv, [n]) + call c_f_pointer(c_d_av, f_d_av, [n]) + !$omp target teams loop is_device_ptr(f_d_bv, f_d_av) + do i = 1, n + f_d_bv(i) = f_d_av(i) * i + end do + end subroutine + + + subroutine foo(c_bv, c_av, n) + use iso_c_binding, only: c_ptr, c_f_pointer + type(c_ptr), intent(in) :: c_bv, c_av + integer, intent(in) :: n + real, pointer :: f_bv(:) + real, pointer :: f_av(:) + integer :: i + !$omp declare variant(foo_variant) & + !$omp match(construct={parallel}) + + call c_f_pointer(c_bv, f_bv, [n]) + call c_f_pointer(c_av, f_av, [n]) + !$omp parallel loop + do i = 1, n + f_bv(i) = f_av(i) * i + end do + end subroutine +end program -- 2.42.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-20 14:02 [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) Paul-Antoine Arras @ 2023-10-24 16:12 ` Tobias Burnus 2023-10-26 11:24 ` Paul-Antoine Arras 0 siblings, 1 reply; 8+ messages in thread From: Tobias Burnus @ 2023-10-24 16:12 UTC (permalink / raw) To: fortran, gcc-patches, Paul-Antoine Arras [-- Attachment #1: Type: text/plain, Size: 5596 bytes --] Hi PA, hello all, First, I hesitate to review/approve a patch I am involved in; Thus, I would like if someone could have a second look. Regarding the patch itself: On 20.10.23 16:02, Paul-Antoine Arraswrote: > Hi all, > > The attached patch fixes a bug that causes valid OpenMP declare > variant directive > and functions to be rejected with the following error (see testcase): > > [...] > Error: variant ‘foo_variant’ and base ‘foo’ at (1) have incompatible > types: Type mismatch in argument 'c_bv' (INTEGER(8)/TYPE(c_ptr)) > > The fix consists in special-casing this situation in gfc_compare_types(). > OK for mainline? ... > Subject: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and > TYPE(c_ptr) > > In the context of an OpenMP declare variant directive, arguments of type C_PTR > are sometimes recognised as C_PTR in the base function and as INTEGER(8) in the > variant - or the other way around, depending on the parsing order. > This patch prevents such situation from turning into a compile error. > > 2023-10-20 Paul-Antoine Arras<pa@codesourcery.com> > Tobias Burnus<tobias@codesourcery.com> > > gcc/fortran/ChangeLog: > > * interface.cc (gfc_compare_types): Return true in this situation. That's a bad description. It makes sense when reading the commit log but if you only read gcc/fortran/ChangeLog, 'this situation' is a dangling reference. > gcc/fortran/ChangeLog.omp | 5 ++ > gcc/testsuite/ChangeLog.omp | 4 ++ On mainline, the ChangeLog not ChangeLog.omp is used. This changelog is automatically filled by the data in the commit log. Thus, no need to include it in the patch. (Besides: It keeps getting outdated by any other commit to that file.) As you have a commit, running the following, possibly with the commit hash as argument (unless it is the last commit) will show that the nightly script will use: ./contrib/gcc-changelog/git_check_commit.py -v -p It is additionally a good check whether you got the syntax right. (This is run as pre-commit hook.) * * * Regarding the patch, I think it will work, but I wonder whether we can do better - esp. regarding c_ptr vs. c_funptr. I started by looking why the current code fails: > index e9843e9549c..8bd35fd6d22 100644 > --- a/gcc/fortran/interface.cc > +++ b/gcc/fortran/interface.cc > @@ -705,12 +705,17 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec *ts2) > - > - if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) > - || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) > - && ts1->u.derived && ts2->u.derived > - && ts1->u.derived == ts2->u.derived) This does not work because the pointers to the derived type are different: (gdb) p *ts1 $10 = {type = BT_INTEGER, kind = 8, u = {derived = 0x30c66b0, cl = 0x30c66b0, pad = 51144368}, interface = 0x0, is_c_interop = 1, is_iso_c = 0, f90_type = BT_VOID, deferred = false, interop_kind = 0x0} (gdb) p *ts2 $11 = {type = BT_DERIVED, kind = 0, u = {derived = 0x30c2930, cl = 0x30c2930, pad = 51128624}, interface = 0x0, is_c_interop = 0, is_iso_c = 0, f90_type = BT_UNKNOWN, deferred = false, interop_kind = 0x0} The reason seems to be that they are freshly created in different namespaces. Consequently, attr.use_assoc = 1 and the namespace is different, i.e. (gdb) p ts1->u.derived->ns->proc_name->name $18 = 0x7ffff6ff4138 "foo" (gdb) p ts2->u.derived->ns->proc_name->name $19 = 0x7ffff6ffc260 "foo_variant" * * * Having said this, I think we can combine the current and the modified version, i.e. > + if ((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED > + && ts1->f90_type == BT_VOID > + && ts2->u.derived->ts.is_iso_c > + && ts2->u.derived->ts.u.derived->ts.f90_type == BT_VOID) > + || (ts2->type == BT_INTEGER && ts1->type == BT_DERIVED > + && ts2->f90_type == BT_VOID > + && ts1->u.derived->ts.is_iso_c > + && ts1->u.derived->ts.u.derived->ts.f90_type == BT_VOID)) See attached patch for a combined version, which checks now whether from_intmod == INTMOD_ISO_C_BINDING and then compares the names (to distinguish c_ptr and c_funptr). Those are unaffected by 'use' renames, hence, we should be fine. While in this example, the name pointers are identical, I fear that won't hold in some more complex indirect use via module-use. Thus, strcmp is used. (gdb) p ts1->u.derived->name $13 = 0x7ffff6ff4100 "c_ptr" (gdb) p ts2->u.derived->name $14 = 0x7ffff6ff4100 "c_ptr" * * * Additionally, I think it would be good to have a testcase which checks for c_funptr vs. c_ptr mismatch. Just changing c_ptr to c_funptr in the testcase (+ commenting the c_f_pointer) prints: Error: variant ‘foo_variant’ and base ‘foo’ at (1) have incompatible types: Type mismatch in argument 'c_bv' (INTEGER(8)/TYPE(c_funptr)) I think that would be a good invalid testcase besides the valid one. But with a tweak to get better messages to give: Error: variant ‘foo_variant’ and base ‘foo’ at (1) have incompatible types: Type mismatch in argument 'c_bv' (TYPE(c_ptr)/TYPE(c_funptr)) cf. misc.cc in the attached proposal for the *.cc files, only. Tobias ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 [-- Attachment #2: proposed.diff --] [-- Type: text/x-patch, Size: 1937 bytes --] gcc/fortran/interface.cc | 16 ++++++++++++---- gcc/fortran/misc.cc | 7 ++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index c01df0460d7..8c4571e0aa6 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -736,10 +736,18 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec *ts2) better way of doing this. When ISO C binding is cleared up, this can probably be removed. See PR 57048. */ - if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) - || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) - && ts1->u.derived && ts2->u.derived - && ts1->u.derived == ts2->u.derived) + if ((ts1->type == BT_INTEGER + && ts2->type == BT_DERIVED + && ts1->f90_type == BT_VOID + && ts2->u.derived->from_intmod == INTMOD_ISO_C_BINDING + && ts1->u.derived + && strcmp (ts1->u.derived->name, ts2->u.derived->name) == 0) + || (ts2->type == BT_INTEGER + && ts1->type == BT_DERIVED + && ts2->f90_type == BT_VOID + && ts1->u.derived->from_intmod == INTMOD_ISO_C_BINDING + && ts2->u.derived + && strcmp (ts1->u.derived->name, ts2->u.derived->name) == 0)) return true; /* The _data component is not always present, therefore check for its diff --git a/gcc/fortran/misc.cc b/gcc/fortran/misc.cc index bae6d292dc5..edffba07013 100644 --- a/gcc/fortran/misc.cc +++ b/gcc/fortran/misc.cc @@ -138,7 +138,12 @@ gfc_typename (gfc_typespec *ts, bool for_hash) switch (ts->type) { case BT_INTEGER: - sprintf (buffer, "INTEGER(%d)", ts->kind); + if (ts->f90_type == BT_VOID + && ts->u.derived + && ts->u.derived->from_intmod == INTMOD_ISO_C_BINDING) + sprintf (buffer, "TYPE(%s)", ts->u.derived->name); + else + sprintf (buffer, "INTEGER(%d)", ts->kind); break; case BT_REAL: sprintf (buffer, "REAL(%d)", ts->kind); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-24 16:12 ` Tobias Burnus @ 2023-10-26 11:24 ` Paul-Antoine Arras 2023-10-26 12:33 ` Tobias Burnus 2023-10-26 16:16 ` Thomas Schwinge 0 siblings, 2 replies; 8+ messages in thread From: Paul-Antoine Arras @ 2023-10-26 11:24 UTC (permalink / raw) To: Tobias Burnus, fortran, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1483 bytes --] Hi Tobias, Please see the updated patch attached incorporating your input and details below. On 24/10/2023 18:12, you wrote: > On 20.10.23 16:02, Paul-Antoine Arras wrote: >> gcc/fortran/ChangeLog: >> >> * interface.cc (gfc_compare_types): Return true in this situation. > > That's a bad description. It makes sense when reading the commit log but > if you > only read gcc/fortran/ChangeLog, 'this situation' is a dangling reference. Updated Changelog with a more helpful description. >> gcc/fortran/ChangeLog.omp | 5 ++ >> gcc/testsuite/ChangeLog.omp | 4 ++ > > On mainline, the ChangeLog not ChangeLog.omp is used. This changelog is > automatically > filled by the data in the commit log. Thus, no need to include it in the > patch. Removed ChangeLog.omp from the patch. > See attached patch for a combined version, which checks now > whether from_intmod == INTMOD_ISO_C_BINDING and then compares > the names (to distinguish c_ptr and c_funptr). Those are unaffected > by 'use' renames, hence, we should be fine. Added the proposed diff for interface.cc and misc.cc to the patch. > Additionally, I think it would be good to have a testcase which checks for > c_funptr vs. c_ptr > mismatch. Added new testcase c_ptr_tests_21.f90 to check that incompatibilities between c_funptr vs. c_ptr are properly reported. Is this latest revision ready to commit? Thanks, -- PA [-- Attachment #2: 0001-Fortran-Fix-incompatible-types-between-INTEGER-8-and.patch --] [-- Type: text/x-patch, Size: 7172 bytes --] From 691d1050ce39c27231dc610b799bf180871820b8 Mon Sep 17 00:00:00 2001 From: Paul-Antoine Arras <pa@codesourcery.com> Date: Fri, 20 Oct 2023 12:42:49 +0200 Subject: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) In the context of an OpenMP declare variant directive, arguments of type C_PTR are sometimes recognised as C_PTR in the base function and as INTEGER(8) in the variant - or the other way around, depending on the parsing order. This patch prevents such situation from turning into a compile error. 2023-10-20 Paul-Antoine Arras <pa@codesourcery.com> Tobias Burnus <tobias@codesourcery.com> gcc/fortran/ChangeLog: * interface.cc (gfc_compare_types): Return true if one type is C_PTR and the other is a compatible INTEGER(8). * misc.cc (gfc_typename): Handle the case where an INTEGER(8) actually holds a TYPE(C_PTR). gcc/testsuite/ChangeLog: * gfortran.dg/c_ptr_tests_20.f90: New test, checking that INTEGER(8) and TYPE(C_PTR) are recognised as compatible. * gfortran.dg/c_ptr_tests_21.f90: New test, exercising the error detection for C_FUNPTR. --- gcc/fortran/interface.cc | 16 ++++-- gcc/fortran/misc.cc | 7 ++- gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 | 57 ++++++++++++++++++++ gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 | 57 ++++++++++++++++++++ 4 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index e9843e9549c..ed1613b16fb 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -707,10 +707,18 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec *ts2) better way of doing this. When ISO C binding is cleared up, this can probably be removed. See PR 57048. */ - if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) - || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) - && ts1->u.derived && ts2->u.derived - && ts1->u.derived == ts2->u.derived) + if ((ts1->type == BT_INTEGER + && ts2->type == BT_DERIVED + && ts1->f90_type == BT_VOID + && ts2->u.derived->from_intmod == INTMOD_ISO_C_BINDING + && ts1->u.derived + && strcmp (ts1->u.derived->name, ts2->u.derived->name) == 0) + || (ts2->type == BT_INTEGER + && ts1->type == BT_DERIVED + && ts2->f90_type == BT_VOID + && ts1->u.derived->from_intmod == INTMOD_ISO_C_BINDING + && ts2->u.derived + && strcmp (ts1->u.derived->name, ts2->u.derived->name) == 0)) return true; /* The _data component is not always present, therefore check for its diff --git a/gcc/fortran/misc.cc b/gcc/fortran/misc.cc index bae6d292dc5..edffba07013 100644 --- a/gcc/fortran/misc.cc +++ b/gcc/fortran/misc.cc @@ -138,7 +138,12 @@ gfc_typename (gfc_typespec *ts, bool for_hash) switch (ts->type) { case BT_INTEGER: - sprintf (buffer, "INTEGER(%d)", ts->kind); + if (ts->f90_type == BT_VOID + && ts->u.derived + && ts->u.derived->from_intmod == INTMOD_ISO_C_BINDING) + sprintf (buffer, "TYPE(%s)", ts->u.derived->name); + else + sprintf (buffer, "INTEGER(%d)", ts->kind); break; case BT_REAL: sprintf (buffer, "REAL(%d)", ts->kind); diff --git a/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 new file mode 100644 index 00000000000..7dd510400f3 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 @@ -0,0 +1,57 @@ +! { dg-do compile } +! { dg-additional-options "-fopenmp" } +! +! This failed to compile the declare variant directive due to the C_PTR +! arguments to foo being recognised as INTEGER(8) + +program adjust_args + use iso_c_binding, only: c_loc + implicit none + + integer, parameter :: N = 1024 + real, allocatable, target :: av(:), bv(:), cv(:) + + call foo(c_loc(bv), c_loc(av), N) + + !$omp target data map(to: av(:N)) map(from: cv(:N)) + !$omp parallel + call foo(c_loc(cv), c_loc(av), N) + !$omp end parallel + !$omp end target data + +contains + subroutine foo_variant(c_d_bv, c_d_av, n) + use iso_c_binding, only: c_ptr, c_f_pointer + type(c_ptr), intent(in) :: c_d_bv, c_d_av + integer, intent(in) :: n + real, pointer :: f_d_bv(:) + real, pointer :: f_d_av(:) + integer :: i + + call c_f_pointer(c_d_bv, f_d_bv, [n]) + call c_f_pointer(c_d_av, f_d_av, [n]) + !$omp target teams loop is_device_ptr(f_d_bv, f_d_av) + do i = 1, n + f_d_bv(i) = f_d_av(i) * i + end do + end subroutine + + + subroutine foo(c_bv, c_av, n) + use iso_c_binding, only: c_ptr, c_f_pointer + type(c_ptr), intent(in) :: c_bv, c_av + integer, intent(in) :: n + real, pointer :: f_bv(:) + real, pointer :: f_av(:) + integer :: i + !$omp declare variant(foo_variant) & + !$omp match(construct={parallel}) + + call c_f_pointer(c_bv, f_bv, [n]) + call c_f_pointer(c_av, f_av, [n]) + !$omp parallel loop + do i = 1, n + f_bv(i) = f_av(i) * i + end do + end subroutine +end program diff --git a/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 b/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 new file mode 100644 index 00000000000..05ccb771eee --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 @@ -0,0 +1,57 @@ +! { dg-do compile } +! { dg-additional-options "-fopenmp" } +! +! Ensure that C_PTR and C_FUNPTR are reported as incompatible types in variant +! argument lists + +program adjust_args + use iso_c_binding, only: c_loc + implicit none + + integer, parameter :: N = 1024 + real, allocatable, target :: av(:), bv(:), cv(:) + + call foo(c_loc(bv), c_loc(av), N) + + !$omp target data map(to: av(:N)) map(from: cv(:N)) + !$omp parallel + call foo(c_loc(cv), c_loc(av), N) + !$omp end parallel + !$omp end target data + +contains + subroutine foo_variant(c_d_bv, c_d_av, n) + use iso_c_binding, only: c_funptr, c_f_pointer + type(c_funptr), intent(in) :: c_d_bv, c_d_av + integer, intent(in) :: n + real, pointer :: f_d_bv(:) + real, pointer :: f_d_av(:) + integer :: i + +! call c_f_pointer(c_d_bv, f_d_bv, [n]) +! call c_f_pointer(c_d_av, f_d_av, [n]) + !$omp target teams loop is_device_ptr(f_d_bv, f_d_av) + do i = 1, n + f_d_bv(i) = f_d_av(i) * i + end do + end subroutine + + + subroutine foo(c_bv, c_av, n) + use iso_c_binding, only: c_ptr, c_f_pointer + type(c_ptr), intent(in) :: c_bv, c_av + integer, intent(in) :: n + real, pointer :: f_bv(:) + real, pointer :: f_av(:) + integer :: i + !$omp declare variant(foo_variant) & ! { dg-error "variant 'foo_variant' and base 'foo' at .1. have incompatible types: Type mismatch in argument 'c_bv' .TYPE.c_ptr./TYPE.c_funptr.." } + !$omp match(construct={parallel}) + + call c_f_pointer(c_bv, f_bv, [n]) + call c_f_pointer(c_av, f_av, [n]) + !$omp parallel loop + do i = 1, n + f_bv(i) = f_av(i) * i + end do + end subroutine +end program -- 2.42.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-26 11:24 ` Paul-Antoine Arras @ 2023-10-26 12:33 ` Tobias Burnus 2023-10-26 16:16 ` Thomas Schwinge 1 sibling, 0 replies; 8+ messages in thread From: Tobias Burnus @ 2023-10-26 12:33 UTC (permalink / raw) To: Paul-Antoine Arras, fortran, gcc-patches Hi Paul-Antoine, On 26.10.23 13:24, Paul-Antoine Arras wrote: > Please see the updated patch attached incorporating your input and > details below. ... > Is this latest revision ready to commit? LGTM. Thanks, Tobias ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-26 11:24 ` Paul-Antoine Arras 2023-10-26 12:33 ` Tobias Burnus @ 2023-10-26 16:16 ` Thomas Schwinge 2023-10-26 16:28 ` Paul-Antoine Arras 1 sibling, 1 reply; 8+ messages in thread From: Thomas Schwinge @ 2023-10-26 16:16 UTC (permalink / raw) To: Paul-Antoine Arras, Tobias Burnus; +Cc: fortran, gcc-patches Hi! On 2023-10-26T13:24:04+0200, Paul-Antoine Arras <pa@codesourcery.com> wrote: > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 > @@ -0,0 +1,57 @@ > +! { dg-do compile } > +! { dg-additional-options "-fopenmp" } > +[...] > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 > @@ -0,0 +1,57 @@ > +! { dg-do compile } > +! { dg-additional-options "-fopenmp" } > +[...] OpenMP is not universally supported across different GCC configurations, so this will FAIL for some. Therefore, please either guard with effective target: @item fopenmp Target supports OpenMP via @option{-fopenmp}. ..., or move into 'gcc/testsuite/gfortran.dg/gomp/' (may then remove explicit 'dg-additional-options "-fopenmp"'). I don't know which variant makes more sense, here. Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-26 16:16 ` Thomas Schwinge @ 2023-10-26 16:28 ` Paul-Antoine Arras 2023-10-26 16:49 ` Thomas Schwinge 2023-10-26 17:00 ` tobias.burnus 0 siblings, 2 replies; 8+ messages in thread From: Paul-Antoine Arras @ 2023-10-26 16:28 UTC (permalink / raw) To: Thomas Schwinge, Tobias Burnus; +Cc: fortran, gcc-patches Hi Thomas, On 26/10/2023 18:16, you wrote: > Hi! > > On 2023-10-26T13:24:04+0200, Paul-Antoine Arras <pa@codesourcery.com> wrote: >> --- /dev/null >> +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 >> @@ -0,0 +1,57 @@ >> +! { dg-do compile } >> +! { dg-additional-options "-fopenmp" } >> +[...] > >> --- /dev/null >> +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 >> @@ -0,0 +1,57 @@ >> +! { dg-do compile } >> +! { dg-additional-options "-fopenmp" } >> +[...] > > OpenMP is not universally supported across different GCC configurations, > so this will FAIL for some. Therefore, please either guard with > effective target: > > @item fopenmp > Target supports OpenMP via @option{-fopenmp}. > Would the following be enough? > diff --git gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 > index 7dd510400f3..131603d3819 100644 > --- gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 > +++ gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 > @@ -1,4 +1,5 @@ > ! { dg-do compile } > +! { dg-require-effective-target fopenmp } > ! { dg-additional-options "-fopenmp" } > ! > ! This failed to compile the declare variant directive due to the C_PTR > diff --git gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 > index 05ccb771eee..060d29d0275 100644 > --- gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 > +++ gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 > @@ -1,4 +1,5 @@ > ! { dg-do compile } > +! { dg-require-effective-target fopenmp } > ! { dg-additional-options "-fopenmp" } > ! > ! Ensure that C_PTR and C_FUNPTR are reported as incompatible types in variant Thanks, -- PA ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-26 16:28 ` Paul-Antoine Arras @ 2023-10-26 16:49 ` Thomas Schwinge 2023-10-26 17:00 ` tobias.burnus 1 sibling, 0 replies; 8+ messages in thread From: Thomas Schwinge @ 2023-10-26 16:49 UTC (permalink / raw) To: Paul-Antoine Arras; +Cc: fortran, gcc-patches, Tobias Burnus Hi PA! On 2023-10-26T18:28:07+0200, Paul-Antoine Arras <pa@codesourcery.com> wrote: > On 26/10/2023 18:16, you wrote: >> On 2023-10-26T13:24:04+0200, Paul-Antoine Arras <pa@codesourcery.com> wrote: >>> --- /dev/null >>> +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 >>> @@ -0,0 +1,57 @@ >>> +! { dg-do compile } >>> +! { dg-additional-options "-fopenmp" } >>> +[...] >> >>> --- /dev/null >>> +++ b/gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 >>> @@ -0,0 +1,57 @@ >>> +! { dg-do compile } >>> +! { dg-additional-options "-fopenmp" } >>> +[...] >> >> OpenMP is not universally supported across different GCC configurations, >> so this will FAIL for some. Therefore, please either guard with >> effective target: >> >> @item fopenmp >> Target supports OpenMP via @option{-fopenmp}. >> > > Would the following be enough? > >> diff --git gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 >> index 7dd510400f3..131603d3819 100644 >> --- gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 >> +++ gcc/testsuite/gfortran.dg/c_ptr_tests_20.f90 >> @@ -1,4 +1,5 @@ >> ! { dg-do compile } >> +! { dg-require-effective-target fopenmp } >> ! { dg-additional-options "-fopenmp" } >> ! >> ! This failed to compile the declare variant directive due to the C_PTR >> diff --git gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 >> index 05ccb771eee..060d29d0275 100644 >> --- gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 >> +++ gcc/testsuite/gfortran.dg/c_ptr_tests_21.f90 >> @@ -1,4 +1,5 @@ >> ! { dg-do compile } >> +! { dg-require-effective-target fopenmp } >> ! { dg-additional-options "-fopenmp" } >> ! >> ! Ensure that C_PTR and C_FUNPTR are reported as incompatible types in variant Yes, that looks good to me -- you may push "as obvious". Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) 2023-10-26 16:28 ` Paul-Antoine Arras 2023-10-26 16:49 ` Thomas Schwinge @ 2023-10-26 17:00 ` tobias.burnus 1 sibling, 0 replies; 8+ messages in thread From: tobias.burnus @ 2023-10-26 17:00 UTC (permalink / raw) To: Paul-Antoine Arras, Thomas Schwinge; +Cc: fortran, gcc-patches [-- Attachment #1: Type: text/plain, Size: 407 bytes --] Missed that, sorry. Please move the test to gfortran.dg/gomp/ — those tests are automatically compiled with -fopenmp, hence, no need for dg-(additional-)options. Not applicable here, but tests that use omp.h or "use omp_lib" or the runtime have to be under libgomp/testsuite - or, for compile checks (esp. for is-invalid diagnostic), the required decls have to be copied into the test file. Tobias ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-26 17:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-10-20 14:02 [PATCH] Fortran: Fix incompatible types between INTEGER(8) and TYPE(c_ptr) Paul-Antoine Arras 2023-10-24 16:12 ` Tobias Burnus 2023-10-26 11:24 ` Paul-Antoine Arras 2023-10-26 12:33 ` Tobias Burnus 2023-10-26 16:16 ` Thomas Schwinge 2023-10-26 16:28 ` Paul-Antoine Arras 2023-10-26 16:49 ` Thomas Schwinge 2023-10-26 17:00 ` tobias.burnus
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).