On Mon, Nov 15, 2021 at 12:29:31PM +0100, Tobias Burnus wrote:
> --- a/gcc/fortran/dump-parse-tree.c
> +++ b/gcc/fortran/dump-parse-tree.c
> @@ -1926,6 +1930,22 @@ show_omp_clauses (gfc_omp_clauses *omp_clauses)
> fputc (' ', dumpfile);
> fputs (memorder, dumpfile);
> }
> + if (omp_clauses->fail != OMP_MEMORDER_UNSET)
> + {
> + const char *memorder;
> + switch (omp_clauses->fail)
> + {
> + case OMP_MEMORDER_ACQ_REL: memorder = "ACQ_REL"; break;
No need for the above line.
> + case OMP_MEMORDER_ACQUIRE: memorder = "AQUIRE"; break;
> + case OMP_MEMORDER_RELAXED: memorder = "RELAXED"; break;
> + case OMP_MEMORDER_RELEASE: memorder = "RELEASE"; break;
And above line either. They aren't allowed for fail clause and
you reject it already during parsing, so the default: gcc_unreachable ();
can handle it fine.
> + case OMP_MEMORDER_SEQ_CST: memorder = "SEQ_CST"; break;
> + default: gcc_unreachable ();
> @@ -1449,8 +1452,9 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> gcc_checking_assert (OMP_MASK1_LAST <= 64 && OMP_MASK2_LAST <= 64);
> *cp = NULL;
> while (1)
> - {
> - if ((first || gfc_match_char (',') != MATCH_YES)
> + {
Why the added trailing whitespace after { ?
> + match m = MATCH_NO;
> + if ((first || (m = gfc_match_char (',')) != MATCH_YES)
> && (needs_space && gfc_match_space () != MATCH_YES))
> break;
> needs_space = false;
> @@ -1460,7 +1464,11 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> gfc_omp_namelist **head;
> old_loc = gfc_current_locus;
> char pc = gfc_peek_ascii_char ();
> - match m;
> + if (pc == '\n' && m == MATCH_YES)
> + {
> + gfc_error ("Clause expected at %C after tailing comma");
Do you mean trailing ?
> + if ((mask & OMP_CLAUSE_FAIL)
> + && (m = gfc_match_dupl_check (c->fail == OMP_MEMORDER_UNSET,
> + "fail", true)) != MATCH_NO)
> + {
> + if (m == MATCH_ERROR)
> + goto error;
> + if (gfc_match ("seq_cst") == MATCH_YES)
> + c->fail = OMP_MEMORDER_SEQ_CST;
> + else if (gfc_match ("acquire") == MATCH_YES)
> + c->fail = OMP_MEMORDER_ACQUIRE;
> + else if (gfc_match ("relaxed") == MATCH_YES)
> + c->fail = OMP_MEMORDER_RELAXED;
> + else
> + {
> + gfc_error ("Expected SEQ_CST, ACQUIRE or RELAXED at %C");
> + break;
> + }
Here is where you make sure c->fail isn't OMP_MEMORDER_{RELEASE,ACQ_REL} ...
> -static void
> +/*static */ void
> resolve_omp_atomic (gfc_code *code)
Why?
> + if (stmt && !capture_stmt && next->block->block)
> + {
> + if (next->block->block->expr1)
> + {
> + gfc_error ("Expected ELSE at %L in atomic compare capture",
> + &next->block->block->expr1->where);
> + }
No {}s around single statement body.
> @@ -4508,6 +4508,17 @@ gfc_trans_omp_atomic (gfc_code *code)
> case OMP_MEMORDER_SEQ_CST: mo = OMP_MEMORY_ORDER_SEQ_CST; break;
> default: gcc_unreachable ();
> }
> + switch (atomic_code->ext.omp_clauses->fail)
> + {
> + case OMP_MEMORDER_UNSET: fail_mo = OMP_FAIL_MEMORY_ORDER_UNSPECIFIED;
> break;
> + case OMP_MEMORDER_ACQ_REL: fail_mo = OMP_FAIL_MEMORY_ORDER_RELAXED;
> break;
> + case OMP_MEMORDER_ACQUIRE: fail_mo = OMP_FAIL_MEMORY_ORDER_ACQUIRE;
> break;
> + case OMP_MEMORDER_RELAXED: fail_mo = OMP_FAIL_MEMORY_ORDER_RELAXED;
> break;
> + case OMP_MEMORDER_RELEASE: fail_mo = OMP_FAIL_MEMORY_ORDER_RELEASE;
> break;
> + case OMP_MEMORDER_SEQ_CST: fail_mo = OMP_FAIL_MEMORY_ORDER_SEQ_CST;
> break;
Again, no reason to handle OMP_MEMORDER_ACQ_REL and OMP_MEMORDER_RELEASE
above.
Otherwise LGTM.
Jakub
