On 1/13/25 12:18 AM, Tobias Burnus wrote:
Hi,
On 9/25/24 3:18 AM, Andre Vehreschild wrote:
@@ -3089,7 +3099,15 @@ typedef struct gfc_code
gfc_inquire *inquire;
gfc_wait *wait;
gfc_dt *dt;
- gfc_forall_iterator *forall_iterator;
+
+ struct
+ {
+ gfc_forall_iterator *forall_iterator;
+ gfc_expr_list *locality[LOCALITY_NUM];
+ bool default_none;
+ }
+ concur;
I am more than unhappy about that construct. Because every concurrent
loop has
a forall_iterator, but not every forall_iterator is a concurrent loop. I
therefore propose to move the forall_iterator out of the struct and
only have
the concurrent specific elements in the struct. This would also reduce
the
changes significantly.
First, regarding the naming, Fortran 2018 had:
FORALL forall-header
[do-stmt …]
CONCURRENT forall-header
Thus, both do-concurrent and forall used both a forall-header.
Since Fortran 2023, there is now:
FORALL concurrent-header
etc.
Thus, in Fortran 2024 both use a concurrent-header.
* * *
On the technical side:
DO CONCURRENT (i = 1:5) mask(.true.) local(x) default(none)
This has two parts:
* A forall_iterator: 'i = 1:5' and one 'mask' expression
* A locality spec: default(none) local(x)
The forall-header is saved on mainline as:
new_st.expr1 = mask;
new_st.ext.forall_iterator = head;
Since Fortran 2018 (and for this patch) we additionally have to save
for 'do concurrent' somehow a boolean ('default(none)') and a list of
symbols (with knowledge about in which locality they appeared).
Storing the forall-header iterator in <gfc_code>.ext.forall_iterator
sounds fine, but where to put locality other data?
The most sensible place is to put it also into <gfc_code>.ext, but as
the latter is a union, we need to ensure that both the iterator *and*
the locality data is available. - Thus, we create a struct for
'do concurrent'. But as the forall-header / concurrent-header is identical,
it makes sense to also use the same struct for FORALL and not to duplicate
code here.
IMHO the current code is fine.
* * *
On 1/7/25 12:06 PM, Jerry D wrote:
cannot understand why moving the forall_iterator from the sub-
structure 'concur' back to where it was at the 'ext' sub-structure of
typedef struct gfc_code. 'ext' is a union. I suspected there is an
overlap going on there such that something is getting overwritten or
optimized away.
Well, as mentioned, a DO CONCURRENT can have both. Assume a simple do-
concurrent loop:
DO, concurrent (I = 0:4)
This will fill code.ext.forall_iterator, which is fine. When this being
resolved
in resolve.cc, for checking the iterator, the access goes to
code.ext.forall_iterator
which is fine - this will work for the example above.
As next step, the locality is checked, accessing
code.ext.concurr.*
but that variable shares the memory (union!) with
code.ext.forall_iterator. Thus,
accessing locality[0] will be identical to
static_cast<gfc_expr_list *>(code.ext.forall_iterator)
and this has a very high chance to crash.
When using 'do concurrent(i=0:5) local(x)', the code.ext.forall_iterator
would be already overridden during parsing.
* * *
I hope that helps!
Tobias
Thank you for this explanation. I did do one other thing which is
rearranged the typedef as follows which is more readable to me. If this
is OK I will push the whole thing. I do need to do the gcc-commit-mklog
part of the whole thing.
Regression tested on x86_64.
OK?
Jerry
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 70913e3312b..7367db8853c 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3111,6 +3111,16 @@ enum gfc_exec_op
EXEC_OMP_ERROR, EXEC_OMP_ALLOCATE, EXEC_OMP_ALLOCATORS,
EXEC_OMP_DISPATCH
};
+/* Enum Definition for locality types. */
+enum locality_type
+{
+ LOCALITY_LOCAL = 0,
+ LOCALITY_LOCAL_INIT,
+ LOCALITY_SHARED,
+ LOCALITY_REDUCE,
+ LOCALITY_NUM
+};
+
typedef struct gfc_code
{
gfc_exec_op op;
@@ -3131,6 +3141,20 @@ typedef struct gfc_code
{
gfc_actual_arglist *actual;
gfc_iterator *iterator;
+ gfc_open *open;
+ gfc_close *close;
+ gfc_filepos *filepos;
+ gfc_inquire *inquire;
+ gfc_wait *wait;
+ gfc_dt *dt;
+ struct gfc_code *which_construct;
+ gfc_entry_list *entry;
+ gfc_oacc_declare *oacc_declare;
+ gfc_omp_clauses *omp_clauses;
+ const char *omp_name;
+ gfc_omp_namelist *omp_namelist;
+ bool omp_bool;
+ int stop_code;
struct
{
@@ -3152,21 +3176,13 @@ typedef struct gfc_code
}
block;
- gfc_open *open;
- gfc_close *close;
- gfc_filepos *filepos;
- gfc_inquire *inquire;
- gfc_wait *wait;
- gfc_dt *dt;
- gfc_forall_iterator *forall_iterator;
- struct gfc_code *which_construct;
- int stop_code;
- gfc_entry_list *entry;
- gfc_oacc_declare *oacc_declare;
- gfc_omp_clauses *omp_clauses;
- const char *omp_name;
- gfc_omp_namelist *omp_namelist;
- bool omp_bool;
+ struct
+ {
+ gfc_forall_iterator *forall_iterator;
+ gfc_expr_list *locality[LOCALITY_NUM];
+ bool default_none;
+ }
+ concur;
}
ext; /* Points to additional structures required by statement */
