[libfortran] Use flexible array members for array descriptor

Hello world,

in the attached patch, I have used flexible array members for
using the different descriptor types (following Richi's advice).
This does not change the binary ABI, but the library code
maches what we are actually doing in the front end.  I have
not yet given up hope of enabling LTO for the library :-)
and this, I think, will be a prerequisite.

OK for trunk?

Regards

	Thomas

2018-02-04  Thomas Koenig  <tkoenig@gcc.gnu.org>

	* libgfortran.h (GFC_ARRAY_DESCRIPTOR): Remove dimension
	of descriptor to use vaiable members for dim.
	Change usage of GFC_ARRAY_DESCRIPTOR accordingly.
	(GFC_FILL_ARRAY_DESCRIPTOR): New macro.
	(gfc_full_array_i4): New type.
	* intrinsics/date_and_time.c (secnds): Use sizeof
	(gfc_full_array_i4) for memory allocation.
	* intrinsics/reshape_generic.c: Use GFC_FULL_ARRAY_DESCRIPTOR.
	* io/format.c: Use sizeof (gfc_full_array_i4) for memoy
	allocation.
	* io/list_read.c (list_formatted_read_scalar): Use
	gfc_full_array_i4 for variable.
	(nml_read_obj): Likewise.
	* io/write.c (list_formatted_write_scalar): Likewise.
	(nml_write_obj): Likewise.
	* m4/reshape.m4: Use GFC_FULL_ARRAY_DESCRIPTOR.
	* generated/reshape_c10.c: Regenerated.
	* generated/reshape_c16.c: Regenerated.
	* generated/reshape_c4.c: Regenerated.
	* generated/reshape_c8.c: Regenerated.
	* generated/reshape_i16.c: Regenerated.
	* generated/reshape_i4.c: Regenerated.
	* generated/reshape_i8.c: Regenerated.
	* generated/reshape_r10.c: Regenerated.
	* generated/reshape_r16.c: Regenerated.
	* generated/reshape_r4.c: Regenerated.
	* generated/reshape_r8.c: Regenerated.

On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hello world,
>
> in the attached patch, I have used flexible array members for
> using the different descriptor types (following Richi's advice).
> This does not change the binary ABI, but the library code
> maches what we are actually doing in the front end.  I have
> not yet given up hope of enabling LTO for the library :-)
> and this, I think, will be a prerequisite.
>
> OK for trunk?

Given that Jakub and Richi apparently weren't yet unanimous in their
recommendations on the best path forward, maybe wait a bit for the
smoke to clear?

In the meantime, a few comments:

1) Is there some particular benefit to all those macroized
descriptors, given that the only thing different is the type of the
base_addr pointer? Wouldn't it be simpler to just have a single
descriptor type with base_addr a void pointer, then typecast that
pointer to whatever type is needed?

2)

Index: intrinsics/date_and_time.c
===================================================================
--- intrinsics/date_and_time.c (Revision 257347)
+++ intrinsics/date_and_time.c (Arbeitskopie)
@@ -268,7 +268,7 @@ secnds (GFC_REAL_4 *x)
   GFC_REAL_4 temp1, temp2;

   /* Make the INTEGER*4 array for passing to date_and_time.  */
-  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
+  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));


Since date_and_time requires the values array to always be rank 1,
can't this be "xmalloc (sizeof (gfc_array_i4) +
sizeof(dimension_data))" ?

(The GFC_FULL_DESCRIPTOR stuff is useful for stack allocated
descriptors to avoid VLA's / alloca(), but for heap allocated ones we
can allocate only the needed size, I think)


3)

