[Bug fortran/62131] [4.9/5 Regression] OpenMP: Subobject of an allocatable array not allowed in OMP ATOMIC

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62131

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-08-14
   Target Milestone|4.9.3                       |4.9.2
            Summary|[4.9.1 Regression] OpenMP:  |[4.9/5 Regression] OpenMP:
                   |Subobject of an allocatable |Subobject of an allocatable
                   |array not allowed in OMP    |array not allowed in OMP
                   |ATOMIC                      |ATOMIC
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #1)
> The current code, cf. line 2744 of the gcc/fortran/openmp.c, uses:
>   var = code->expr1->symtree->n.sym;
> ...
>   if (var->attr.allocatable)
> 
> I think the simplest would be to use:
>    if (gfc_expr_attr (code->expr1).allocatable))

That looks good to me, preapproved for trunk/4.9.2 with a testcase.

> However, there might be other spots in the code where using "var" looks at
> the wrong thing for component references (like: "var%comp"). For instance,
>               || expr2_tmp->symtree->n.sym == var)
> might reject  "var%b = var%a" code - which may or may not be intended.

Fix that is non-easy though.
var is used e.g. for the expr_references_sym checks.
Some of those are used to complain about invalid code, but if
expr1->ref is non-NULL, then we may reject valid code.
Say
!$omp atomic write
  var(1) = var(2)
is fine, yet we'll complain.  Those could be fixed by only calling
expr_references_sym if expr1->ref is NULL, and simply not diagnose it at all
otherwise.  Or expr_references_sym could be enhanced not to take symbol, but
the EXPR_VARIABLE gfc_expr * instead, and it could just try to compare the refs
too.  I believe in the spec when it talks about x, it should be always the same
sequence of tokens, so say
!$omp atomic
  var(i) = var(j) + 1
is likely invalid even if i has the same value as j, the standard requires that
no other expression may reference the same memory as x (etc.), but that is
pretty much impossible to check.
Anyway, bigger issue is the atomic swap (for which I'm guilty as the one who
requested the addition of that into the standard).
For that unfortunately the being conservative is bad, but the current state is
bad too.
!$omp atomic capture
  v = var(i)
  var(i) = var(j) + var(k)
!$omp end atomic
(of course, invalid with i == j or i == k) should be valid atomic swap that we
likely mishandle right now, while
!$omp atomic capture
  v = var(i)
  var(i) = var(j) + var(i)
!$omp end atomic
is not a capture.  So, I'm afraid we need to improve expr_references_sym.
And then there are the other comparisons you've mentioned, not sure what
exactly we can do about those :(.