Re: [Patch, fortran] PR37577 - [meta-bug] change internal array descriptor format for better syntax, C interop TR, rank 15

[Janne Blomqvist <blomqvist dot janne at gmail dot com>]

On Mon, Jan 22, 2018 at 9:12 PM, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> This patch has been triggered by Thomas's recent message to the list.
> Not only did I start work late relative to stage 3 but debugging took
> somewhat longer than anticipated. Therefore, to get this committed
> asap, we will have to beg the indulgence of the release managers and
> prompt review and/or testing by fortran maintainers. (Dominique has
> already undertaken to test -m32.)
>
> The patch delivers rank up to 15 for F2008, the descriptor information
> needed to enact the F2018 C descriptor macros and an attribute field
> to store such information as pointer/allocatable, contiguous etc..
> Only the first has been enabled so far but it was necessary to submit
> the array descriptor changes now to avoid any further ABI breakage in
> 9.0.0.
>
> I took the design choice choice to replace the dtype with a structure:
> typedef struct dtype_type
> {
>   size_t elem_len;
>   int version;
>   int rank;
>   int type;
>   int attribute;
> }
> dtype_type;
>
> This choice was intended to reduce the changes to a minimum, since in
> most references to the dtype, one dtype is assigned to another.
>
> The F2018 interop defines the 'type and 'attribute fields to be signed
> char types. I used this intially but found that using int was the best
> way to silence the warnings about padding since it also allows for
> more attribute information to be carried.
>
> Some parts of the patch (eg. in get_scalar_to_descriptor_type) look as
> if latent bugs were uncovered by the change to the descriptor. If so,
> the time spent debugging was well worthwhile.
>
> It should be noted that some of the intrinsics, which use switch/case
> for the type/kind selection, limit the effective element size that
> they handle to the maximum value of size_t, less 7 bits. A bit of
> straightforward work there would fix this limitation and would allow
> the GFC_DTYPE shifts and masks to be eliminated.
>
> Bootstraps and regtests on FC23/x86_64 - OK for trunk?

In trans-types.c:

structure dtype_type dtype;

Should be "struct dtype_type dtype". (This is in a comment, so doesn't
affect the code, but still).

I have to say, the patch is much smaller and less invasive than I had
feared for such a fundamental change. I guess you were right about
making dtype it's own type.

As for the DTYPE shifting and masking thing, now that I have read the
patch, if I understood it correctly, that's an internal issue in
libgfortran and has no effect on the descriptor.  That being said, the
F2018 C descriptor has both the type and kind encoded into the type
field, see table 18.4 in F2018 N2146. (In a way that's redundant,
since the type and the elem_len fields suffice to figure out the
type&kind combination). Anyway, is there a case for following suit,
and if so, is it a too invasive change at this point?

In any case, since we seem to agree that we have no big lingering
issues that would require us to break the ABI again for GCC 9, IMHO
this is Ok for trunk. You might want to get an explicit Ok from the
release manager, though.

-- 
Janne Blomqvist