Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling

[Tobias Burnus <tobias@codesourcery.com>]

Hi José,

On 21.06.21 17:51, José Rui Faustino de Sousa via Fortran wrote:
> On 21/06/21 13:46, Tobias Burnus wrote:
>>
>> (in principle, I'd like to have the libgfortran function moved to the
>> compiler proper to avoid some issues, but that's admittedly a task
>> independent of your work.)
> cfi_desc_to_gfc_desc and gfc_desc_to_cfi_desc from ISO_c_binding.c,
> right?
Yes.
>
> So, I could look further into that. Were would you like them placed?
Well, as said: directly into the compiler where currently the call to
libgomp is.
>> LGTM – except for one minor nit.

Found a second tiny nit:

+  if (GFC_DESCRIPTOR_DATA (d))
+    for (n = 0; n < GFC_DESCRIPTOR_RANK (d); n++)
+      {
+       CFI_index_t lb = 1;
+
+       if (s->attribute != CFI_attribute_other)

There is tailing whitespace in the otherwise empty line.

>> In trans-expr.c's gfc_conv_gfc_desc_to_cfi_desc:
>>
>>     /* Transfer values back to gfc descriptor.  */
>> +  if (cfi_attribute != 2
>> +      && !fsym->attr.value
>> +      && fsym->attr.intent != INTENT_IN)
>>
>> Can you add after the '2' the string '  /* CFI_attribute_other. */'
>> to make the number less magic.
>
> Yes... I had the same idea... :-) But all those constants are defined
> in "ISO_Fortran_binding.h"... And moving all those definitions would
> be a major change... So I left it as it was...

Well, I am currently only asking to add a comment after the "2;".

This fixing those two nits (removing tailing whitespace + adding a
comment) and is be trivial.

* * *

However, in the long run, I think we should put it into either a
separate file, which is included into ISO_Fortran_binding.h and the
proper compiler (and installed alongside ISO_Fortran_binding.h) - or
just in libgfortran.h and adding some check/(static)assert that it
matches to the value in ISO_Fortran_binding.h.

Or, possibly, we could also include ISO_Fortran_binding.h when building
the compiler itself, possibly adding some '#ifdef' code to disable parts
we do not want when we do #include. it.

(We already have '#include "libgfortran.h"' in gcc/fortran/gfortran.h.)

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf