This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.)

I think that if we can "guarantee" that we're happy with the current
ABI for GCC 9 (and hopefully 10, 11, ...?) we have a quite strong case
for committing it now. But if anybody if planning on doing some
ABI-breaking work in the foreseeable future then maybe we should wait
until GCC 9 stage1 opens. Anybody with such plans?

> 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 only potential objection I can come up with is that if we at some
point want to make the F2018 C interoperability descriptor the native
descriptor type, then we have to mash those fields into the parent
descriptor anyway.  But I don't really think this is a very strong
objection, as I don't think it's a huge difference in the amount of
work whichever path is taken. If this is the easiest path for now, so
be it.

> 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.

To be pedantic, F2018 doesn't require that they be signed char, it's
just what I wrote on https://gcc.gnu.org/wiki/ArrayDescriptorUpdate as
*one* potential implementation of ISO_Fortran_binding.h. What the
(draft) F2018 standard says is:

CFI_attribute_t is a typedef name for a standard integer type capable
of representing the values of the attribute codes.

CFI_type_t is a typedef name for a standard integer type capable of
representing the values for the supported type specifiers.

CFI_rank_t is a typedef name for a standard integer type capable of
representing the largest supported rank.

So using int is standard conforming.

(elem_len and version are explicitly specified to be size_t and int,
respectively)

> 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.

Hmm, is this a hidden ABI break, then? Are you saying that even after
this patch we encode the type/kind into the elem_len field? If at some
point we stop doing that and use the type field, it's an ABI break
even though the descriptor layout doesn't change?

(I haven't had time to look into the patch itself, yet. If we decide
to do this for GCC 8 I'll try to find some time.)


-- 
Janne Blomqvist


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]