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