Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
On 21.06.21 22:29, Tobias Burnus wrote:
However, that's independent from the patch you had submitted
and which is fine except for the two tiny nits.
As I just did run into a test, which does trigger the error, I think
it would be useful to have something like the following on top
of your patch – what do you think?
(Two of the changes are the nit changes I mentioned in the
LGTM approval.)
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 52e243bd463..73ce33185f1 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5616,3 +5616,3 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
/* Transfer values back to gfc descriptor. */
- if (cfi_attribute != 2
+ if (cfi_attribute != 2 /* CFI_attribute_other. */
&& !fsym->attr.value
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 801b7556765..1b845df0e77 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -56,3 +56,4 @@ cfi_desc_to_gfc_desc (gfc_array_void *d, CFI_cdesc_t **s_ptr)
default:
- internal_error (NULL, "INVALID CFI DESCRIPTOR");
+ runtime_error ("Unallocated, unassociated actual argument to "
+ "BIND(C) with non-allocatable, non-pointer dummy");
break;
@@ -94,3 +95,3 @@ cfi_desc_to_gfc_desc (gfc_array_void *d, CFI_cdesc_t **s_ptr)
CFI_index_t lb = 1;
-
+
if (s->attribute != CFI_attribute_other)
@@ -134,3 +135,4 @@ gfc_desc_to_cfi_desc (CFI_cdesc_t **d_ptr, const gfc_array_void *s)
default:
- internal_error (NULL, "INVALID GFC DESCRIPTOR");
+ runtime_error ("Unallocated, unassociated actual argument to "
+ "BIND(C) with non-allocatable, non-pointer dummy");
break;
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi Tobias, thanks for the review. To the questions: - I added a test only for -fcoarray=single because in the library case the optional stat is just propagated to the library, which is already tested a lot of times and which needs to handle the optional stat in any case. So an error there would have been detected in one of the earlier tests. I did not want to add unnecessary test overhead given that the tests already run for a long time. - I did not add tests for the other CO_* routines, i.e. CO_MIN, CO_MAX, CO_REDUCE or CO_SUM, that are also handled by this routine, because I believe that showing that the fix works for CO_BROADCAST shows that the others work, too. Because the four others do not have any special handling in their implementation in trans_intrinsic. Or do you mean other coarray-routines besides the five handled by conv_co_collective()? If it is ok for you, I would apply the patch as is, or do you see a reason to add more tests? Regards, Andre On Mon, 21 Jun 2021 14:30:21 +0200 Tobias Burnus wrote: > Any reason that you did not put it under >gfortran.dg/coarray/ > such that it is also run with -fcoarray=lib (-lcaf_single)? > I know that the issue only exists for single, but it also makes > sense to check that libcaf_single works > > In that sense, I wonder whether also the other CO_* should be > checked in the testsuite as they are handled differently in > libcaf_... (but identical with -fcoarray=single). > > Except for those two nits, it LGTM. Thanks! > > Tobias > > PS: The function is used by > case GFC_ISYM_CO_BROADCAST: > case GFC_ISYM_CO_MIN: > case GFC_ISYM_CO_MAX: > case GFC_ISYM_CO_REDUCE: > case GFC_ISYM_CO_SUM: > and, with -fcoarray=single, errmsg is not touched > as stat is (unconditionally) 0 (success).. > > > On 19.06.21 13:23, Andre Vehreschild via Fortran wrote: > > PING! > > > > On Fri, 4 Jun 2021 18:05:18 +0200 > > Andre Vehreschild wrote: > > > >> Ping! > >> > >> On Fri, 21 May 2021 15:33:11 +0200 > >> Andre Vehreschild wrote: > >> > >>> Hi, > >>> > >>> the attached patch fixes an issue when calling CO_BROADCAST in > >>> -fcoarray=single mode, where the optional but non-present (in the calling > >>> scope) stat variable was assigned to before checking for it being not > >>> present. > >>> > >>> Regtests fine on x86-64-linux/f33. Ok for trunk? > >>> > >>> Regards, > >>> Andre > >> > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > - > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi Andre, On 22.06.21 09:40, Andre Vehreschild via Fortran wrote: To the questions: - I added a test only for -fcoarray=single because in the library case the optional stat is just propagated to the library, which is already tested a lot of times and which needs to handle the optional stat in any case. So an error there would have been detected in one of the earlier tests. I did not want to add unnecessary test overhead given that the tests already run for a long time. Fair point. - I did not add tests for the other CO_* routines, i.e. CO_MIN, CO_MAX, CO_REDUCE or CO_SUM, that are also handled by this routine, because I believe that showing that the fix works for CO_BROADCAST shows that the others work, too. Because the four others do not have any special handling in their implementation in trans_intrinsic. Or do you mean other coarray-routines besides the five handled by conv_co_collective()? Well, that relates more to the first point – for -fcoarray=lib, it likely makes a difference. For -fcoarray=single not. If the former is skipped, it is much less relevant for the second. If it is ok for you, I would apply the patch as is, or do you see a reason to add more tests? OK. Although, I am not that sure that libcaf_single gets that much testing. On the other hand, -fcoarray=lib with -lcaf_single is also not that relevant in the real world, either. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
