Janne, Thanks.
Jakub, is this OK with you?
Paul
On 23 January 2018 at 19:09, Janne Blomqvist <blomqvist.ja...@gmail.com> wrote:
> On Mon, Jan 22, 2018 at 9:12 PM, Paul Richard Thomas
> <paul.richard.tho...@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
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
