* [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
@ 2017-11-29 10:03 Paul Richard Thomas
0 siblings, 0 replies; 18+ messages in thread
From: Paul Richard Thomas @ 2017-11-29 10:03 UTC (permalink / raw)
To: fortran, gcc-patches, Andre Vehreschild
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
This problem is not really a regression but is a "feature" that was
exposed by my patch for PR81447.
The testcase fails because the caf token for the pointer component is
not present in the type. This is fixed in trans-types.c
(gfc_get_derived_type) in the manner described in the ChangeLog.
Bootstrapped and regtested on FC23/x86_64 - OK for trunk?
I would be grateful if caf aficionados would give the patch a whirl on
their favourite codes.
Cheers
Paul
2017-11-29 Paul Thomas <pault@gcc.gnu.org>
PR fortran/83076
* trans-types.c (gfc_get_derived_type): Flag GFC_FCOARRAY_LIB
for module derived types that are not vtypes. Use this flag to
use the module backend_decl as the canonical type and to build
the type anew, ensuring that scalar allocatable and pointer
components have the caf token field added.
2017-11-29 Paul Thomas <pault@gcc.gnu.org>
PR fortran/83076
* gfortran.dg/coarray_45.f90 : New test.
[-- Attachment #2: submit.diff --]
[-- Type: text/plain, Size: 3132 bytes --]
Index: gcc/fortran/trans-types.c
===================================================================
*** gcc/fortran/trans-types.c (revision 255161)
--- gcc/fortran/trans-types.c (working copy)
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2483,2488 ****
--- 2483,2492 ----
gfc_dt_list *dt;
gfc_namespace *ns;
tree tmp;
+ bool coarray_flag;
+
+ coarray_flag = flag_coarray == GFC_FCOARRAY_LIB
+ && derived->module && !derived->attr.vtype;
gcc_assert (!derived->attr.pdt_template);
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2523,2534 ****
return derived->backend_decl;
}
! /* If use associated, use the module type for this one. */
if (derived->backend_decl == NULL
&& derived->attr.use_assoc
&& derived->module
&& gfc_get_module_backend_decl (derived))
! goto copy_derived_types;
/* The derived types from an earlier namespace can be used as the
canonical type. */
--- 2527,2545 ----
return derived->backend_decl;
}
! /* If use associated, use the module type for this one, except for the case
! where codimensions are present or where a caf_token is needed for pointer
! or allocatable components. */
if (derived->backend_decl == NULL
&& derived->attr.use_assoc
&& derived->module
&& gfc_get_module_backend_decl (derived))
! {
! if (coarray_flag || codimen)
! got_canonical = true;
! else
! goto copy_derived_types;
! }
/* The derived types from an earlier namespace can be used as the
canonical type. */
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2764,2770 ****
GFC_DECL_PTR_ARRAY_P (c->backend_decl) = 1;
/* Do not add a caf_token field for classes' data components. */
! if (codimen && !c->attr.dimension && !c->attr.codimension
&& (c->attr.allocatable || c->attr.pointer)
&& c->caf_token == NULL_TREE && strcmp ("_data", c->name) != 0)
{
--- 2775,2782 ----
GFC_DECL_PTR_ARRAY_P (c->backend_decl) = 1;
/* Do not add a caf_token field for classes' data components. */
! if ((codimen || coarray_flag)
! && !c->attr.dimension && !c->attr.codimension
&& (c->attr.allocatable || c->attr.pointer)
&& c->caf_token == NULL_TREE && strcmp ("_data", c->name) != 0)
{
Index: gcc/testsuite/gfortran.dg/coarray_45.f90
===================================================================
*** gcc/testsuite/gfortran.dg/coarray_45.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/coarray_45.f90 (working copy)
***************
*** 0 ****
--- 1,24 ----
+ ! { dg-do compile }
+ ! { dg-options "-fcoarray=lib -lcaf_single " }
+ !
+ ! Test the fix for PR83076
+ !
+ module m
+ type t
+ integer, pointer :: z
+ end type
+ type(t) :: ptr
+ contains
+ function g(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z) ! This used to ICE with -fcoarray=lib
+ end
+ end module
+
+ use m
+ contains
+ function f(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z)
+ end
+ end
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2018-01-03 0:22 ` Damian Rouson
@ 2018-01-03 9:23 ` Paul Richard Thomas
0 siblings, 0 replies; 18+ messages in thread
From: Paul Richard Thomas @ 2018-01-03 9:23 UTC (permalink / raw)
To: Damian Rouson
Cc: gcc-patches, Dominique d'Humières, gfortran,
Izaak Zaak Beekman, Thomas Koenig, Andre Vehreschild
Many thanks, Damian. I will commit soonish; probably tomorrow.
Paul
On 3 January 2018 at 00:22, Damian Rouson <damian@sourceryinstitute.org> wrote:
> I have now confirmed that the patch works the same for the 7 branch: it
> doesn’t break any previously passing tests.
>
> Damian
>
> On January 1, 2018 at 9:44:59 AM, Paul Richard Thomas
> (paul.richard.thomas@gmail.com) wrote:
>
> Committed to trunk as revision 256065.
>
> Damian, it would be good if you would confirm that there are no issues
> with applying the patch to 7-branch.
>
> Thanks for all the help.
>
> Paul
>
> On 28 December 2017 at 19:58, Damian Rouson
> <damian@sourceryinstitute.org> wrote:
>> I applied the patch the trunk and confirmed that it doesn’t break any
>> previously
>> passing OpenCoarrays tests. Is that sufficient or should I also try
>> applying the
>> patch to the 7 branch?
>>
>> Damian
>>
>>
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2018-01-01 17:45 ` Paul Richard Thomas
@ 2018-01-03 0:22 ` Damian Rouson
2018-01-03 9:23 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Damian Rouson @ 2018-01-03 0:22 UTC (permalink / raw)
To: Paul Richard Thomas
Cc: gcc-patches, Dominique d'Humières, gfortran,
Izaak Zaak Beekman, Thomas Koenig, Andre Vehreschild
I have now confirmed that the patch works the same for the 7 branch: it doesn’t break any previously passing tests.
Damian
On January 1, 2018 at 9:44:59 AM, Paul Richard Thomas (paul.richard.thomas@gmail.com) wrote:
Committed to trunk as revision 256065.
Damian, it would be good if you would confirm that there are no issues
with applying the patch to 7-branch.
Thanks for all the help.
Paul
On 28 December 2017 at 19:58, Damian Rouson
<damian@sourceryinstitute.org> wrote:
> I applied the patch the trunk and confirmed that it doesn’t break any previously
> passing OpenCoarrays tests. Is that sufficient or should I also try applying the
> patch to the 7 branch?
>
> Damian
>
>
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-28 19:58 ` Damian Rouson
@ 2018-01-01 17:45 ` Paul Richard Thomas
2018-01-03 0:22 ` Damian Rouson
0 siblings, 1 reply; 18+ messages in thread
From: Paul Richard Thomas @ 2018-01-01 17:45 UTC (permalink / raw)
To: Damian Rouson
Cc: Andre Vehreschild, gcc-patches, gfortran, Izaak Zaak Beekman,
Thomas Koenig, Dominique d'Humières
Committed to trunk as revision 256065.
Damian, it would be good if you would confirm that there are no issues
with applying the patch to 7-branch.
Thanks for all the help.
Paul
On 28 December 2017 at 19:58, Damian Rouson
<damian@sourceryinstitute.org> wrote:
> I applied the patch the trunk and confirmed that it doesn’t break any previously
> passing OpenCoarrays tests. Is that sufficient or should I also try applying the
> patch to the 7 branch?
>
> Damian
>
>
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-28 14:20 ` Andre Vehreschild
@ 2017-12-28 19:58 ` Damian Rouson
2018-01-01 17:45 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Damian Rouson @ 2017-12-28 19:58 UTC (permalink / raw)
To: Andre Vehreschild, Paul Richard Thomas
Cc: gcc-patches, gfortran, Izaak Zaak Beekman, Thomas Koenig,
Dominique d'Humières
I applied the patch the trunk and confirmed that it doesn’t break any previously
passing OpenCoarrays tests. Is that sufficient or should I also try applying the
patch to the 7 branch?
Damian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-28 11:37 ` Paul Richard Thomas
@ 2017-12-28 14:20 ` Andre Vehreschild
2017-12-28 19:58 ` Damian Rouson
0 siblings, 1 reply; 18+ messages in thread
From: Andre Vehreschild @ 2017-12-28 14:20 UTC (permalink / raw)
To: Paul Richard Thomas
Cc: Damian Rouson, Thomas Koenig, Dominique d'Humières,
gfortran, gcc-patches, Izaak Zaak Beekman
Hi all,
as long as the computation where the token can be found is adapted in the same
way, i.e. the token's offset in the derived type monitors the changed position,
everything is fine. When I remember correctly, then this is done
automatically by the routines setting up the caf_ref-chain for referencing into
coarrays of derived type's (trans-intrinsic.c:~1239 for example). So if
everything works, ok for trunk and gcc-7.
Regards,
Andre
On Thu, 28 Dec 2017 11:37:00 +0000
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> Hi All,
>
> OK - I'll hold back until I hear from Damian & Zaak.
>
> Cheers
>
> Paul
>
> On 27 December 2017 at 21:06, Damian Rouson
> <damian@sourceryinstitute.org> wrote:
> >
> > Thanks for the additional information Thomas. It sounds like I should test
> > Paul’s patch. I should be able to do so today and will post the results by
> > tomorrow. I’m adding OpenCoarrays developer Zaak Beekman to the cc and
> > attaching the patch again in case he wants to try it as well.
> >
> > Zaak, the full thread is at https://gcc.gnu.org/ml/fortran/ and starts with
> > a message from Paul on November 29.
> >
> > Damian
> >
> > On December 27, 2017 at 11:09:29 AM, Thomas Koenig
> > (tkoenig@netcologne.de(mailto:tkoenig@netcologne.de)) wrote:
> >> Hi Damian,
> >>
> >> > Does breaking binary compatibility simply mean that CAF codes will need
> >> > to be recompiled (which is fine)
> >>
> >> Well... not really. We are not supposed to break binary compatibility
> >> in a release. For gcc-8, we have greater freedom because we had to
> >> do it anyway.
> >>
> >> Now, the interesting question is the impact. If we break binary
> >> compatibilty for something that never worked anyway or was useless, or
> >> something that was broken by a gcc-7 regression, I think we're fine.
> >>
> >> If not, well... one possible decision would be to wait for gcc-8 to
> >> fix this.
> >>
> >> > or does it mean that there will need to be work done on OpenCoarrays
> >> to support the changes
> >>
> >> This, I don't know, never having looked at the OpenCoarrays source.
> >>
> >> Regards
> >>
> >> Thomas
>
>
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 21:06 ` Damian Rouson
@ 2017-12-28 11:37 ` Paul Richard Thomas
2017-12-28 14:20 ` Andre Vehreschild
0 siblings, 1 reply; 18+ messages in thread
From: Paul Richard Thomas @ 2017-12-28 11:37 UTC (permalink / raw)
To: Damian Rouson
Cc: Thomas Koenig, Dominique d'Humières, gfortran,
gcc-patches, Izaak Zaak Beekman
Hi All,
OK - I'll hold back until I hear from Damian & Zaak.
Cheers
Paul
On 27 December 2017 at 21:06, Damian Rouson
<damian@sourceryinstitute.org> wrote:
>
> Thanks for the additional information Thomas. It sounds like I should test Paul’s patch. I should be able to do so today and will post the results by tomorrow. I’m adding OpenCoarrays developer Zaak Beekman to the cc and attaching the patch again in case he wants to try it as well.
>
> Zaak, the full thread is at https://gcc.gnu.org/ml/fortran/ and starts with a message from Paul on November 29.
>
> Damian
>
> On December 27, 2017 at 11:09:29 AM, Thomas Koenig (tkoenig@netcologne.de(mailto:tkoenig@netcologne.de)) wrote:
>
>> Hi Damian,
>>
>> > Does breaking binary compatibility simply mean that CAF codes will need to be recompiled (which is fine)
>>
>> Well... not really. We are not supposed to break binary compatibility
>> in a release. For gcc-8, we have greater freedom because we had to
>> do it anyway.
>>
>> Now, the interesting question is the impact. If we break binary
>> compatibilty for something that never worked anyway or was useless, or
>> something that was broken by a gcc-7 regression, I think we're fine.
>>
>> If not, well... one possible decision would be to wait for gcc-8 to
>> fix this.
>>
>> > or does it mean that there will need to be work done on OpenCoarrays
>> to support the changes
>>
>> This, I don't know, never having looked at the OpenCoarrays source.
>>
>> Regards
>>
>> Thomas
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 19:09 ` Thomas Koenig
@ 2017-12-27 21:06 ` Damian Rouson
2017-12-28 11:37 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Damian Rouson @ 2017-12-27 21:06 UTC (permalink / raw)
To: Paul Richard Thomas, Thomas Koenig
Cc: Dominique d'Humières, gfortran, gcc-patches, Izaak Zaak Beekman
[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]
Thanks for the additional information Thomas. It sounds like I should test Paul’s patch. I should be able to do so today and will post the results by tomorrow. I’m adding OpenCoarrays developer Zaak Beekman to the cc and attaching the patch again in case he wants to try it as well.
Zaak, the full thread is at https://gcc.gnu.org/ml/fortran/ and starts with a message from Paul on November 29.
Damian
On December 27, 2017 at 11:09:29 AM, Thomas Koenig (tkoenig@netcologne.de(mailto:tkoenig@netcologne.de)) wrote:
> Hi Damian,
>
> > Does breaking binary compatibility simply mean that CAF codes will need to be recompiled (which is fine)
>
> Well... not really. We are not supposed to break binary compatibility
> in a release. For gcc-8, we have greater freedom because we had to
> do it anyway.
>
> Now, the interesting question is the impact. If we break binary
> compatibilty for something that never worked anyway or was useless, or
> something that was broken by a gcc-7 regression, I think we're fine.
>
> If not, well... one possible decision would be to wait for gcc-8 to
> fix this.
>
> > or does it mean that there will need to be work done on OpenCoarrays
> to support the changes
>
> This, I don't know, never having looked at the OpenCoarrays source.
>
> Regards
>
> Thomas
[-- Attachment #2: submit.diff --]
[-- Type: application/octet-stream, Size: 3132 bytes --]
Index: gcc/fortran/trans-types.c
===================================================================
*** gcc/fortran/trans-types.c (revision 255161)
--- gcc/fortran/trans-types.c (working copy)
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2483,2488 ****
--- 2483,2492 ----
gfc_dt_list *dt;
gfc_namespace *ns;
tree tmp;
+ bool coarray_flag;
+
+ coarray_flag = flag_coarray == GFC_FCOARRAY_LIB
+ && derived->module && !derived->attr.vtype;
gcc_assert (!derived->attr.pdt_template);
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2523,2534 ****
return derived->backend_decl;
}
! /* If use associated, use the module type for this one. */
if (derived->backend_decl == NULL
&& derived->attr.use_assoc
&& derived->module
&& gfc_get_module_backend_decl (derived))
! goto copy_derived_types;
/* The derived types from an earlier namespace can be used as the
canonical type. */
--- 2527,2545 ----
return derived->backend_decl;
}
! /* If use associated, use the module type for this one, except for the case
! where codimensions are present or where a caf_token is needed for pointer
! or allocatable components. */
if (derived->backend_decl == NULL
&& derived->attr.use_assoc
&& derived->module
&& gfc_get_module_backend_decl (derived))
! {
! if (coarray_flag || codimen)
! got_canonical = true;
! else
! goto copy_derived_types;
! }
/* The derived types from an earlier namespace can be used as the
canonical type. */
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2764,2770 ****
GFC_DECL_PTR_ARRAY_P (c->backend_decl) = 1;
/* Do not add a caf_token field for classes' data components. */
! if (codimen && !c->attr.dimension && !c->attr.codimension
&& (c->attr.allocatable || c->attr.pointer)
&& c->caf_token == NULL_TREE && strcmp ("_data", c->name) != 0)
{
--- 2775,2782 ----
GFC_DECL_PTR_ARRAY_P (c->backend_decl) = 1;
/* Do not add a caf_token field for classes' data components. */
! if ((codimen || coarray_flag)
! && !c->attr.dimension && !c->attr.codimension
&& (c->attr.allocatable || c->attr.pointer)
&& c->caf_token == NULL_TREE && strcmp ("_data", c->name) != 0)
{
Index: gcc/testsuite/gfortran.dg/coarray_45.f90
===================================================================
*** gcc/testsuite/gfortran.dg/coarray_45.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/coarray_45.f90 (working copy)
***************
*** 0 ****
--- 1,24 ----
+ ! { dg-do compile }
+ ! { dg-options "-fcoarray=lib -lcaf_single " }
+ !
+ ! Test the fix for PR83076
+ !
+ module m
+ type t
+ integer, pointer :: z
+ end type
+ type(t) :: ptr
+ contains
+ function g(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z) ! This used to ICE with -fcoarray=lib
+ end
+ end module
+
+ use m
+ contains
+ function f(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z)
+ end
+ end
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 18:58 ` Damian Rouson
@ 2017-12-27 19:09 ` Thomas Koenig
2017-12-27 21:06 ` Damian Rouson
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Koenig @ 2017-12-27 19:09 UTC (permalink / raw)
To: Damian Rouson, Paul Richard Thomas
Cc: Dominique d'Humières, gfortran, gcc-patches
Hi Damian,
> Does breaking binary compatibility simply mean that CAF codes will need to be recompiled (which is fine)
Well... not really. We are not supposed to break binary compatibility
in a release. For gcc-8, we have greater freedom because we had to
do it anyway.
Now, the interesting question is the impact. If we break binary
compatibilty for something that never worked anyway or was useless, or
something that was broken by a gcc-7 regression, I think we're fine.
If not, well... one possible decision would be to wait for gcc-8 to
fix this.
> or does it mean that there will need to be work done on OpenCoarrays
to support the changes
This, I don't know, never having looked at the OpenCoarrays source.
Regards
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 18:04 ` Thomas Koenig
2017-12-27 18:55 ` Dominique d'Humières
@ 2017-12-27 18:58 ` Damian Rouson
2017-12-27 19:09 ` Thomas Koenig
1 sibling, 1 reply; 18+ messages in thread
From: Damian Rouson @ 2017-12-27 18:58 UTC (permalink / raw)
To: Paul Richard Thomas, Thomas Koenig
Cc: Dominique d'Humières, gfortran, gcc-patches
Hi Paul,
Does breaking binary compatibility simply mean that CAF codes will need to be recompiled (which is fine) or does it mean that there will need to be work done on OpenCoarrays to support the changes in this patch (which is unlikely to happen soon without new contributors to OpenCoarrays)? If it’s the latter, could you make the appropriate changes to OpenCoarrays before committing this? We’re still dealing with some regressions that Andre introduced a year ago through changes in gfortran that have never been reflected in OpenCoarrays. In fact, just within the past week, the number of such regressions was finally reduced from 3 to 2 for the first time in many months. I’m hopeful that we can continue vectoring toward 0 rather than increasing the number. It’s already the case that several projects won’t even beginning evaluating CAF for production use until our 2 current regressions get fixed and there’s one project that is still stuck using GCC 6 because of the current regressions. We actually had to publish results using GCC 6 for that reason (see https://github.com/sourceryinstitute/coarray-icar-paw17/blob/master/main.pdf).
Damian
On December 27, 2017 at 10:04:13 AM, Thomas Koenig (tkoenig@netcologne.de(mailto:tkoenig@netcologne.de)) wrote:
> Hi Paul,
>
> by the way, the patch is OK for trunk. It is just gcc-7 that I am
> worried about.
>
> Regards
>
> Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 18:04 ` Thomas Koenig
@ 2017-12-27 18:55 ` Dominique d'Humières
2017-12-27 18:58 ` Damian Rouson
1 sibling, 0 replies; 18+ messages in thread
From: Dominique d'Humières @ 2017-12-27 18:55 UTC (permalink / raw)
To: Thomas Koenig
Cc: Paul Richard Thomas, Dominique d'Humières, gfortran,
gcc-patches, Damian Rouson
It also fixes pr78983 and partially pr80235.
Thanks
Dominique
> Le 27 déc. 2017 à 19:04, Thomas Koenig <tkoenig@netcologne.de> a écrit :
>
> Hi Paul,
>
> by the way, the patch is OK for trunk. It is just gcc-7 that I am
> worried about.
>
> Regards
>
> Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 17:57 ` Paul Richard Thomas
@ 2017-12-27 18:04 ` Thomas Koenig
2017-12-27 18:55 ` Dominique d'Humières
2017-12-27 18:58 ` Damian Rouson
0 siblings, 2 replies; 18+ messages in thread
From: Thomas Koenig @ 2017-12-27 18:04 UTC (permalink / raw)
To: Paul Richard Thomas
Cc: Dominique d'Humières, gfortran, gcc-patches, Damian Rouson
Hi Paul,
by the way, the patch is OK for trunk. It is just gcc-7 that I am
worried about.
Regards
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 17:50 ` Thomas Koenig
@ 2017-12-27 17:57 ` Paul Richard Thomas
2017-12-27 18:04 ` Thomas Koenig
0 siblings, 1 reply; 18+ messages in thread
From: Paul Richard Thomas @ 2017-12-27 17:57 UTC (permalink / raw)
To: Thomas Koenig
Cc: Dominique d'Humières, gfortran, gcc-patches, Damian Rouson
Hi Thomas,
That's a good question. I have kept Damian in copy. It must be said
that the OpenCoarray folk are more concerned with PR83319, where there
is no problem of binary compatibility. I am awaiting some input from
him.
Cheers
Paul
On 27 December 2017 at 17:50, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hi Paul,
>
>> It will break binary compatibility for caf with scalar,
>> allocatable/pointer components. This comes about because I decided
>> that the caf tokens for thes components, should come after all other
>> components, rather than immediately after the "owner" component. This
>> has the advantage, that they are then binary compatible with external
>> functions or modules that are not compiled with -fcoarray = lib.
>
>
> So, is this something that we can do for gcc-7 (where we shold maintain
> binary compatibility)?
>
> Regards
>
> Thomas
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 16:58 ` Paul Richard Thomas
@ 2017-12-27 17:50 ` Thomas Koenig
2017-12-27 17:57 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Koenig @ 2017-12-27 17:50 UTC (permalink / raw)
To: Paul Richard Thomas
Cc: Dominique d'Humières, gfortran, gcc-patches, Damian Rouson
Hi Paul,
> It will break binary compatibility for caf with scalar,
> allocatable/pointer components. This comes about because I decided
> that the caf tokens for thes components, should come after all other
> components, rather than immediately after the "owner" component. This
> has the advantage, that they are then binary compatible with external
> functions or modules that are not compiled with -fcoarray = lib.
So, is this something that we can do for gcc-7 (where we shold maintain
binary compatibility)?
Regards
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-27 16:09 ` Thomas Koenig
@ 2017-12-27 16:58 ` Paul Richard Thomas
2017-12-27 17:50 ` Thomas Koenig
0 siblings, 1 reply; 18+ messages in thread
From: Paul Richard Thomas @ 2017-12-27 16:58 UTC (permalink / raw)
To: Thomas Koenig
Cc: Dominique d'Humières, gfortran, gcc-patches, Damian Rouson
Hi Thomas,
It will break binary compatibility for caf with scalar,
allocatable/pointer components. This comes about because I decided
that the caf tokens for thes components, should come after all other
components, rather than immediately after the "owner" component. This
has the advantage, that they are then binary compatible with external
functions or modules that are not compiled with -fcoarray = lib.
Arrays will continue to be binary compatible.
Thanks
Paul
On 27 December 2017 at 16:09, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hi Paul,
>
>> This is a complete rework of the patch and of the original mechanism
>> for adding caf token fields and finding them.
>>
>> In this patch, the token fields are added to the derived types after
>> all the components have been resolved. This is done so that all the
>> tokens appear at the very end of the derived type, including the
>> hidden string lengths. This avoids the present situation, where the
>> token appears immediately after its associated component such that the
>> the derived types are not compatible with modules or libraries
>> compiled without -fcoarray selected. All trans-types has to do now is
>> to find the component and have the component token field point to its
>> backend_decl. PR83319 is fixed by unconditionally adding the token
>> field to the descriptor, when -fcoarray=lib whatever the value of
>> codimen.
>
>
> This looks very good.
>
> I just have one question: Will this break binary compatibility for
> the 7-branch? Or will this only break compatibility for something
> that never worked anyway?
>
> Regards
>
> Thomas
>
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-26 16:17 ` Paul Richard Thomas
@ 2017-12-27 16:09 ` Thomas Koenig
2017-12-27 16:58 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Koenig @ 2017-12-27 16:09 UTC (permalink / raw)
To: Paul Richard Thomas, Dominique d'Humières; +Cc: gfortran, gcc-patches
Hi Paul,
> This is a complete rework of the patch and of the original mechanism
> for adding caf token fields and finding them.
>
> In this patch, the token fields are added to the derived types after
> all the components have been resolved. This is done so that all the
> tokens appear at the very end of the derived type, including the
> hidden string lengths. This avoids the present situation, where the
> token appears immediately after its associated component such that the
> the derived types are not compatible with modules or libraries
> compiled without -fcoarray selected. All trans-types has to do now is
> to find the component and have the component token field point to its
> backend_decl. PR83319 is fixed by unconditionally adding the token
> field to the descriptor, when -fcoarray=lib whatever the value of
> codimen.
This looks very good.
I just have one question: Will this break binary compatibility for
the 7-branch? Or will this only break compatibility for something
that never worked anyway?
Regards
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
2017-12-03 23:48 Dominique d'Humières
@ 2017-12-26 16:17 ` Paul Richard Thomas
2017-12-27 16:09 ` Thomas Koenig
0 siblings, 1 reply; 18+ messages in thread
From: Paul Richard Thomas @ 2017-12-26 16:17 UTC (permalink / raw)
To: Dominique d'Humières; +Cc: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]
Hi All,
This is a complete rework of the patch and of the original mechanism
for adding caf token fields and finding them.
In this patch, the token fields are added to the derived types after
all the components have been resolved. This is done so that all the
tokens appear at the very end of the derived type, including the
hidden string lengths. This avoids the present situation, where the
token appears immediately after its associated component such that the
the derived types are not compatible with modules or libraries
compiled without -fcoarray selected. All trans-types has to do now is
to find the component and have the component token field point to its
backend_decl. PR83319 is fixed by unconditionally adding the token
field to the descriptor, when -fcoarray=lib whatever the value of
codimen.
This is something of a belt-and-braces approach, in that the token
fields will sometimes be added when not needed. However, it is better
that than the ICEs that occur when they are missing.
Bootstrapped and regtested on FC23/x86_64 - OK for trunk and 7-branch?
Paul
2017-12-26 Paul Thomas <pault@gcc.gnu.org>
PR fortran/83076
* resolve.c (resolve_fl_derived0): Add caf_token fields for
allocatable and pointer scalars, when -fcoarray selected.
* trans-types.c (gfc_copy_dt_decls_ifequal): Copy the token
field as well as the backend_decl.
(gfc_get_derived_type): Flag GFC_FCOARRAY_LIB for module
derived types that are not vtypes. Components with caf_token
attribute are pvoid types. For a component requiring it, find
the caf_token field and have the component token field point to
its backend_decl.
PR fortran/83319
*trans-types.c (gfc_get_array_descriptor_base): Add the token
field to the descriptor even when codimen not set.
2017-12-26 Paul Thomas <pault@gcc.gnu.org>
PR fortran/83076
* gfortran.dg/coarray_45.f90 : New test.
PR fortran/83319
* gfortran.dg/coarray_46.f90 : New test.
On 3 December 2017 at 23:48, Dominique d'Humières
<dominiq@tournesol.lps.ens.fr> wrote:
> Dear Paul,
>
>> Bootstrapped and regtested on FC23/x86_64 - OK for trunk?
>
> See my comment 7 in the PR.
>
> Dominique
>
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
[-- Attachment #2: submit.diff --]
[-- Type: text/plain, Size: 7293 bytes --]
Index: gcc/fortran/gfortran.h
===================================================================
*** gcc/fortran/gfortran.h (revision 256000)
--- gcc/fortran/gfortran.h (working copy)
*************** typedef struct
*** 870,876 ****
unsigned alloc_comp:1, pointer_comp:1, proc_pointer_comp:1,
private_comp:1, zero_comp:1, coarray_comp:1, lock_comp:1,
event_comp:1, defined_assign_comp:1, unlimited_polymorphic:1,
! has_dtio_procs:1;
/* This is a temporary selector for SELECT TYPE or an associate
variable for SELECT_TYPE or ASSOCIATE. */
--- 870,876 ----
unsigned alloc_comp:1, pointer_comp:1, proc_pointer_comp:1,
private_comp:1, zero_comp:1, coarray_comp:1, lock_comp:1,
event_comp:1, defined_assign_comp:1, unlimited_polymorphic:1,
! has_dtio_procs:1, caf_token:1;
/* This is a temporary selector for SELECT TYPE or an associate
variable for SELECT_TYPE or ASSOCIATE. */
Index: gcc/fortran/resolve.c
===================================================================
*** gcc/fortran/resolve.c (revision 256000)
--- gcc/fortran/resolve.c (working copy)
*************** resolve_fl_derived0 (gfc_symbol *sym)
*** 13992,13997 ****
--- 13992,14022 ----
if (!success)
return false;
+ /* Now add the caf token field, where needed. */
+ if (flag_coarray != GFC_FCOARRAY_NONE
+ && !sym->attr.is_class && !sym->attr.vtype)
+ {
+ for (c = sym->components; c; c = c->next)
+ if (!c->attr.dimension && !c->attr.codimension
+ && (c->attr.allocatable || c->attr.pointer))
+ {
+ char name[GFC_MAX_SYMBOL_LEN+9];
+ gfc_component *token;
+ sprintf (name, "_caf_%s", c->name);
+ token = gfc_find_component (sym, name, true, true, NULL);
+ if (token == NULL)
+ {
+ if (!gfc_add_component (sym, name, &token))
+ return false;
+ token->ts.type = BT_VOID;
+ token->ts.kind = gfc_default_integer_kind;
+ token->attr.access = ACCESS_PRIVATE;
+ token->attr.artificial = 1;
+ token->attr.caf_token = 1;
+ }
+ }
+ }
+
check_defined_assignments (sym);
if (!sym->attr.defined_assign_comp && super_type)
Index: gcc/fortran/trans-types.c
===================================================================
*** gcc/fortran/trans-types.c (revision 256000)
--- gcc/fortran/trans-types.c (working copy)
*************** gfc_get_array_descriptor_base (int dimen
*** 1837,1843 ****
TREE_NO_WARNING (decl) = 1;
}
! if (flag_coarray == GFC_FCOARRAY_LIB && codimen)
{
decl = gfc_add_field_to_struct_1 (fat_type,
get_identifier ("token"),
--- 1837,1843 ----
TREE_NO_WARNING (decl) = 1;
}
! if (flag_coarray == GFC_FCOARRAY_LIB)
{
decl = gfc_add_field_to_struct_1 (fat_type,
get_identifier ("token"),
*************** gfc_copy_dt_decls_ifequal (gfc_symbol *f
*** 2373,2378 ****
--- 2373,2379 ----
for (; to_cm; to_cm = to_cm->next, from_cm = from_cm->next)
{
to_cm->backend_decl = from_cm->backend_decl;
+ to_cm->caf_token = from_cm->caf_token;
if (from_cm->ts.type == BT_UNION)
gfc_get_union_type (to_cm->ts.u.derived);
else if (from_cm->ts.type == BT_DERIVED
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2483,2488 ****
--- 2484,2493 ----
gfc_dt_list *dt;
gfc_namespace *ns;
tree tmp;
+ bool coarray_flag;
+
+ coarray_flag = flag_coarray == GFC_FCOARRAY_LIB
+ && derived->module && !derived->attr.vtype;
gcc_assert (!derived->attr.pdt_template);
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2677,2683 ****
field_type = build_pointer_type (tmp);
}
else if (c->ts.type == BT_DERIVED || c->ts.type == BT_CLASS)
! field_type = c->ts.u.derived->backend_decl;
else
{
if (c->ts.type == BT_CHARACTER
--- 2682,2690 ----
field_type = build_pointer_type (tmp);
}
else if (c->ts.type == BT_DERIVED || c->ts.type == BT_CLASS)
! field_type = c->ts.u.derived->backend_decl;
! else if (c->attr.caf_token)
! field_type = pvoid_type_node;
else
{
if (c->ts.type == BT_CHARACTER
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2762,2780 ****
&& !(c->ts.type == BT_DERIVED
&& strcmp (c->name, "_data") == 0))
GFC_DECL_PTR_ARRAY_P (c->backend_decl) = 1;
-
- /* Do not add a caf_token field for classes' data components. */
- if (codimen && !c->attr.dimension && !c->attr.codimension
- && (c->attr.allocatable || c->attr.pointer)
- && c->caf_token == NULL_TREE && strcmp ("_data", c->name) != 0)
- {
- char caf_name[GFC_MAX_SYMBOL_LEN];
- snprintf (caf_name, GFC_MAX_SYMBOL_LEN, "_caf_%s", c->name);
- c->caf_token = gfc_add_field_to_struct (typenode,
- get_identifier (caf_name),
- pvoid_type_node, &chain);
- TREE_NO_WARNING (c->caf_token) = 1;
- }
}
/* Now lay out the derived type, including the fields. */
--- 2769,2774 ----
*************** gfc_get_derived_type (gfc_symbol * deriv
*** 2800,2805 ****
--- 2794,2817 ----
copy_derived_types:
+ for (c = derived->components; c; c = c->next)
+ {
+ /* Do not add a caf_token field for class container components. */
+ if ((codimen || coarray_flag)
+ && !c->attr.dimension && !c->attr.codimension
+ && (c->attr.allocatable || c->attr.pointer)
+ && !derived->attr.is_class)
+ {
+ char caf_name[GFC_MAX_SYMBOL_LEN];
+ gfc_component *token;
+ snprintf (caf_name, GFC_MAX_SYMBOL_LEN, "_caf_%s", c->name);
+ token = gfc_find_component (derived, caf_name, true, true, NULL);
+ gcc_assert (token);
+ c->caf_token = token->backend_decl;
+ TREE_NO_WARNING (c->caf_token) = 1;
+ }
+ }
+
for (dt = gfc_derived_types; dt; dt = dt->next)
gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
Index: gcc/testsuite/gfortran.dg/coarray_45.f90
===================================================================
*** gcc/testsuite/gfortran.dg/coarray_45.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/coarray_45.f90 (working copy)
***************
*** 0 ****
--- 1,24 ----
+ ! { dg-do compile }
+ ! { dg-options "-fcoarray=lib -lcaf_single " }
+ !
+ ! Test the fix for PR83076
+ !
+ module m
+ type t
+ integer, pointer :: z
+ end type
+ type(t) :: ptr
+ contains
+ function g(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z) ! This used to ICE with -fcoarray=lib
+ end
+ end module
+
+ use m
+ contains
+ function f(x)
+ type(t) :: x[*]
+ if (associated (x%z, ptr%z)) deallocate (x%z)
+ end
+ end
Index: gcc/testsuite/gfortran.dg/coarray_46.f90
===================================================================
*** gcc/testsuite/gfortran.dg/coarray_46.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/coarray_46.f90 (working copy)
***************
*** 0 ****
--- 1,17 ----
+ ! { dg-do compile }
+ ! { dg-options "-fcoarray=lib -lcaf_single" }
+ !
+ ! Test the fix for PR83319
+ !
+ module foo_module
+ implicit none
+ type foo
+ integer, allocatable :: i(:)
+ end type
+ end module
+
+ use foo_module
+ implicit none
+ type(foo), save :: bar[*]
+ allocate(bar%i(1)) ! Used to ICE here.
+ end
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598
@ 2017-12-03 23:48 Dominique d'Humières
2017-12-26 16:17 ` Paul Richard Thomas
0 siblings, 1 reply; 18+ messages in thread
From: Dominique d'Humières @ 2017-12-03 23:48 UTC (permalink / raw)
To: Paul Richard Thomas; +Cc: gfortran, gcc-patches
Dear Paul,
> Bootstrapped and regtested on FC23/x86_64 - OK for trunk?
See my comment 7 in the PR.
Dominique
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-01-03 9:23 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 10:03 [Patch, fortran] PR83076 - [8 Regression] ICE in gfc_deallocate_scalar_with_status, at fortran/trans.c:1598 Paul Richard Thomas
2017-12-03 23:48 Dominique d'Humières
2017-12-26 16:17 ` Paul Richard Thomas
2017-12-27 16:09 ` Thomas Koenig
2017-12-27 16:58 ` Paul Richard Thomas
2017-12-27 17:50 ` Thomas Koenig
2017-12-27 17:57 ` Paul Richard Thomas
2017-12-27 18:04 ` Thomas Koenig
2017-12-27 18:55 ` Dominique d'Humières
2017-12-27 18:58 ` Damian Rouson
2017-12-27 19:09 ` Thomas Koenig
2017-12-27 21:06 ` Damian Rouson
2017-12-28 11:37 ` Paul Richard Thomas
2017-12-28 14:20 ` Andre Vehreschild
2017-12-28 19:58 ` Damian Rouson
2018-01-01 17:45 ` Paul Richard Thomas
2018-01-03 0:22 ` Damian Rouson
2018-01-03 9:23 ` Paul Richard Thomas
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).