diff mbox series

[Fortran,96418] Fix Test coarray_alloc_comp_4.f08 ICEs

Message ID 20240611161238.14634f57@vepi2
State New
Headers show
Series [Fortran,96418] Fix Test coarray_alloc_comp_4.f08 ICEs | expand

Commit Message

Andre Vehreschild June 11, 2024, 2:12 p.m. UTC 
Hi all,

attached patch has already been present in 2020, but lost my attention. It
fixes an ICE in the testsuite. The old mails description is:

attached patch fixes PR96418 where the code in the testsuite when compiled with
-fcoarray=single  lead to an ICE. The reason was that the coarray object was
derefed as an array, but it was no array. Introducing the test for the
descriptor removes the ICE.

Regtests ok on x86_64-linux/Fedora 39. Ok for mainline?

Regards,
	Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de

Comments

Andre Vehreschild June 14, 2024, 7:22 a.m. UTC | #1 
Hi all,

I messed up renaming of the coarray_alloc_comp-test. This is fixed in the second
version of the patch. Sorry for the inconvenience.

Additionally I figured that this patch also fixed PR fortran/103112.

Regtests ok on x86_64 Fedora 39. Ok for mainline?

Regards,
	Andre

On Tue, 11 Jun 2024 16:12:38 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
>
> attached patch has already been present in 2020, but lost my attention. It
> fixes an ICE in the testsuite. The old mails description is:
>
> attached patch fixes PR96418 where the code in the testsuite when compiled
> with -fcoarray=single  lead to an ICE. The reason was that the coarray object
> was derefed as an array, but it was no array. Introducing the test for the
> descriptor removes the ICE.
>
> Regtests ok on x86_64-linux/Fedora 39. Ok for mainline?
>
> Regards,
> 	Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de
Harald Anlauf June 14, 2024, 7:43 p.m. UTC | #2 
Hi Andre,

the patch looks fairly simple and obvious, so OK from my side.

***

Regarding the testsuite: since you renamed one of the testcases
gfortran.dg/coarray_alloc_comp_* and moved it to gfortran.dg/coarray/,
I checked and noticed that there are other similar runtime tests for
coarrays (while some are compile-time only tests).

Do we plan to "clean" this up and move more/all related runtime
tests to the coarray/ subdirectory?  What is the general opinion on
this?

***

Thanks for the patch!

Harald


Am 14.06.24 um 09:22 schrieb Andre Vehreschild:
> Hi all,
>
> I messed up renaming of the coarray_alloc_comp-test. This is fixed in the second
> version of the patch. Sorry for the inconvenience.
>
> Additionally I figured that this patch also fixed PR fortran/103112.
>
> Regtests ok on x86_64 Fedora 39. Ok for mainline?
>
> Regards,
> 	Andre
>
> On Tue, 11 Jun 2024 16:12:38 +0200
> Andre Vehreschild <vehre@gmx.de> wrote:
>
>> Hi all,
>>
>> attached patch has already been present in 2020, but lost my attention. It
>> fixes an ICE in the testsuite. The old mails description is:
>>
>> attached patch fixes PR96418 where the code in the testsuite when compiled
>> with -fcoarray=single  lead to an ICE. The reason was that the coarray object
>> was derefed as an array, but it was no array. Introducing the test for the
>> descriptor removes the ICE.
>>
>> Regtests ok on x86_64-linux/Fedora 39. Ok for mainline?
>>
>> Regards,
>> 	Andre
>> --
>> Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
Andre Vehreschild June 17, 2024, 7:51 a.m. UTC | #3 
Hi Harald,

thank you very much for the review. Committed as:

gcc-15-1369-gdb75a6657e9

Regarding your question on the coarray-tests that are not in the
coarray-directory: These test in most cases test only one method of
implementing coarrays. I.e., they are either testing just -fcoarray=single or
-fcoarray=lib -lcaf_single, which are two different approaches. The tests in
the coarray-directory test all available methods to implement coarrays. Pushing
all coarray-tests into the coarray-directory will fail a lot of them, because
the behavior of -fcoarray=single and -fcoarray=lib -lcaf_single is different in
some corner cases. That's why the coarray-tests in the main gfortran-dir are
separate.

I do understand why it may be confusing, but I don't see an easy solution. Does
this answer your question?

Thanks again for the review.

Regards,
	Andre

On Fri, 14 Jun 2024 21:43:47 +0200
Harald Anlauf <anlauf@gmx.de> wrote:

> Hi Andre,
>
> the patch looks fairly simple and obvious, so OK from my side.
>
> ***
>
> Regarding the testsuite: since you renamed one of the testcases
> gfortran.dg/coarray_alloc_comp_* and moved it to gfortran.dg/coarray/,
> I checked and noticed that there are other similar runtime tests for
> coarrays (while some are compile-time only tests).
>
> Do we plan to "clean" this up and move more/all related runtime
> tests to the coarray/ subdirectory?  What is the general opinion on
> this?
>
> ***
>
> Thanks for the patch!
>
> Harald
>
>
> Am 14.06.24 um 09:22 schrieb Andre Vehreschild:
> > Hi all,
> >
> > I messed up renaming of the coarray_alloc_comp-test. This is fixed in the
> > second version of the patch. Sorry for the inconvenience.
> >
> > Additionally I figured that this patch also fixed PR fortran/103112.
> >
> > Regtests ok on x86_64 Fedora 39. Ok for mainline?
> >
> > Regards,
> > 	Andre
> >
> > On Tue, 11 Jun 2024 16:12:38 +0200
> > Andre Vehreschild <vehre@gmx.de> wrote:
> >
> >> Hi all,
> >>
> >> attached patch has already been present in 2020, but lost my attention. It
> >> fixes an ICE in the testsuite. The old mails description is:
> >>
> >> attached patch fixes PR96418 where the code in the testsuite when compiled
> >> with -fcoarray=single  lead to an ICE. The reason was that the coarray
> >> object was derefed as an array, but it was no array. Introducing the test
> >> for the descriptor removes the ICE.
> >>
> >> Regtests ok on x86_64-linux/Fedora 39. Ok for mainline?
> >>
> >> Regards,
> >> 	Andre
> >> --
> >> Andre Vehreschild * Email: vehre ad gmx dot de
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>


