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