Index: io/format.c
===================================================================
--- io/format.c (Revision 257347)
+++ io/format.c (Arbeitskopie)
@@ -1025,7 +1025,7 @@ parse_format_list (st_parameter_dt *dtp, bool *see
       t = format_lex (fmt);

       /* Initialize the vlist to a zero size array.  */
-      tail->u.udf.vlist= xmalloc (sizeof(gfc_array_i4));
+      tail->u.udf.vlist= xmalloc (sizeof(gfc_full_array_i4));
       GFC_DESCRIPTOR_DATA(tail->u.udf.vlist) = NULL;
       GFC_DIMENSION_SET(tail->u.udf.vlist->dim[0],1, 0, 0);


And same here?

Am 05.02.2018 um 13:09 schrieb Janne Blomqvist:
> On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
>> Hello world,
>>
>> in the attached patch, I have used flexible array members for
>> using the different descriptor types (following Richi's advice).
>> This does not change the binary ABI, but the library code
>> maches what we are actually doing in the front end.  I have
>> not yet given up hope of enabling LTO for the library :-)
>> and this, I think, will be a prerequisite.
>>
>> OK for trunk?
> 
> Given that Jakub and Richi apparently weren't yet unanimous in their
> recommendations on the best path forward, maybe wait a bit for the
> smoke to clear?

Make sense.  Depending on what the solution is, this (or a variant)
will probably part of the patch.

> In the meantime, a few comments:
> 
> 1) Is there some particular benefit to all those macroized
> descriptors, given that the only thing different is the type of the
> base_addr pointer? Wouldn't it be simpler to just have a single
> descriptor type with base_addr a void pointer, then typecast that
> pointer to whatever type is needed?

I don't particulary like going through void* pointers - it is
clearer to leave an int* as an int*.

> 2)
> 
> Index: intrinsics/date_and_time.c
> ===================================================================
> --- intrinsics/date_and_time.c (Revision 257347)
> +++ intrinsics/date_and_time.c (Arbeitskopie)
> @@ -268,7 +268,7 @@ secnds (GFC_REAL_4 *x)
>     GFC_REAL_4 temp1, temp2;
> 
>     /* Make the INTEGER*4 array for passing to date_and_time.  */
> -  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
> +  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));
> 
> 
> Since date_and_time requires the values array to always be rank 1,
> can't this be "xmalloc (sizeof (gfc_array_i4) +
> sizeof(dimension_data))" ?

I think we can be fairly sure that this would be OK at the moment, since
(I think) there are no gaps in our descriptors at the moment. Anybody
invents an architecture where this is not the case, we introduce
a bug. This way is safer.

Regards

	Thomas

On Mon, Feb 5, 2018 at 10:53 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Am 05.02.2018 um 13:09 schrieb Janne Blomqvist:
>>
>> On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig <tkoenig@netcologne.de>
>> wrote:
>>>
>>> Hello world,
>>>
>>> in the attached patch, I have used flexible array members for
>>> using the different descriptor types (following Richi's advice).
>>> This does not change the binary ABI, but the library code
>>> maches what we are actually doing in the front end.  I have
>>> not yet given up hope of enabling LTO for the library :-)
>>> and this, I think, will be a prerequisite.
>>>
>>> OK for trunk?
>>
>>
>> Given that Jakub and Richi apparently weren't yet unanimous in their
>> recommendations on the best path forward, maybe wait a bit for the
>> smoke to clear?
>
>
> Make sense.  Depending on what the solution is, this (or a variant)
> will probably part of the patch.
>
>> In the meantime, a few comments:
>>
>> 1) Is there some particular benefit to all those macroized
>> descriptors, given that the only thing different is the type of the
>> base_addr pointer? Wouldn't it be simpler to just have a single
>> descriptor type with base_addr a void pointer, then typecast that
>> pointer to whatever type is needed?
>
>
> I don't particulary like going through void* pointers - it is
> clearer to leave an int* as an int*.

Ok. Well, I disagree, but not vehemently, so it's fine as it is if you think so.

>> 2)
>>
>> Index: intrinsics/date_and_time.c
>> ===================================================================
>> --- intrinsics/date_and_time.c (Revision 257347)
>> +++ intrinsics/date_and_time.c (Arbeitskopie)
>> @@ -268,7 +268,7 @@ secnds (GFC_REAL_4 *x)
>>     GFC_REAL_4 temp1, temp2;
>>
>>     /* Make the INTEGER*4 array for passing to date_and_time.  */
>> -  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
>> +  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));
>>
>>
>> Since date_and_time requires the values array to always be rank 1,
>> can't this be "xmalloc (sizeof (gfc_array_i4) +
>> sizeof(dimension_data))" ?
>
>
> I think we can be fairly sure that this would be OK at the moment, since
> (I think) there are no gaps in our descriptors at the moment. Anybody
> invents an architecture where this is not the case, we introduce
> a bug. This way is safer.

