This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [gfortran,patch] Fix PACK, SPREAD and CSHIFT for zero-sized arrays
- From: Steven Bosscher <stevenb dot gcc at gmail dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: "FranÃois-Xavier Coudert" <fxcoudert at gmail dot com>, gfortran <fortran at gcc dot gnu dot org>
- Date: Mon, 31 Jul 2006 00:49:16 +0200
- Subject: Re: [gfortran,patch] Fix PACK, SPREAD and CSHIFT for zero-sized arrays
- References: <19c433eb0607260132h5ec0a245n63bd961b49d3ea2b@mail.gmail.com>
:REVIEWMAIL
On Wednesday 26 July 2006 10:32, FranÃois-Xavier Coudert wrote:
> Hi all,
>
> Attached patch introduces special cases in the library helper
> functions of PACK, SPREAD and CSHIFT intrinsics to take care of
> zero-sized arrays. In current state, both three intrinsics just return
> garbage when called with zero-sized argument(s).
>
> From my investigations, after that patch only RESHAPE will still
> require some tweaking; all other transformational intrinsics are
> working fine AFAICT. I've had this patch in my tree for long, but
> didn't post it because I was trying to devise a fix for RESHAPE and
> submit the whole thing; unfortunately, I haven't yet managed to do
> that in more than a month, so it's time to submit this partial patch.
Better partial than nothing.
> Bootstrapped and regtested on i686-linux, comes with a testcase. OK
> for mainline (and 4.1 after some time)
I'd like to suggest some changes:
>
> Index: intrinsics/cshift0.c
> ===================================================================
> --- intrinsics/cshift0.c (revision 115644)
> +++ intrinsics/cshift0.c (working copy)
> @@ -144,8 +144,8 @@
> if (ret->data == NULL)
> {
> int i;
> + index_type arraysize = size0 ((array_t *)array);
>
> - ret->data = internal_malloc_size (size * size0 ((array_t *)array));
> ret->offset = 0;
> ret->dtype = array->dtype;
> for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
> @@ -156,8 +156,17 @@
> if (i == 0)
> ret->dim[i].stride = 1;
> else
> - ret->dim[i].stride = (ret->dim[i-1].ubound + 1) * ret->dim[i-1].stride;
> + ret->dim[i].stride = (ret->dim[i-1].ubound + 1)
> + * ret->dim[i-1].stride;
> }
> +
> + if (arraysize > 0)
> + ret->data = internal_malloc_size (size * arraysize);
> + else
> + {
> + ret->data = internal_malloc_size (1);
> + return;
> + }
> }
>
> for (dim = 0; dim < GFC_DESCRIPTOR_RANK (array); dim++)
This whole hunk could be implemented as the easier-to-understand:
Index: intrinsics/cshift0.c
===================================================================
--- intrinsics/cshift0.c (revision 115817)
+++ intrinsics/cshift0.c (working copy)
@@ -144,8 +144,14 @@ cshift0 (gfc_array_char * ret, const gfc
if (ret->data == NULL)
{
int i;
+ index_type arraysize = size0 ((array_t *)array);
- ret->data = internal_malloc_size (size * size0 ((array_t *)array));
+ /* For zero-sized arrays, we still want to allocate memory for
+ the DATA field of the descriptor. */
+ if (arraysize == 0)
+ arraysize = 1;
+
+ ret->data = internal_malloc_size (size * arraysize);
ret->offset = 0;
ret->dtype = array->dtype;
for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
Likewise for the other ones.
I like it how you break that long line, btw. :-)
Gr.
Steven