[flang-commits] [PATCH] D89473: [flang] Document and use intrinsic subroutine argument intents

[Jean Perier via Phabricator via flang-commits]

jeanPerier marked an inline comment as done.
jeanPerier added inline comments.


================
Comment at: flang/lib/Semantics/check-call.cpp:316
   }
-  if (actualLastObject && actualLastObject->IsCoarray() &&
+  if (!isIntrinsicCall && actualLastObject && actualLastObject->IsCoarray() &&
       IsAllocatable(*actualLastSymbol) &&
----------------
klausler wrote:
> jeanPerier wrote:
> > klausler wrote:
> > > Are all ALLOCATABLE coarrays really acceptable as actual arguments to `INTENT(OUT)` intrinsic subroutine arguments?
> > What I am sure of is that C846 does not apply to intrinsic procedure, and that is is clearly OK for some intrinsic subroutine to have ALLOCATBLE coarrays as actual argument of INTENT(OUT) intrinsic arguments.
> > 
> > What I am not sure of is that it is OK in all intrinsic subroutine. e.g.: is it OK for COUNT argument of SYSTEM_CLOCK to get a coarray allocatable argument ? I can't find a rule that prevents it given C846 clearly does not apply for SYSTEM_CLOCK and SYSTEM_CLOCK says nothing about coarray/coindexed arguments.
> > 
> > I can't find narrower sort of allocable coarrays that would always be forbidden in intrinsic subroutine INTENT(OUT) arguments. Maybe coindexed objects should always be banned from INTENT(OUT), but the problem seems orthogonal to allocatables, all INTENT(OUT) intrinsic argument specific that implicitly tells that coarray are ok (by restricting things on their coranks) bans coindexed objects, whether their base is allocatable or not. Even then, it seems left to each intrinsic to define this (since some are explicitly banning them).
> > 
> > My conclusion from this is that we cannot derive a generic rules about allocatable coarrays in INTENT(OUT) intrinsic subroutine argument that we could enforce here. However, we may want to strengthen case by case intrinsic checks in intrinsic.cpp. For instance its clear coindexed objects are not OK in move_alloc and we are not complaining about this (both before or after this patch).
> > 
> > **Why I am sure it is OK for at least some intrinsics**
> > C846 :
> > > An INTENT (OUT) dummy argument of a nonintrinsic procedure shall not be an allocatable coarray or
> > > have a subobject that is an allocatable coarray.
> > 
> > MOVE_ALLOC TO argument that is INTENT(OUT) clearly accepts allocatable coarray actual arguments (16.9.137):
> > > TO be type compatible (7.3.2.3) with FROM and have the same rank and corank.
> > > It shall be allocatable and shall not be a coindexed object.
> > > It shall be polymorphic if FROM is polymorphic. It is an INTENT (OUT) argument.
> > > Each nondeferred parameter of the declared type of TO shall have the same value as the corresponding parameter of the declared type of FROM.
> > 
> > **Why I am not sure this is OK for all subroutines**
> > I can't find something that clearly forbids it, but I agree it's weird to allow it in all intrinsic subroutines.
> > 
> > Regrading the actual <-> dummy association rules, they are rules that prevents allocatable coindexed  objects to be passed to intent(out)/intent(intout), but nothing regarding coarrays arguments (My understanding is that coarray are not coindexed objects):
> > > 15.5.2.4 .6 If the actual argument is a coindexed object with an allocatable ultimate component, the dummy argument shall have the INTENT (IN) or the VALUE attribute.
> > > 15.5.2.6 If the actual argument is a coindexed object, the dummy argument shall have the INTENT (IN) attribute.
> > 
> > It looks like it's up to each intrinsic definition in the standard to rule what's allowed here. However, I feel like there may be gaps in the "old" subroutines that may not have been updated regarding coarrays. The atomic subroutine all specify that coindexed object are not OK for their STAT argument that is INTENT(OUT). However, no such restriction is given for the STATUS field of GET_COMMAND for instance. That sounds weird to me. I am surprised allocable coarrays are something we would want in SYSTEM_CLOCK arguments, supporting this would bring some implementation complexity here.
> Anything that can change the address of a coarray allocatable on an image implies synchronization, it's pretty heavyweight.  
> 
> Can we allow INTENT(OUT) ALLOCATABLE coarrays only where they are explicitly allowed and necessary (MOVE_ALLOC) and disallow them otherwise?
Sounds like a safer approach. I have modified the patch to only allow this in MOVE_ALLOC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89473/new/

https://reviews.llvm.org/D89473