On Mon, Feb 5, 2018 at 10:53 PM, Thomas Koenig <tkoe...@netcologne.de> wrote: > Am 05.02.2018 um 13:09 schrieb Janne Blomqvist: >> >> 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? > > > 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. -- Janne Blomqvist
