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
