[Patch] (general Fortran + OpenMP) [Fortran] Fix/modify present() handling for assumed-shape optional (PR 94672)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch] (general Fortran + OpenMP) [Fortran] Fix/modify present() handling for assumed-shape optional (PR 94672)

Tobias Burnus-3
The main purpose of this patch is to fix OpenMP, but it modifies
the general Fortran handling of assumed-shape optional arguments.

For assumed shape, gfortran generates an "arg.0 = arg->data"
artificial variable – and with optional one has something like

if (arg != NULL && arg->data != NULL)
   {
     arg.0 = arg->data;
     lbound.0 = ...
   }

And an "if (present(arg))" becomes
"if (arg != NULL && arg->data != NULL)".

The proposed change changes the init to:

if (arg != NULL && arg->data != NULL)
   {
     arg.0 = arg->data;
     lbound.0 = ...
   }
else
   arg.0 = NULL;  // <-- new

Such that an "if (present(arg))" becomes "if (arg.0 != NULL)".

I think for Fortran code itself, it does not really make any
difference. However, for OpenMP (and OpenACC) it does.

Currently,
   !$omp …
     if (present(arg)) stop 1
   !$omp end …

has decl = "arg.0" and SAVED_DESCRIPTOR(decl) == "arg" such
that inside the omp block everything is "arg.0" – except for
"if (present(arg))" which is converted to the "!arg && !arg->data".

This causes the problems shown in the PR (PR94672).

For optional & 'omp target' where one has to map the variable and
has to check it inside the target function, I even ended up setting
"arg.0 = NULL" explicitly as this was much simpler than adding more
checking in gcc/omp-low.c.


Thus: I think either variant (checking arg directly vs. checking arg.0
plus setting it to NULL) works equally well with normal Fortran code;
one can probably design code where one or the other is slightly faster,
but at the end it should not matter.
And for OpenMP/OpenACC, the new variant avoids several problems.

Hence:
OK for the trunk – and GCC 10 (regression, rejects valid code)?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

fix-omp-optional-v3.diff (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] (general Fortran + OpenMP) [Fortran] Fix/modify present() handling for assumed-shape optional (PR 94672)

gcc - fortran mailing list
Hi Tobias,

> For assumed shape, gfortran generates an "arg.0 = arg->data"
> artificial variable – and with optional one has something like
>
> if (arg != NULL && arg->data != NULL)
>    {
>      arg.0 = arg->data;
>      lbound.0 = ...
>    }
>
> And an "if (present(arg))" becomes
> "if (arg != NULL && arg->data != NULL)".
>
> The proposed change changes the init to:
>
> if (arg != NULL && arg->data != NULL)
>    {
>      arg.0 = arg->data;
>      lbound.0 = ...
>    }
> else
>    arg.0 = NULL;  // <-- new

I think this is a good idea in general. We later use the arg.0
variable to access the contents of the argument, so this generates
a clear segfault for all cases where the user accesses non-present
opiontal arguments instead of something random.

Also, this avoids potential warnings about undefined variables.


> Thus: I think either variant (checking arg directly vs. checking arg.0
> plus setting it to NULL) works equally well with normal Fortran code;

Like I said, I think it is better :-)

> one can probably design code where one or the other is slightly faster,
> but at the end it should not matter.
> And for OpenMP/OpenACC, the new variant avoids several problems.
>
> Hence:
> OK for the trunk – and GCC 10 (regression, rejects valid code)?

OK for both, and thanks for the patch!

Regards

        Thomas