Re: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469

2021-09-07 Thread Tobias Burnus

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

2021-09-07 Thread Jakub Jelinek via Fortran
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

2021-09-07 Thread Tobias Burnus

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)

2021-09-07 Thread Tobias Burnus

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

2021-09-07 Thread Tobias Burnus

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)

2021-09-07 Thread Harald Anlauf via Fortran
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

2021-09-07 Thread Harald Anlauf via Fortran
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