On Mon, Feb 5, 2018 at 1:09 PM, Janne Blomqvist <blomqvist.ja...@gmail.com> wrote: > On Sun, Feb 4, 2018 at 9:49 PM, Thomas Koenig <tkoe...@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