According to the C standard (C11 6.7.2.1.18 and example 6.7.2.1.20),
this is guaranteed to work. That is, sizeof() of a struct with a
flexible array member must include whatever padding is necessary so
that the flexible array member can be naturally aligned (or well, I
guess "must" only if the target doesn't handle misaligned accesses,
otherwise it's a QoI issue). In any case, if this does not work
properly for some target, a bug against the C frontend is in order.

Am 06.02.2018 um 09:53 schrieb Janne Blomqvist:
>>>      /* Make the INTEGER*4 array for passing to date_and_time.  */
>>> -  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
>>> +  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));
>>>
>>>
>>> Since date_and_time requires the values array to always be rank 1,
>>> can't this be "xmalloc (sizeof (gfc_array_i4) +
>>> sizeof(dimension_data))" ?
>>
>> I think we can be fairly sure that this would be OK at the moment, since
>> (I think) there are no gaps in our descriptors at the moment. Anybody
>> invents an architecture where this is not the case, we introduce
>> a bug. This way is safer.
> According to the C standard (C11 6.7.2.1.18 and example 6.7.2.1.20),
> this is guaranteed to work.

OK, I didn't know that, and I see how that would save memory.
I'll make that change (with a comment) if this patch ends up being
committed.

Regards

	Thomas

On Mon, Feb 5, 2018 at 1:09 PM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
>> Hello world,
>>
>> in the attached patch, I have used flexible array members for
>> using the different descriptor types (following Richi's advice).
>> This does not change the binary ABI, but the library code
>> maches what we are actually doing in the front end.  I have
>> not yet given up hope of enabling LTO for the library :-)
>> and this, I think, will be a prerequisite.
>>
>> OK for trunk?
>
> Given that Jakub and Richi apparently weren't yet unanimous in their
> recommendations on the best path forward, maybe wait a bit for the
> smoke to clear?

