[patch] Fortran/OpenMP: Support most of 5.1 atomic extensions

Jakub Jelinek jakub@redhat.com
Fri Dec 3 14:47:31 GMT 2021


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



More information about the Gcc-patches mailing list