* [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).