If the effect of the patch is (it doesn't include generated files) that the
function arguments now have pointers to array descriptor types with
the flexible array then yes, that's what will be needed anyways, no need
for any dust to settle here.

The other part would then be to change the FE declarations of the
intrinsic functions as they will now mismatch as well (and it still
generates multiple ones).  So making the IL emitted by the
FE also call a function with the flexible array descriptor pointer type
is the second step to fix the LTO ODR warning.

Then the warning is gone but the alias issue still exists which means
the FE has to do all actual accesses to any array descriptor via
the flex array variant type (or using its alias set).

Richard.

> In the meantime, a few comments:
>
> 1) Is there some particular benefit to all those macroized
> descriptors, given that the only thing different is the type of the
> base_addr pointer? Wouldn't it be simpler to just have a single
> descriptor type with base_addr a void pointer, then typecast that
> pointer to whatever type is needed?
>
> 2)
>
> Index: intrinsics/date_and_time.c
> ===================================================================
> --- intrinsics/date_and_time.c (Revision 257347)
> +++ intrinsics/date_and_time.c (Arbeitskopie)
> @@ -268,7 +268,7 @@ secnds (GFC_REAL_4 *x)
>    GFC_REAL_4 temp1, temp2;
>
>    /* Make the INTEGER*4 array for passing to date_and_time.  */
> -  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
> +  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_full_array_i4));
>
>
> Since date_and_time requires the values array to always be rank 1,
> can't this be "xmalloc (sizeof (gfc_array_i4) +
> sizeof(dimension_data))" ?
>
> (The GFC_FULL_DESCRIPTOR stuff is useful for stack allocated
> descriptors to avoid VLA's / alloca(), but for heap allocated ones we
> can allocate only the needed size, I think)
>
>
> 3)
>
> Index: io/format.c
> ===================================================================
> --- io/format.c (Revision 257347)
> +++ io/format.c (Arbeitskopie)
> @@ -1025,7 +1025,7 @@ parse_format_list (st_parameter_dt *dtp, bool *see
>        t = format_lex (fmt);
>
>        /* Initialize the vlist to a zero size array.  */
> -      tail->u.udf.vlist= xmalloc (sizeof(gfc_array_i4));
> +      tail->u.udf.vlist= xmalloc (sizeof(gfc_full_array_i4));
>        GFC_DESCRIPTOR_DATA(tail->u.udf.vlist) = NULL;
>        GFC_DIMENSION_SET(tail->u.udf.vlist->dim[0],1, 0, 0);
>
>
> And same here?
>
>
>
> --
> Janne Blomqvist

Am 08.02.2018 um 12:27 schrieb Richard Biener:
> If the effect of the patch is (it doesn't include generated files) that the
> function arguments now have pointers to array descriptor types with
> the flexible array then yes, that's what will be needed anyways, no need
> for any dust to settle here.

All right.

I have attached the patch updated with Janne's comments.

Regression-tested. OK for trunk then?

Regards

	Thomas

2018-02-12  Thomas Koenig  <tkoenig@gcc.gnu.org>

	* libgfortran.h (GFC_ARRAY_DESCRIPTOR): Remove dimension
	of descriptor to use vaiable members for dim.
	Change usage of GFC_ARRAY_DESCRIPTOR accordingly.
	(GFC_FILL_ARRAY_DESCRIPTOR): New macro.
	(gfc_full_array_i4): New type.
	* intrinsics/date_and_time.c (secnds): Use sizeof
	(gfc_array_i4) + sizeof (descriptor_dimension) for memory
	allocation.
	* intrinsics/reshape_generic.c: Use GFC_FULL_ARRAY_DESCRIPTOR.
	* io/format.c: Use sizeof (gfc_array_i4) + sizeof
	(descriptor_dimension) for memoy allocation.
	* io/list_read.c (list_formatted_read_scalar): Use
	gfc_full_array_i4 for variable.
	(nml_read_obj): Likewise.
	* io/write.c (list_formatted_write_scalar): Likewise.
	(nml_write_obj): Likewise.
	* m4/reshape.m4: Use GFC_FULL_ARRAY_DESCRIPTOR.
	* generated/reshape_c10.c: Regenerated.
	* generated/reshape_c16.c: Regenerated.
	* generated/reshape_c4.c: Regenerated.
	* generated/reshape_c8.c: Regenerated.
	* generated/reshape_i16.c: Regenerated.
	* generated/reshape_i4.c: Regenerated.
	* generated/reshape_i8.c: Regenerated.
	* generated/reshape_r10.c: Regenerated.
	* generated/reshape_r16.c: Regenerated.
	* generated/reshape_r4.c: Regenerated.
	* generated/reshape_r8.c: Regenerated.
Index: generated/reshape_c10.c
===================================================================
--- generated/reshape_c10.c	(Revision 257347)
+++ generated/reshape_c10.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_COMPLEX_10)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_c10 (gfc_array_c10 * const restrict, 
Index: generated/reshape_c16.c
===================================================================
--- generated/reshape_c16.c	(Revision 257347)
+++ generated/reshape_c16.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_COMPLEX_16)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_c16 (gfc_array_c16 * const restrict, 
Index: generated/reshape_c4.c
===================================================================
--- generated/reshape_c4.c	(Revision 257347)
+++ generated/reshape_c4.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_COMPLEX_4)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_c4 (gfc_array_c4 * const restrict, 
Index: generated/reshape_c8.c
===================================================================
--- generated/reshape_c8.c	(Revision 257347)
+++ generated/reshape_c8.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_COMPLEX_8)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_c8 (gfc_array_c8 * const restrict, 
Index: generated/reshape_i16.c
===================================================================
--- generated/reshape_i16.c	(Revision 257347)
+++ generated/reshape_i16.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_INTEGER_16)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_16 (gfc_array_i16 * const restrict, 
Index: generated/reshape_i4.c
===================================================================
--- generated/reshape_i4.c	(Revision 257347)
+++ generated/reshape_i4.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_INTEGER_4)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_4 (gfc_array_i4 * const restrict, 
Index: generated/reshape_i8.c
===================================================================
--- generated/reshape_i8.c	(Revision 257347)
+++ generated/reshape_i8.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_INTEGER_8)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_8 (gfc_array_i8 * const restrict, 
Index: generated/reshape_r10.c
===================================================================
--- generated/reshape_r10.c	(Revision 257347)
+++ generated/reshape_r10.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_REAL_10)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_r10 (gfc_array_r10 * const restrict, 
Index: generated/reshape_r16.c
===================================================================
--- generated/reshape_r16.c	(Revision 257347)
+++ generated/reshape_r16.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_REAL_16)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_r16 (gfc_array_r16 * const restrict, 
Index: generated/reshape_r4.c
===================================================================
--- generated/reshape_r4.c	(Revision 257347)
+++ generated/reshape_r4.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_REAL_4)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_r4 (gfc_array_r4 * const restrict, 
Index: generated/reshape_r8.c
===================================================================
--- generated/reshape_r8.c	(Revision 257347)
+++ generated/reshape_r8.c	(Arbeitskopie)
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #if defined (HAVE_GFC_REAL_8)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
 
 
 extern void reshape_r8 (gfc_array_r8 * const restrict, 
Index: intrinsics/date_and_time.c
===================================================================
--- intrinsics/date_and_time.c	(Revision 257347)
+++ intrinsics/date_and_time.c	(Arbeitskopie)
@@ -267,8 +267,10 @@ secnds (GFC_REAL_4 *x)
   GFC_INTEGER_4 values[VALUES_SIZE];
   GFC_REAL_4 temp1, temp2;
 
-  /* Make the INTEGER*4 array for passing to date_and_time.  */
-  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4));
+  /* Make the INTEGER*4 array for passing to date_and_time, with enough space
+   for a rank-one array.  */
+  gfc_array_i4 *avalues = xmalloc (sizeof (gfc_array_i4)
+				   + sizeof (descriptor_dimension));
   avalues->base_addr = &values[0];
   GFC_DESCRIPTOR_DTYPE (avalues).type = BT_REAL;
   GFC_DESCRIPTOR_DTYPE (avalues).elem_len = 4;
Index: intrinsics/reshape_generic.c
===================================================================
--- intrinsics/reshape_generic.c	(Revision 257347)
+++ intrinsics/reshape_generic.c	(Arbeitskopie)
@@ -26,8 +26,8 @@ see the files COPYING3 and COPYING.RUNTIME respect
 #include "libgfortran.h"
 #include <string.h>
 
-typedef GFC_ARRAY_DESCRIPTOR(1, index_type) shape_type;
-typedef GFC_ARRAY_DESCRIPTOR(GFC_MAX_DIMENSIONS, char) parray;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, index_type) shape_type;
+typedef GFC_FULL_ARRAY_DESCRIPTOR(GFC_MAX_DIMENSIONS, char) parray;
 
 static void
 reshape_internal (parray *ret, parray *source, shape_type *shape,
Index: io/format.c
===================================================================
--- io/format.c	(Revision 257347)
+++ io/format.c	(Arbeitskopie)
@@ -1024,8 +1024,9 @@ parse_format_list (st_parameter_dt *dtp, bool *see
 
       t = format_lex (fmt);
 
-      /* Initialize the vlist to a zero size array.  */
-      tail->u.udf.vlist= xmalloc (sizeof(gfc_array_i4));
+      /* Initialize the vlist to a zero size, rank-one array.  */
+      tail->u.udf.vlist= xmalloc (sizeof(gfc_array_i4)
+				  + sizeof (descriptor_dimension));
       GFC_DESCRIPTOR_DATA(tail->u.udf.vlist) = NULL;
       GFC_DIMENSION_SET(tail->u.udf.vlist->dim[0],1, 0, 0);
 
Index: io/list_read.c
===================================================================
--- io/list_read.c	(Revision 257347)
+++ io/list_read.c	(Arbeitskopie)
@@ -2198,7 +2198,7 @@ list_formatted_read_scalar (st_parameter_dt *dtp,
 	  gfc_charlen_type child_iomsg_len;
 	  int noiostat;
 	  int *child_iostat = NULL;
-	  gfc_array_i4 vlist;
+	  gfc_full_array_i4 vlist;
 
 	  GFC_DESCRIPTOR_DATA(&vlist) = NULL;
 	  GFC_DIMENSION_SET(vlist.dim[0],1, 0, 0);
@@ -2996,7 +2996,7 @@ nml_read_obj (st_parameter_dt *dtp, namelist_info
 		gfc_charlen_type child_iomsg_len;
 		int noiostat;
 		int *child_iostat = NULL;
-		gfc_array_i4 vlist;
+		gfc_full_array_i4 vlist;
 		formatted_dtio dtio_ptr = (formatted_dtio)nl->dtio_sub;
 
 		GFC_DESCRIPTOR_DATA(&vlist) = NULL;
Index: io/write.c
===================================================================
--- io/write.c	(Revision 257347)
+++ io/write.c	(Arbeitskopie)
@@ -1886,7 +1886,7 @@ list_formatted_write_scalar (st_parameter_dt *dtp,
 	  gfc_charlen_type child_iomsg_len;
 	  int noiostat;
 	  int *child_iostat = NULL;
-	  gfc_array_i4 vlist;
+	  gfc_full_array_i4 vlist;
 
 	  GFC_DESCRIPTOR_DATA(&vlist) = NULL;
 	  GFC_DIMENSION_SET(vlist.dim[0],1, 0, 0);
@@ -2211,7 +2211,7 @@ nml_write_obj (st_parameter_dt *dtp, namelist_info
 		  gfc_charlen_type child_iomsg_len;
 		  int noiostat;
 		  int *child_iostat = NULL;
-		  gfc_array_i4 vlist;
+		  gfc_full_array_i4 vlist;
 		  formatted_dtio dtio_ptr = (formatted_dtio)obj->dtio_sub;
 
 		  GFC_DIMENSION_SET(vlist.dim[0],1, 0, 0);
Index: libgfortran.h
===================================================================
--- libgfortran.h	(Revision 257347)
+++ libgfortran.h	(Arbeitskopie)
@@ -339,51 +339,65 @@ typedef struct dtype_type
 }
 dtype_type;
 
-#define GFC_ARRAY_DESCRIPTOR(r, type) \
+#define GFC_ARRAY_DESCRIPTOR(type) \
 struct {\
   type *base_addr;\
   size_t offset;\
   dtype_type dtype;\
   index_type span;\
-  descriptor_dimension dim[r];\
+  descriptor_dimension dim[];\
 }
 
 /* Commonly used array descriptor types.  */
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, void) gfc_array_void;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, char) gfc_array_char;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_1) gfc_array_i1;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_2) gfc_array_i2;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_4) gfc_array_i4;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_8) gfc_array_i8;
+typedef GFC_ARRAY_DESCRIPTOR (void) gfc_array_void;
+typedef GFC_ARRAY_DESCRIPTOR (char) gfc_array_char;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_1) gfc_array_i1;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_2) gfc_array_i2;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_4) gfc_array_i4;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_8) gfc_array_i8;
 #ifdef HAVE_GFC_INTEGER_16
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_16) gfc_array_i16;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_16) gfc_array_i16;
 #endif
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_REAL_4) gfc_array_r4;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_REAL_8) gfc_array_r8;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_REAL_4) gfc_array_r4;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_REAL_8) gfc_array_r8;
 #ifdef HAVE_GFC_REAL_10
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_REAL_10) gfc_array_r10;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_REAL_10) gfc_array_r10;
 #endif
 #ifdef HAVE_GFC_REAL_16
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_REAL_16) gfc_array_r16;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_REAL_16) gfc_array_r16;
 #endif
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_COMPLEX_4) gfc_array_c4;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_COMPLEX_8) gfc_array_c8;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_COMPLEX_4) gfc_array_c4;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_COMPLEX_8) gfc_array_c8;
 #ifdef HAVE_GFC_COMPLEX_10
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_COMPLEX_10) gfc_array_c10;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_COMPLEX_10) gfc_array_c10;
 #endif
 #ifdef HAVE_GFC_COMPLEX_16
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_COMPLEX_16) gfc_array_c16;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_COMPLEX_16) gfc_array_c16;
 #endif
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_LOGICAL_1) gfc_array_l1;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_LOGICAL_2) gfc_array_l2;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_LOGICAL_4) gfc_array_l4;
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_LOGICAL_8) gfc_array_l8;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_LOGICAL_1) gfc_array_l1;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_LOGICAL_2) gfc_array_l2;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_LOGICAL_4) gfc_array_l4;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_LOGICAL_8) gfc_array_l8;
 #ifdef HAVE_GFC_LOGICAL_16
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_LOGICAL_16) gfc_array_l16;
+typedef GFC_ARRAY_DESCRIPTOR (GFC_LOGICAL_16) gfc_array_l16;
 #endif
 typedef gfc_array_i1 gfc_array_s1;
 typedef gfc_array_i4 gfc_array_s4;
 
