[Patch][OpenMP][Fortran] Support absent optional args with use_device_{ptr,addr} (+ OpenACC's use_device clause)

[Tobias Burnus-3]

This patch is based on Kwok's patch, posted as (4/5) at
https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00964.html – which is
targeting OpenACC's use_device* – but it also applies to OpenMP
use_device_{ptr,addr}.

I added an OpenMP test case. It showed that for arguments with value
attribute and for assumed-shape array, one needs to do more — as the
decl cannot be directly used for the is-argument-present check.

(For 'value', a hidden boolean '_' + arg-name is passed in addition; for
assumed-shape arrays, the array descriptor "x" is replaced by the local
variable "x.0" (with "x.0 = x->data") and the original decl "x" is in
GFC_DECL_SAVED_DESCRIPTOR. Especially for assumed-shape arrays, the new
decl cannot be used unconditionally as it is uninitialized when the
argument is absent.)

Bootstrapped and regtested on x86_64-gnu-linux without offloading + with
nvptx.
OK?

Cheers,

Tobias

*The OpenACC test cases are in 5/5 and depend on some other changes.
Submission of {1,missing one line of 2,3,5}/5 is planned next.
PPS: For fully absent-optional support, mapping needs to be handled for
OpenACC (see Kwok's …/5 patches) and OpenMP (which is quite different on
FE level) – and OpenMP also needs changes for the share clauses.]

[Tobias Burnus-3]

Thinking about the patch over night, I have now updated it a bit:
Namely, I only add the "if(present-check)" condition, if the original
variable is dereferenced. There is no need for code like

   omp_data_arr.c = c == NULL ? NULL : c;

and then, after the libgomp call, code like "c_2 = c == NULL ? NULL :
omp_data_arr.c;"; due to the libgomp call, the latter cannot even be
optimized away.

Hence, I added 'do_optional_check'; additionally, I had a
libgomp.fortran/use_device_ptr-optional-1.f90 change floating around,
which I included. Otherwise unchanged.

Retested. OK for the trunk?

Cheers,

Tobias

On 11/6/19 4:04 PM, Tobias Burnus wrote:

> This patch is based on Kwok's patch, posted as (4/5) at
> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00964.html – which is
> targeting OpenACC's use_device* – but it also applies to OpenMP
> use_device_{ptr,addr}.
>
> I added an OpenMP test case. It showed that for arguments with value
> attribute and for assumed-shape array, one needs to do more — as the
> decl cannot be directly used for the is-argument-present check.
>
> (For 'value', a hidden boolean '_' + arg-name is passed in addition;
> for assumed-shape arrays, the array descriptor "x" is replaced by the
> local variable "x.0" (with "x.0 = x->data") and the original decl "x"
> is in GFC_DECL_SAVED_DESCRIPTOR. Especially for assumed-shape arrays,
> the new decl cannot be used unconditionally as it is uninitialized
> when the argument is absent.)
>
> Bootstrapped and regtested on x86_64-gnu-linux without offloading +
> with nvptx.
> OK?
>
> Cheers,
>
> Tobias
>
> *The OpenACC test cases are in 5/5 and depend on some other changes.
> Submission of {1,missing one line of 2,3,5}/5 is planned next.
> PPS: For fully absent-optional support, mapping needs to be handled
> for OpenACC (see Kwok's …/5 patches) and OpenMP (which is quite
> different on FE level) – and OpenMP also needs changes for the share
> clauses.]
>

[Tobias Burnus-3]

Hi Jakub,

thanks for the review.

On 11/8/19 3:39 PM, Jakub Jelinek wrote:
>> +      /* Walk function argument list to find the hidden arg.  */
>> +      decl = DECL_ARGUMENTS (DECL_CONTEXT (decl));
>> +      for ( ; decl != NULL_TREE; decl = TREE_CHAIN (decl))
>> + if (DECL_NAME (decl) == tree_name)
>> +  break;
> Is this reliable?  I mean, consider -fallow-leading-underscore with:
> subroutine foo (a, _a)

I also assume that this will break; unlikely in real-world code but still.

> Not really sure if additional DECL_ARTIFICIAL (decl) test would be enough.

At least, I cannot quickly come up with a case where it will break. – I
have now added it; also to the existing trans-expr.c function – which
uses the used and, hence, same algorithm.

>> +omp_check_optional_argument (tree decl, bool also_value)
> Why is the argument called for_present_check in the langhook and
> also_value here?  Looks inconsistent.

Because I initially was thinking only of the VALUE attribute until I
realized that assumed-shape arrays have the same issue; they use a local
variable for the data – and make the actual array descriptor available
via lang-specific field. – As the use is either an extra deref is needed
or a check whether the variable is present, I changed the meaning –
seemingly, three places survived with the old name.

With DECL_ARTIFICIAL added and also_value replaced:
Build on x86-64-gnu-linux. OK once regtested?

Tobias