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

[Harald Anlauf <anlauf@gmx.de>]

Hi Tobias,

Am 13.12.22 um 22:41 schrieb Tobias Burnus:
> Note: We only talk about those two functions, the other functions are used
> by both GCC <= 11 and GCC >= 12.

yes.

> Fortunately, these functions matter most as they map GFC internals to CFI
> internals or vice versa. Most other functions are user callable and there
> incompatibilities are less likely to show up and GCC 11 users also could
> profit from fixes there. It looks as if CFI_section and CFI_select_part had
> some larger changes, likewise CFI_setpointer.
> 
> 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.

If it is just decoration (for now), then it is not important.
I just didn't see where it is used.

> @@ -76,0 +80 @@ cfi_desc_to_gfc_desc (gfc_array_void *d,
> +  if (GFC_DESCRIPTOR_DATA (d))
> @@ -79,3 +83,7 @@ cfi_desc_to_gfc_desc (gfc_array_void *d,
> -      GFC_DESCRIPTOR_LBOUND(d, n) = (index_type)s->dim[n].lower_bound;
> -      GFC_DESCRIPTOR_UBOUND(d, n) = (index_type)(s->dim[n].extent
> -                                               + s->dim[n].lower_bound 
> - 1);
> +       CFI_index_t lb = 1;
> +
> +       if (s->attribute != CFI_attribute_other)
> +         lb = s->dim[n].lower_bound;
> +
> +       GFC_DESCRIPTOR_LBOUND(d, n) = (index_type)lb;
> +       GFC_DESCRIPTOR_UBOUND(d, n) = (index_type)(s->dim[n].extent + lb 
> - 1);

Oh, now I see that on 11-branch in trans-expr.c we set a hard-coded
attribute = 2 instead of using CFI_attribute_other, even if that was
available as a macro defining the very same value.  Thus it is ok.

> @@ -89,0 +98,2 @@ export_proto(gfc_desc_to_cfi_desc);
> +/* NOTE: Since GCC 12, the FE generates code to do the conversion
> +   directly without calling this function.  */
> @@ -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?

> Probably yes. I don't have a better suggestion. The problem is that it
> usually only matters in some corner cases, like in the PR where a not
> some argument is passed to the GFC→CFI conversion but first a Fortran
> function is called with TYPE(*) any only then it is passed on. – Such
> cases are usually not in the testsuite. (With GCC 12 we have a rather
> complex testsuite, but obviously it also does not cover everything.)

Well, I understand we have to draw a line here, whether we
reproduce every bug in <= gcc-11 where the user may have
attempted to implement a workaround.  That might be tough.

>> Well, in the real world there are larger installations with large
>> software stacks, and it is easier said to "compile each component
>> with the same compiler version" than done...
> 
> I concur – but there were really many fixes for the array descriptor /
> TS29113 in GCC 12.
> 
> It is simply not possible to fix tons of bugs and be 100% compatible
> with the working bits of the old version – especially if they only work
> if one does not look sharply at the result. (Like here, were 'type' is
> wrong, which does not matter unless in programs which use them.)

True.  I was only thinking of the 90+ percent of valid and common
uses that we really consider important.

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

Thanks,
Harald

> Thanks,
> 
> 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
>