+/* These are for when you actually want to declare a descriptor, as
+   opposed to a pointer to it.  */
+
+#define GFC_FULL_ARRAY_DESCRIPTOR(r, type) \
+struct {\
+  type *base_addr;\
+  size_t offset;\
+  dtype_type dtype;\
+  index_type span;\
+  descriptor_dimension dim[r];\
+}
+
+typedef GFC_FULL_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, GFC_INTEGER_4) gfc_full_array_i4;
+
 #define GFC_DESCRIPTOR_RANK(desc) ((desc)->dtype.rank)
 #define GFC_DESCRIPTOR_TYPE(desc) ((desc)->dtype.type)
 #define GFC_DESCRIPTOR_SIZE(desc) ((desc)->dtype.elem_len)
@@ -1345,7 +1359,7 @@ iexport_proto(random_seed_i8);
 
 /* size.c */
 
-typedef GFC_ARRAY_DESCRIPTOR (GFC_MAX_DIMENSIONS, void) array_t;
+typedef GFC_ARRAY_DESCRIPTOR (void) array_t;
 
 extern index_type size0 (const array_t * array); 
 iexport_proto(size0);
Index: m4/reshape.m4
===================================================================
--- m4/reshape.m4	(Revision 257347)
+++ m4/reshape.m4	(Arbeitskopie)
@@ -29,7 +29,7 @@ include(iparm.m4)dnl
 
 `#if defined (HAVE_'rtype_name`)
 
