On Wed, Jul 29, 2020 at 10:10:35AM +0200, Tobias Burnus wrote:
> + case AB_OMP_REQ_REVERSE_OFFLOAD:
> + gfc_omp_requires_add_clause (OMP_REQ_REVERSE_OFFLOAD,
> + "reverse_offload",
> + &gfc_current_locus,
> + module_name);
Even visually something is wrong with the indentation here and in a few
others, where I'd expect all the arguments to be in the same column (even
with the > + before it, but they are not. E.g. above I think the
gfc_omp_... is indented by 3 instead of 2 columns from case, and "rev... and
&gfc... match that, but module_name is probably correct.
> + break;
> + case AB_OMP_REQ_UNIFIED_ADDRESS:
> + gfc_omp_requires_add_clause (OMP_REQ_UNIFIED_ADDRESS,
> + "unified_address",
> + &gfc_current_locus,
And e.g. here the &gfc... is off.
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see
> #include "diagnostic.h"
> #include "gomp-constants.h"
>
> +
Unnecessary.
> /* Match an end of OpenMP directive. End of OpenMP directive is optional
> whitespace, followed by '\n' or comment '!'. */
>
> @@ -3745,6 +3970,26 @@ gfc_match_omp_oacc_atomic (bool omp_p)
> new_st.op = (omp_p ? EXEC_OMP_ATOMIC : EXEC_OACC_ATOMIC);
> if (seq_cst)
> op = (gfc_omp_atomic_op) (op | GFC_OMP_ATOMIC_SEQ_CST);
> + else
I wonder if this shouldn't be else if (omp_p), I'd think
OpenACC atomics shouldn't be affected by OpenMP requires directive.
> for (gfc_current_ns = gfc_global_ns_list; gfc_current_ns;
> gfc_current_ns = gfc_current_ns->sibling)
> - gfc_check_externals (gfc_current_ns);
> + gfc_check_omp_requires (gfc_current_ns, omp_requires);
Again indentation looks weird.
Otherwise LGTM.
Jakub