--
Andre Vehreschild * Email: vehre ad gmx dot de
Harald Anlauf June 17, 2024, 7:45 p.m. UTC | #4 
Hi Andre,

Am 17.06.24 um 09:51 schrieb Andre Vehreschild:
> Regarding your question on the coarray-tests that are not in the
> coarray-directory: These test in most cases test only one method of
> implementing coarrays. I.e., they are either testing just -fcoarray=single or
> -fcoarray=lib -lcaf_single, which are two different approaches. The tests in
> the coarray-directory test all available methods to implement coarrays.  Pushing

ah, that explains it.  I only looked at some of the test sources,
but did not think of looking at caf.exp ...

> all coarray-tests into the coarray-directory will fail a lot of them, because
> the behavior of -fcoarray=single and -fcoarray=lib -lcaf_single is different in
> some corner cases. That's why the coarray-tests in the main gfortran-dir are
> separate.
>
> I do understand why it may be confusing, but I don't see an easy solution. Does
> this answer your question?

Indeed it does!

Thanks,
Harald
diff mbox series

Patch

From e56f32ed836c1ecc2b46497d1d7b9c7c08749521 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild <vehre@gcc.gnu.org>
Date: Tue, 11 Jun 2024 15:24:55 +0200
Subject: [PATCH] Fix ICE when compiling with -fcoarray=single, when derefing a
 non-array.

            PR fortran/96418

gcc/fortran/ChangeLog:

            * trans.c (gfc_deallocate_with_status): Check that object to deref
            is an array, before applying array deref.

gcc/testsuite/ChangeLog:

            * gfortran.dg/coarray_alloc_comp_3.f08: Moved to...
            * gfortran.dg/coarray/alloc_comp_6.f90: ...here.
            Should be tested for both -fcoarray=single and lib, resp.
            * gfortran.dg/coarray_alloc_comp_4.f08: Fix program name.
---
 gcc/fortran/trans.cc                                           | 3 ++-
 .../{coarray_alloc_comp_3.f08 => coarray/alloc_comp_6.f08}     | 3 +--
 gcc/testsuite/gfortran.dg/coarray_alloc_comp_4.f08             | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
 rename gcc/testsuite/gfortran.dg/{coarray_alloc_comp_3.f08 => coarray/alloc_comp_6.f08} (95%)

diff --git a/gcc/fortran/trans.cc b/gcc/fortran/trans.cc
index a208afe90ab..1335b8cc48b 100644
--- a/gcc/fortran/trans.cc
+++ b/gcc/fortran/trans.cc
@@ -1838,7 +1838,8 @@  gfc_deallocate_with_status (tree pointer, tree status, tree errmsg,
 	  else
 	    caf_dereg_type = (enum gfc_coarray_deregtype) coarray_dealloc_mode;
 	}
-      else if (flag_coarray == GFC_FCOARRAY_SINGLE)
+      else if (flag_coarray == GFC_FCOARRAY_SINGLE
+	       && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (pointer)))
 	pointer = gfc_conv_descriptor_data_get (pointer);
     }
   else if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (pointer)))
diff --git a/gcc/testsuite/gfortran.dg/coarray_alloc_comp_3.f08 b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f08
similarity index 95%
rename from gcc/testsuite/gfortran.dg/coarray_alloc_comp_3.f08
rename to gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f08
index e2037aa5809..8b153925129 100644
--- a/gcc/testsuite/gfortran.dg/coarray_alloc_comp_3.f08
+++ b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f08
@@ -1,12 +1,11 @@ 
 ! { dg-do run }
-! { dg-options "-fcoarray=lib -lcaf_single" }
 ! { dg-additional-options "-latomic" { target libatomic_available } }
 !
 ! Contributed by Andre Vehreschild
 ! Check that manually freeing components does not lead to a runtime crash,
 ! when the auto-deallocation is taking care.

-program coarray_alloc_comp_3
+program alloc_comp_6
   implicit none

   type dt
diff --git a/gcc/testsuite/gfortran.dg/coarray_alloc_comp_4.f08 b/gcc/testsuite/gfortran.dg/coarray_alloc_comp_4.f08
index 6586ec651dd..4c71a90af8f 100644
--- a/gcc/testsuite/gfortran.dg/coarray_alloc_comp_4.f08
+++ b/gcc/testsuite/gfortran.dg/coarray_alloc_comp_4.f08
@@ -5,7 +5,7 @@ 
 ! Contributed by Andre Vehreschild
 ! Check that sub-components are caf_deregistered and not freed.

-program coarray_alloc_comp_3
+program coarray_alloc_comp_4
   implicit none

   type dt
--
2.45.1