-typedef GFC_ARRAY_DESCRIPTOR(1, 'index_type`) 'shape_type`;'
+typedef GFC_FULL_ARRAY_DESCRIPTOR(1, 'index_type`) 'shape_type`;'
 
 dnl For integer routines, only the kind (ie size) is used to name the
 dnl function.  The same function will be used for integer and logical

On Mon, Feb 12, 2018 at 9:41 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Am 08.02.2018 um 12:27 schrieb Richard Biener:
>>
>> If the effect of the patch is (it doesn't include generated files) that
>> the
>> function arguments now have pointers to array descriptor types with
>> the flexible array then yes, that's what will be needed anyways, no need
>> for any dust to settle here.
>
>
> All right.
>
> I have attached the patch updated with Janne's comments.
>
> Regression-tested. OK for trunk then?

Ok, thanks.

On Mon, Feb 12, 2018 at 08:41:56PM +0100, Thomas Koenig wrote:
> Am 08.02.2018 um 12:27 schrieb Richard Biener:
> > If the effect of the patch is (it doesn't include generated files) that the
> > function arguments now have pointers to array descriptor types with
> > the flexible array then yes, that's what will be needed anyways, no need
> > for any dust to settle here.
> 
> All right.
> 
> I have attached the patch updated with Janne's comments.
> 
> Regression-tested. OK for trunk then?

For the library part, you could have just used a single
GFC_ARRAY_DESCRIPTOR macro, just use
GFC_ARRAY_DESCRIPTOR (, GFC_INTEGER_1) etc. when you want the flexible
array member.

The compiler side will be harder, wonder if we want to always use
as the TREE_TYPE of things what we use right now and have somewhere
in the lang_specific part a pointer to a type with flexible array member
and whenever trying to dereference something build a MEM_REF with the
flexible array member type; plus whenever passing it to some other function.
Or if we have some easy way to find out what objects will need local
variables with descriptors (those need the non-flexible array member stuff)
and others (e.g. dummy arguments etc.) where we could use just the flexible
array members.

	Jakub

Hi Jakub,

> Or if we have some easy way to find out what objects will need local
> variables with descriptors (those need the non-flexible array member stuff)
> and others (e.g. dummy arguments etc.) where we could use just the flexible
> array members.

Descriptors are used for passing arguments to procedures, for
allocatable variables and for pointers. Pointers and allocatable
variables can also be passed to procedures.

So, not so easy (unfortunately).

Regards

	Thomas