Re: [Patch, Fortran] libgfortran's ISO_Fortran_binding.c: Use GCC11 version for backward-only code [PR108056]

[Tobias Burnus <tobias@codesourcery.com>]

Hi Harald,

On 13.12.22 23:27, Harald Anlauf wrote:
> Am 13.12.22 um 22:41 schrieb Tobias Burnus:
>> Back to differences: 'diff -U0 -p -w' against the last GCC 11 branch
>> shows:
>>
>> ...
>> @@ -35,0 +37,2 @@ export_proto(cfi_desc_to_gfc_desc);
>> +/* NOTE: Since GCC 12, the FE generates code to do the conversion
>> +   directly without calling this function.  */
>> @@ -63 +66 @@ cfi_desc_to_gfc_desc (gfc_array_void *d,
>> -  d->dtype.version = s->version;
>> +  d->dtype.version = 0;
>
> I was wondering what the significance of "version" is.
> In ISO_Fortran_binding.h we seem to always have
>    #define CFI_VERSION 1
> and it did not change with gcc-12.

The version is 1 for CFI but it is 0 for GFC. However, as we do not
check the GFC version anywhere and it is not publicly exposed, it does
not really matter. Still, "d->dtype.version = 0;" matches what the
compiler itself produces – and for consistency, setting it to 0 is
better than setting it to 1 (via CFI's version field).

Actually 'dtype.version' is not really set anywhere; at least
gfc_get_dtype_rank_type(...) does not set it; zero initialization is
most common but it could be also some random value. In libgfortran,
GFC_DTYPE_CLEAR explicitly sets it to 0.
>> @@ -100,2 +110,2 @@ gfc_desc_to_cfi_desc (CFI_cdesc_t **d_pt
>> -    d = malloc (sizeof (CFI_cdesc_t)
>> -               + (CFI_type_t)(CFI_MAX_RANK * sizeof (CFI_dim_t)));
>> +    d = calloc (1, (sizeof (CFI_cdesc_t)
>> +                   + (CFI_type_t)(CFI_MAX_RANK * sizeof (CFI_dim_t))));
>> @@ -107 +117 @@ gfc_desc_to_cfi_desc (CFI_cdesc_t **d_pt
>> -  d->version = s->dtype.version;
>> +  d->version = CFI_VERSION;
>
> This treatment of "version" was the equivalent to the above that
> confused me.  Assuming we were to change CFI_VERSION in gcc-13+,
> is this the right choice here regarding backward compatibility?

I don't think we will change CFI version any time soon as we rather
closely follow the Fortran standard and I do not see any changes which
are required there.

NOTE: As s->dtype.version is either 0 or some random value, setting
version in the CFI / ISO C descriptor to 1, be it as literal or as macro
constant, makes it the same as CFI_VERSION.

And: I don't think we will change CFI_VERSION or the structure of the
CFI array descriptor any time soon; there does not seem to be any need
for it, it matches the Fortran standard one well (and no plans seem to
be planed on that side) and, finally, changing an array descriptor is
painful!

However, using '1;  /* CFI_VERSION in GCC 11 and at time of writing. */'
would also work – but I would expect that we will go through all CFI
users if we ever change the descriptor (and bump the version), possibly
adding version-number dependent code.

> So besides the "version" question ok from my side.

I hope I could answer the latter.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955