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

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Jul 04, 2022 at 04:10:03PM +0200, Tobias Burnus wrote:
> This patch adds support for the OpenMP 5.2 syntax for the linear clause,
> following the C/C++ patch. The testcases are modified versions from the
> C/C++ ones, plus one added one for duplicated modifiers.
> 
> At least to me it is not quite clear when
>   linear ( var : ref)
> refers to a variable 'ref' and when to the linear-modifier 'ref'; the
> spec does not seem to be very clear about it. I made an attempt, based

See OpenMP 5.2 [59:31-34]:
A modifier that is an expression must neither lexically match the name of a simple modifier
defined for the clause that is an OpenMP keyword nor modifier-name parenthesized-tokens,
where modifier-name is the modifier-name of a complex modifier defined for the clause and
parenthesized-tokens is a token sequence that starts with ( and ends with ).

So, ref can't be step expression because it lexically matches the name of a
simple modifier, so linear (var : ref) is equivalent to old style linear (ref (var):1)
while e.g. linear (var : ref + 0) is equivalent to linear (var : step (ref + 0))

> +	      else if (end_colon)
> +		{
> +		  gfc_symtree *st;
> +		  bool has_modifiers = false;
> +		  bool duplicate_step = false;
> +		  bool duplicate_mod = false;
> +		  while (true)
> +		    {
> +		      old_loc = gfc_current_locus;
> +		      if (gfc_match ("val )") == MATCH_YES)
> +			{

So, if you see val ) even right after colon (when !old_linear_modifiers), it is
always linear modifier, so the if (!has_modifiers) looks wrong.

> +			  if (!has_modifiers)
> +			    {
> +			      gfc_find_sym_tree ("val", NULL, true, &st);
> +			      bool has_val = (st
> +					      && !st->n.sym->attr.function
> +					      && !st->n.sym->attr.dimension);
> +			      locus loc = gfc_current_locus;
> +			      gfc_current_locus = old_loc;
> +			      if (has_val
> +				  && gfc_match (" %e ) ", &step) == MATCH_YES)
> +				break;
> +			      gfc_current_locus = loc;
> +			    }
> +			  if (linear_op != OMP_LINEAR_DEFAULT)
> +			    {
> +			      duplicate_mod = true;
> +			      break;
> +			    }
> +			  linear_op = OMP_LINEAR_VAL;
> +			  has_modifiers = true;
> +			  break;
> +			}
> +		      else if (gfc_match ("uval )") == MATCH_YES)
> +			{

Likewise.

> +			  if (!has_modifiers)

> +		      else if (gfc_match ("ref )") == MATCH_YES)
> +			{

And again.

> +			  if (!has_modifiers)

> +		      else if (gfc_match ("step ( ") == MATCH_YES)
> +			{

step ( could start both valid step expression and be a valid modifier.

But that decision shouldn't be based on whether there is a step symtree or
not, but whether it is step ( whatever ) ) or step ( whatever ) ,
(in that case it should be parsed as the complex modifier with expression
in it), otherwise it is parsed as step expression.

The whatever above means some tokens with balanced parentheses.

I doubt the Fortran FE has something like that right now.

You can certainly try to match "step ( %e ) )" or "step ( %e ) , " first,
those would handle the case of valid complex modifier.
But, I think if there is
  interface
    integer function step (x, y, z)
      integer :: x, y, z
    end function step
  end interface
then
  linear (v : step (x, y, z))
should be rejected, not accepted as valid
  linear (v : step (step (x, y, z)))

I think I should add:
int step (int x, int y, int z) { return x + y + z; }

int
foo (int x)
{
  int i;
  #pragma omp parallel for linear (x : step (step (1, 2, 3)))
  for (i = 0; i < 64; i++)
    x += 6;
  return x;
}

int
bar (int x)
{
  int i;
  #pragma omp parallel for linear (x : step (1, 2, 3))	/* { dg-error "expected" } */
  for (i = 0; i < 64; i++)
    x += 6;
  return x;
}
as another testcase (where foo used to be invalid before and bar used to be
valid).

	Jakub