Re: [Patch] OpenMP/Fortran: Add support for OpenMP 5.2 linear clause syntax

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Jul 04, 2022 at 06:09:31PM +0200, Tobias Burnus wrote:
> thanks for the comment & spec referral. I have now updated the patch –
> and included the new C/Fortran testcase.

Thanks.

> +		  while (true)
> +		    {
> +		      old_loc = gfc_current_locus;
> +		      if (gfc_match ("val )") == MATCH_YES)
> +			{
> +			  if (linear_op != OMP_LINEAR_DEFAULT)
> +			    {
> +			      duplicate_mod = true;
> +			      break;
> +			    }
> +			  linear_op = OMP_LINEAR_VAL;
> +			  has_modifiers = true;
> +			  break;
> +			}
> +		      else if (gfc_match ("val , ") == MATCH_YES)
> +			{
> +			  if (linear_op != OMP_LINEAR_DEFAULT)
> +			    {
> +			      duplicate_mod = true;
> +			      break;
> +			    }
> +			  linear_op = OMP_LINEAR_VAL;
> +			  has_modifiers = true;
> +			  continue;
> +			}

Perhaps you could avoid some code duplication by doing it like:
		      bool close_paren = gfc_match ("val )") == MATCH_YES;
		      if (close_paren || gfc_match ("val , ") == MATCH_YES)
			{
			  if (linear_op != OMP_LINEAR_DEFAULT)
			    {
			      duplicate_mod = true;
			      break;
			    }
			  linear_op = OMP_LINEAR_VAL;
			  has_modifiers = true;
			  if (close_paren)
			    break;
			  else
			    continue;
			}
and similarly for uval and ref.

> +		      else if (!has_modifiers
> +			       && gfc_match ("%e )", &step) == MATCH_YES)
> +			{
> +			  if ((step->expr_type == EXPR_FUNCTION
> +				|| step->expr_type == EXPR_VARIABLE)
> +			      && strcmp (step->symtree->name, "step") == 0)
> +			    {
> +			      gfc_current_locus = old_loc;
> +			      gfc_match ("step (");
> +			      has_error = true;
> +			    }

I think the above should accept even
linear (v : step (1) + 0) or linear (v : step (1, 2, 3) * 1)
which is desirable, because step then will be some other operation (hope
folding isn't performed yet).

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/gomp/linear-4.f90
> @@ -0,0 +1,102 @@
> +! { dg-do compile }
> +! { dg-options "-fopenmp" }
> +
> +module m
> +implicit none
> +
> +integer :: i
> +
> +interface
> +  integer function bar (x,  y, z)
> +    integer :: x, y
> +    integer, value :: z
> +    !!$omp declare simd linear (x : ref, step (1)) linear (y : step (2), uval)

Are all these !! intentional?
The test then doesn't test much.

Or is that a FIXME?

	Jakub