Re: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469
Hi Harald, I spend yesterday about two hours with this. Now I am still tired but understand more. I think the confusion between the two of us is due to wording and in which directions the thoughts then go: Talking about coindexed, all of a[i], b[i]%c and c%d[i] are coindexed and there are many constraints like "shall not be a coindexed variable" – which then rejects all of those. That's what I was thinking of. I think your starting point is that while ('a' = allocatable) a, b%a, c[5]%d(1)%a are ALLOCATABLE, adding a subobject reference such as a(:), b%a(:,:), c[5]%d(1)%a(:,:,:) makes the variable no longer allocatable. I think that's what you were thinking of. We then both argued along those different lines – which caused the confusion as we both thought we talked about the same. While those cases are clear, the question is whether a[i] or b%a[i] is allocatable or not – assuming that 'a' is a scalar. (For an array, '(:)' has to appear before the image-selector, which in turn makes it nonallocatable.) I tried to pinpoint the words for this in the standard – and failed. I think I need a "how to read the Fortran standard" 101 and some long time actually reading it :-( Malcolm has answered me – and he believes (but only offhand) that a[i] and b%a[i] _are_ allocatable. See (6) at https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html This implies that if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1 is valid. However, I do note that coarray allocatables have to be collectively (de)allocated, therefore allocated (a[i]) .and. allocated (b%a[i]) is equivalent to allocated (a) .and. allocated (b%a) at least assuming that no image has failed. First: Does this answer all the questions you had and resolved the confusion? Secondly, do you agree about the last bits of the analysis? Thirdly, what do you think of the attached patch? 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 Fortran: Handle allocated() with coindexed scalars [PR93834] 2021-09-07 Harald Anlauf Tobias Burnus While for an allocatable 'array', 'array(:)' and 'array(:)[1]' are not allocatable, it is believed that not only 'scalar' but also 'scalar[1]' is allocatable. However, coarrays are collectively established/allocated; thus, 'allocated(scalar[i])' is equivalent to 'allocated(scalar)'. [At least when assuming that 'i' does not refer to a failed image.] PR fortran/93834 gcc/fortran/ChangeLog: * trans-intrinsic.c (gfc_conv_allocated): Cleanup. Handle coindexed scalar coarrays. gcc/testsuite/ChangeLog: * gfortran.dg/coarray_allocated.f90: New test. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 46670baae55..6a7a86d245a 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -8887,50 +8887,64 @@ caf_this_image_ref (gfc_ref *ref) static void gfc_conv_allocated (gfc_se *se, gfc_expr *expr) { - gfc_actual_arglist *arg1; gfc_se arg1se; tree tmp; - symbol_attribute caf_attr; + bool coindexed_caf_comp = false; - gfc_init_se (&arg1se, NULL); - arg1 = expr->value.function.actual; + expr = expr->value.function.actual->expr; - if (arg1->expr->ts.type == BT_CLASS) + gfc_init_se (&arg1se, NULL); + if (expr->ts.type == BT_CLASS) { /* Make sure that class array expressions have both a _data component reference and an array reference */ - if (CLASS_DATA (arg1->expr)->attr.dimension) - gfc_add_class_array_ref (arg1->expr); + if (CLASS_DATA (expr)->attr.dimension) + gfc_add_class_array_ref (expr); /* whilst scalars only need the _data component. */ else - gfc_add_data_component (arg1->expr); + gfc_add_data_component (expr); } - /* When arg1 references an allocatable component in a coarray, then call + /* When expr references an allocatable component in a coarray, then call the caf-library function caf_is_present (). */ - if (flag_coarray == GFC_FCOARRAY_LIB && arg1->expr->expr_type == EXPR_FUNCTION - && arg1->expr->value.function.isym - && arg1->expr->value.function.isym->id == GFC_ISYM_CAF_GET) -caf_attr = gfc_caf_attr (arg1->expr->value.function.actual->expr); - else -gfc_clear_attr (&caf_attr); - if (flag_coarray == GFC_FCOARRAY_LIB && caf_attr.codimension - && !caf_this_image_ref (arg1->expr->value.function.actual->expr->ref)) -tmp = trans_caf_is_present (se, arg1->expr->value.function.actual->expr); + if (flag_coarray == GFC_FCOARRAY_LIB && expr->expr_type == EXPR_FUNCTION + && expr->value.function.isym + && expr->value.function.isym->id == GFC_ISYM_CAF_GET) +{ + expr = expr->value.function.actual->expr; + if (caf_this_image_ref (expr->ref)) + coindexed_caf_comp = false;
Re: [Patch] C, C++, Fortran, OpenMP: Add support for 'flush seq_cst' construct
On Mon, Sep 06, 2021 at 06:08:14PM +0200, Marcel Vollweiler wrote: > C, C++, Fortran, OpenMP: Add support for 'flush seq_cst' construct. > > This patch adds support for the 'seq_cst' memory order clause on the 'flush' > directive which was introduced in OpenMP 5.1. > > gcc/c-family/ChangeLog: > > * c-omp.c (c_finish_omp_flush): Handle MEMMODEL_SEQ_CST. > > gcc/c/ChangeLog: > > * c-parser.c (c_parser_omp_flush): Parse 'seq_cst' clause on 'flush' > directive. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_omp_flush): Parse 'seq_cst' clause on 'flush' > directive. > * semantics.c (finish_omp_flush): Handle MEMMODEL_SEQ_CST. > > gcc/fortran/ChangeLog: > > * openmp.c (gfc_match_omp_flush): Parse 'seq_cst' clause on 'flush' > directive. > * trans-openmp.c (gfc_trans_omp_flush): Handle OMP_MEMORDER_SEQ_CST. > > gcc/testsuite/ChangeLog: > > * c-c++-common/gomp/flush-1.c: Add test case for 'seq_cst'. > * c-c++-common/gomp/flush-2.c: Add test case for 'seq_cst'. > * g++.dg/gomp/attrs-1.C: Adapt test to handle all flush clauses. > * gfortran.dg/gomp/flush-1.f90: Add test case for 'seq_cst'. > * gfortran.dg/gomp/flush-2.f90: Add test case for 'seq_cst'. Just single space after :, not two. > --- a/gcc/testsuite/g++.dg/gomp/attrs-1.C > +++ b/gcc/testsuite/g++.dg/gomp/attrs-1.C > @@ -528,6 +528,12 @@ bar (int d, int m, int i1, int i2, int i3, int p, int > *idp, int s, >; >[[omp::directive (flush acq_rel)]] >; > + [[omp::directive (flush acquire)]] > + ; > + [[omp::directive (flush release)]] > + ; > + [[omp::directive (flush seq_cst)]] > + ; >[[omp::directive (flush (p, f))]] >; >[[omp::directive (simd When changing attrs-1.C, please also add corresponding change to attrs-2.C too, with commas after the directive name (i.e. flush, acquire etc.). Ok for trunk with those nits fixed, thanks. Jakub
[Patch] libgfortran: Makefile fix for ISO_Fortran_binding.h
Since the last libgfortran/Makefile.am commit, https://gcc.gnu.org/g:13beaf9e8d2d8264c0ad8f6504793fdcf26f3f73 the ISO_Fortran_binding.h file is no longer copied to $(build)/.../libgfortran/include/ – which breaks in-build-tree testing. The Makefile does contain the rule: ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h but make does not regard this as invitation to copy it from $srcdir to $build but just prints: make: Circular $(srcdir)/ISO_Fortran_binding.h <- $(srcdir)/ISO_Fortran_binding.h dependency dropped. As we do not actually need the ISO_Fortran_binding.h file in the $build directory (we just want to have it ready at $build/include/ for the testsuite runs), the following patch avoids an extra file in $build and also solves the dependency issue. I intent to commit it later as obvious, unless anyone has concerns, comments or a better suggestion. Tobias PS: Due to 'gfor_c_HEADERS = ISO_Fortran_binding.h', the 'make install' file is copied from $srcdir, but that's fine and copying it from $build/include/ is neither better nor worse. - 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 libgfortran: Makefile fix for ISO_Fortran_binding.h libgfortran/ChangeLog: * Makefile.am (gfor_built_src): Depend on include/ISO_Fortran_binding.h not on ISO_Fortran_binding.h. (ISO_Fortran_binding.h): Rename make target to ... (include/ISO_Fortran_binding.h): ... this. * Makefile.in: Regenerate. diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 366198b5938..008f2e7549c 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -817,7 +817,7 @@ gfor_built_src= $(i_all_c) $(i_any_c) $(i_count_c) $(i_maxloc0_c) \ $(i_pow_c) $(i_pack_c) $(i_unpack_c) $(i_matmulavx128_c) \ $(i_spread_c) selected_int_kind.inc selected_real_kind.inc kinds.h \ $(i_cshift0_c) kinds.inc c99_protos.inc fpu-target.h fpu-target.inc \ -ISO_Fortran_binding.h \ +include/ISO_Fortran_binding.h \ $(i_cshift1a_c) $(i_maxloc0s_c) $(i_minloc0s_c) $(i_maxloc1s_c) \ $(i_minloc1s_c) $(i_maxloc2s_c) $(i_minloc2s_c) $(i_maxvals_c) \ $(i_maxval0s_c) $(i_minval0s_c) $(i_maxval1s_c) $(i_minval1s_c) \ @@ -1076,15 +1076,13 @@ fpu-target.inc: fpu-target.h $(srcdir)/libgfortran.h grep '^#define GFC_FPE_' < $(top_srcdir)/../gcc/fortran/libgfortran.h > $@ || true grep '^#define GFC_FPE_' < $(srcdir)/libgfortran.h >> $@ || true -# Place ISO_Fortran_binding.h also under include/ in the build directory such +# Place ISO_Fortran_binding.h under include/ in the build directory such # that it can be used for in-built-tree testsuite runs without interference of # other files in the build dir - like intrinsic .mod files or other .h files. -ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h +include/ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h -rm -f $@ - cp $(srcdir)/ISO_Fortran_binding.h $@ $(MKDIR_P) include - -rm -f include/ISO_Fortran_binding.h - cp $@ include/ISO_Fortran_binding.h + cp $(srcdir)/ISO_Fortran_binding.h $@ ## A 'normal' build shouldn't need to regenerate these ## so we only include them in maintainer mode diff --git a/libgfortran/Makefile.in b/libgfortran/Makefile.in index a3cb6f4c5ca..5dac04e171e 100644 --- a/libgfortran/Makefile.in +++ b/libgfortran/Makefile.in @@ -1382,7 +1382,7 @@ gfor_built_src = $(i_all_c) $(i_any_c) $(i_count_c) $(i_maxloc0_c) \ $(i_pow_c) $(i_pack_c) $(i_unpack_c) $(i_matmulavx128_c) \ $(i_spread_c) selected_int_kind.inc selected_real_kind.inc kinds.h \ $(i_cshift0_c) kinds.inc c99_protos.inc fpu-target.h fpu-target.inc \ -ISO_Fortran_binding.h \ +include/ISO_Fortran_binding.h \ $(i_cshift1a_c) $(i_maxloc0s_c) $(i_minloc0s_c) $(i_maxloc1s_c) \ $(i_minloc1s_c) $(i_maxloc2s_c) $(i_minloc2s_c) $(i_maxvals_c) \ $(i_maxval0s_c) $(i_minval0s_c) $(i_maxval1s_c) $(i_minval1s_c) \ @@ -7042,15 +7042,13 @@ fpu-target.inc: fpu-target.h $(srcdir)/libgfortran.h grep '^#define GFC_FPE_' < $(top_srcdir)/../gcc/fortran/libgfortran.h > $@ || true grep '^#define GFC_FPE_' < $(srcdir)/libgfortran.h >> $@ || true -# Place ISO_Fortran_binding.h also under include/ in the build directory such +# Place ISO_Fortran_binding.h under include/ in the build directory such # that it can be used for in-built-tree testsuite runs without interference of # other files in the build dir - like intrinsic .mod files or other .h files. -ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h +include/ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h -rm -f $@ - cp $(srcdir)/ISO_Fortran_binding.h $@ $(MKDIR_P) include - -rm -f include/ISO_Fortran_binding.h - cp $@ include/ISO_Fortran_binding.h + cp $(srcdir)/ISO_Fortran_binding.h $@ @MAINTAINER_MODE_TRUE@$(i_all_c): m4/all.m4 $(I_M4_DEPS2)
[Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469)
Now I actually tested the patch – and fixed some issues. OK? – It does add support for 'allocated(a[i])' by treating it as 'allocated(a)', as 'a' must be collectively allocated ("established") on all images of the team.* 'a[i]' is (probably) an allocatable, following Malcolm in answer to my question to the J3-list as linked below. Tobias * Ignoring issues related to failed images. It could also be handled by fetching 'a' from the remote image, but I am not sure that's better in terms of handling failed images. PS: On 07.09.21 10:02, Tobias Burnus wrote: Hi Harald, I spend yesterday about two hours with this. Now I am still tired but understand more. I think the confusion between the two of us is due to wording and in which directions the thoughts then go: Talking about coindexed, all of a[i], b[i]%c and c%d[i] are coindexed and there are many constraints like "shall not be a coindexed variable" – which then rejects all of those. That's what I was thinking of. I think your starting point is that while ('a' = allocatable) a, b%a, c[5]%d(1)%a are ALLOCATABLE, adding a subobject reference such as a(:), b%a(:,:), c[5]%d(1)%a(:,:,:) makes the variable no longer allocatable. I think that's what you were thinking of. We then both argued along those different lines – which caused the confusion as we both thought we talked about the same. While those cases are clear, the question is whether a[i] or b%a[i] is allocatable or not – assuming that 'a' is a scalar. (For an array, '(:)' has to appear before the image-selector, which in turn makes it nonallocatable.) I tried to pinpoint the words for this in the standard – and failed. I think I need a "how to read the Fortran standard" 101 and some long time actually reading it :-( Malcolm has answered me – and he believes (but only offhand) that a[i] and b%a[i] _are_ allocatable. See (6) at https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html This implies that if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1 is valid. However, I do note that coarray allocatables have to be collectively (de)allocated, therefore allocated (a[i]) .and. allocated (b%a[i]) is equivalent to allocated (a) .and. allocated (b%a) at least assuming that no image has failed. First: Does this answer all the questions you had and resolved the confusion? Secondly, do you agree about the last bits of the analysis? Thirdly, what do you think of the attached patch? 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 Fortran: Handle allocated() with coindexed scalars [PR93834] 2021-09-07 Harald Anlauf Tobias Burnus While for an allocatable 'array', 'array(:)' and 'array(:)[1]' are not allocatable, it is believed that not only 'scalar' but also 'scalar[1]' is allocatable. However, coarrays are collectively established/allocated; thus, 'allocated(scalar[i])' is equivalent to 'allocated(scalar)'. [At least when assuming that 'i' does not refer to a failed image.] PR fortran/93834 gcc/fortran/ChangeLog: * trans-intrinsic.c (gfc_conv_allocated): Cleanup. Handle coindexed scalar coarrays. gcc/testsuite/ChangeLog: * gfortran.dg/coarray_allocated.f90: New test. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 46670baae55..c9d1aace33e 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -8887,50 +8887,63 @@ caf_this_image_ref (gfc_ref *ref) static void gfc_conv_allocated (gfc_se *se, gfc_expr *expr) { - gfc_actual_arglist *arg1; gfc_se arg1se; tree tmp; - symbol_attribute caf_attr; + bool coindexed_caf_comp = false; + gfc_expr *e = expr->value.function.actual->expr; gfc_init_se (&arg1se, NULL); - arg1 = expr->value.function.actual; - - if (arg1->expr->ts.type == BT_CLASS) + if (e->ts.type == BT_CLASS) { /* Make sure that class array expressions have both a _data component reference and an array reference */ - if (CLASS_DATA (arg1->expr)->attr.dimension) - gfc_add_class_array_ref (arg1->expr); + if (CLASS_DATA (e)->attr.dimension) + gfc_add_class_array_ref (e); /* whilst scalars only need the _data component. */ else - gfc_add_data_component (arg1->expr); + gfc_add_data_component (e); } - /* When arg1 references an allocatable component in a coarray, then call + /* When 'e' references an allocatable component in a coarray, then call the caf-library function caf_is_present (). */ - if (flag_coarray == GFC_FCOARRAY_LIB && arg1->expr->expr_type == EXPR_FUNCTION - && arg1->expr->value.function.isym - && arg1->expr->value.function.isym->id == GFC_ISYM_CAF_GET) -caf_attr = gfc_caf_attr (arg1->expr->value.function.actual->expr); - else -gfc_clear_attr (&caf_att
Re: [Patch] libgfortran: Makefile fix for ISO_Fortran_binding.h
Now committed as r12-3384-gfc4f0631de806c89a383fd02428a16e91068b9f6 Sorry for the breakage – and thanks for the report on IRC, Richard! Tobias On 07.09.21 16:13, Tobias Burnus wrote: Since the last libgfortran/Makefile.am commit, https://gcc.gnu.org/g:13beaf9e8d2d8264c0ad8f6504793fdcf26f3f73 the ISO_Fortran_binding.h file is no longer copied to $(build)/.../libgfortran/include/ – which breaks in-build-tree testing. The Makefile does contain the rule: ISO_Fortran_binding.h: $(srcdir)/ISO_Fortran_binding.h but make does not regard this as invitation to copy it from $srcdir to $build but just prints: make: Circular $(srcdir)/ISO_Fortran_binding.h <- $(srcdir)/ISO_Fortran_binding.h dependency dropped. As we do not actually need the ISO_Fortran_binding.h file in the $build directory (we just want to have it ready at $build/include/ for the testsuite runs), the following patch avoids an extra file in $build and also solves the dependency issue. I intent to commit it later as obvious, unless anyone has concerns, comments or a better suggestion. Tobias PS: Due to 'gfor_c_HEADERS = ISO_Fortran_binding.h', the 'make install' file is copied from $srcdir, but that's fine and copying it from $build/include/ is neither better nor worse. - 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
Re: [Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469)
Hi Tobias, I think I can follow now what you are thinking, and I also had some thoughts about what you be done in principle. I was struggling the way I did because of: (1) Intel rejects the code in the PR. For my previous patch, % ifort coarray_allocated.f90 -coarray coarray_allocated.f90(8): error #7364: The argument to the ALLOCATED intrinsic cannot be a coindexed object. [A] print *, allocated (a[1]) ! { dg-error "shall not be coindexed" } --^ compilation aborted for coarray_allocated.f90 (code 1) (2) F2018: 16.9.11 ALLOCATED (ARRAY) or ALLOCATED (SCALAR) Arguments. ARRAY shall be an allocatable array. SCALAR shall be an allocatable scalar. (3) F2018: 9.4 Scalars 9.4.3 Coindexed named objects A coindexed-named-object is a named scalar coarray variable followed by an image selector. F2018: 9.5 Arrays (or 9.4 Scalars) F2018: 9.6 Image selectors An image selector determines the image index for a coindexed object. Those are the sources of information I had, and which I interpreted in the way that even if A is an allocatable coarray (scalar or array), when adding an image selector, like in A[N], that object would not satisfy the requirements for the ALLOCATED intrinsic. It also doesn't say coarray here. I really didn't think about the synchronization stuff here. I also didn't read the section on the ALLOCATE statement, but in fact there is the following (probably in line with your argument): 9.7.1.2 Execution of an ALLOCATE statement "The coarray shall not become allocated on an image unless it is successfully allocated on all active images in this team." and the following note which says: "When an image executes an ALLOCATE statement, communication is not necessarily involved apart from any required for synchronization. ..." So a dirty shortcut could be allowed if the ALLOCATED() is considered valid. Harald > Gesendet: Dienstag, 07. September 2021 um 16:33 Uhr > Von: "Tobias Burnus" > An: "Harald Anlauf" > Cc: "fortran" , "gcc-patches" > Betreff: [Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] > (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in > trans_caf_is_present, at fortran/trans-intrinsic.c:8469) > > Now I actually tested the patch – and fixed some issues. > > OK? – It does add support for 'allocated(a[i])' by treating > it as 'allocated(a)', as 'a' must be collectively allocated > ("established") on all images of the team.* > > 'a[i]' is (probably) an allocatable, following Malcolm in > answer to my question to the J3-list as linked below. > > Tobias > > * Ignoring issues related to failed images. It could > also be handled by fetching 'a' from the remote > image, but I am not sure that's better in terms of > handling failed images. > > PS: > On 07.09.21 10:02, Tobias Burnus wrote: > > Hi Harald, > > > > I spend yesterday about two hours with this. Now I am still > > tired but understand more. I think the confusion between the > > two of us is due to wording and in which directions the > > thoughts then go: > > > > > > Talking about coindexed, all of a[i], b[i]%c and c%d[i] are > > coindexed and there are many constraints like "shall not be > > a coindexed variable" – which then rejects all of those. > > That's what I was thinking of. > > > > I think your starting point is that while ('a' = allocatable) > > a, b%a, c[5]%d(1)%a > > are ALLOCATABLE, adding a subobject reference such as > > a(:), b%a(:,:), c[5]%d(1)%a(:,:,:) > > makes the variable no longer allocatable. > > I think that's what you were thinking of. > > > > We then both argued along those different lines – which caused > > the confusion as we both thought we talked about the same. > > > > > > While those cases are clear, the question is whether > > a[i] or b%a[i] > > is allocatable or not – assuming that 'a' is a scalar. > > (For an array, '(:)' has to appear before the image-selector, > > which in turn makes it nonallocatable.) > > > > > > I tried to pinpoint the words for this in the standard – and > > failed. I think I need a "how to read the Fortran standard" 101 > > and some long time actually reading it :-( > > > > Malcolm has answered me – and he believes (but only offhand) that > > a[i] and b%a[i] > > _are_ allocatable. See (6) at > > https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html > > > > > > This implies that > > if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1 > > is valid. > > > > However, I do note that coarray allocatables have to be collectively > > (de)allocated, therefore > > allocated (a[i]) .and. allocated (b%a[i]) > > is equivalent to > > allocated (a) .and. allocated (b%a) > > at least assuming that no image has failed. > > > > > > First: Does this answer all the questions you had and resolved the > > confusion? > > Secondly, do you agree about the last bits of the analysis? > > Thirdly, what do you think of the attached patch? > > > > Tobias > - > Sie
[PATCH] PR fortran/82314 - ICE in gfc_conv_expr_descriptor, at fortran/trans-array.c:6972
When adding the initializer for an array, we need to make sure that array bounds are properly simplified if that array is a PARAMETER. Otherwise the generated initializer could be wrong and screw up subsequent simplifications, see PR. The minimal solution is to attempt simplification of array bounds before adding the initializer as in the attached patch. (We could place that part in a helper function if this functionality is considered useful elsewhere). Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Fortran - ensure simplification of bounds of array-valued named constants gcc/fortran/ChangeLog: PR fortran/82314 * decl.c (add_init_expr_to_sym): For proper initialization of array-valued named constants the array bounds need to be simplified before adding the initializer. gcc/testsuite/ChangeLog: PR fortran/82314 * gfortran.dg/pr82314.f90: New test. diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 2e49a673e15..f2e8896b562 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -2169,6 +2169,24 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus) sym->as->type = AS_EXPLICIT; } + /* Ensure that explicit bounds are simplified. */ + if (sym->attr.flavor == FL_PARAMETER && sym->attr.dimension + && sym->as->type == AS_EXPLICIT) + { + for (int dim = 0; dim < sym->as->rank; ++dim) + { + gfc_expr *e; + + e = sym->as->lower[dim]; + if (e->expr_type != EXPR_CONSTANT) + gfc_reduce_init_expr (e); + + e = sym->as->upper[dim]; + if (e->expr_type != EXPR_CONSTANT) + gfc_reduce_init_expr (e); + } + } + /* Need to check if the expression we initialized this to was one of the iso_c_binding named constants. If so, and we're a parameter (constant), let it be iso_c. diff --git a/gcc/testsuite/gfortran.dg/pr82314.f90 b/gcc/testsuite/gfortran.dg/pr82314.f90 new file mode 100644 index 000..3a147e22711 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr82314.f90 @@ -0,0 +1,11 @@ +! { dg-do run } +! PR fortran/82314 - ICE in gfc_conv_expr_descriptor + +program p + implicit none + integer, parameter :: karray(merge(3,7,.true.):merge(3,7,.false.)) = 1 + integer, parameter :: i = size (karray) + integer, parameter :: l = lbound (karray,1) + integer, parameter :: u = ubound (karray,1) + if (l /= 3 .or. u /= 7 .or. i /= 5) stop 1 +end