Hi all,
I finally managed to apply the fixed patch. It still had some stray line break
so check_GNU_style.py wouldn't succeed. But with that fixed I agree to have
only some nonsense bickering of the script.
As to the patch (I have stripped large parts.):
> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 36ed8eeac2d..c6aefb81a73 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -3042,6 +3042,16 @@ enum gfc_exec_op
> EXEC_OMP_ERROR, EXEC_OMP_ALLOCATE, EXEC_OMP_ALLOCATORS
> };
>
> +/* 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;
> @@ -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.
I keep thinking about a "do concurrent " when all that is done is a regular "do
" in Fortran just by having this concur in between.
> +
> struct gfc_code *which_construct;
> int stop_code;
> gfc_entry_list *entry;
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 4f4fafa4217..b0eed12afed 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
<snipp>
+static void
+resolve_locality_spec (gfc_code *code, gfc_namespace *ns)
+{
+ struct check_default_none_data data;
+ data.code = code;
+ data.sym_hash = new hash_set<gfc_symbol *>;
Memory new'ed here is never delete'd. Memory leak!
+ data.ns = ns;
+ data.default_none = code->ext.concur.default_none;
+
+ for (int locality = 0; locality < LOCALITY_NUM; locality++)
I haven't checked the tests, because Harald did so.
Remark: How does this patch interplay with the UNSIGNED patch? I've seen some
warnings about numeric types, where I suppose also UNSIGNED may be valid.
I like to have my questions answered first before I ok this patch.
Regards,
Andre
On Mon, 23 Sep 2024 22:55:51 +0200
Tobias Burnus <tbur...@baylibre.com> wrote:
> Hi all,
>
> I have now downloaded the file at
> https://gcc.gnu.org/pipermail/gcc-patches/2024-September/663534.html (by
> copying it from the browser, not the source code to avoid '>
>
> This file had had to fix spurious line breaks like:
>
> @@ -5171,7 +5171,7 @@ index_interchange (gfc_code **c, int
> *walk_subtrees ATTRIBUTE_UNUSED,
>
> where the *... belongs to the previous line.
>
> the result of this conversion is the attached file.
>
> * * *
>
> Harald Anlauf wrote:
> > Generally speaking, runtime tests should verify that they work as
> > expected.
>
> There are currently only compile-time tests.
>
> [One might argue that some should be run-time tests, albeit the really
> interesting part only happens with local/local_init (currently not
> supported) – and with true concurrency in particular with 'reduce'.]
>
> [The interesting cases of 'local'/'local_init' there is a currently a
> 'sorry' while 'reduce' only becomes truly interesting if one goes
> parallel …]
>
> Tobias
--
Andre Vehreschild * Email: vehre ad gmx dot de
