This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gfortran,patch] Fix PACK, SPREAD and CSHIFT for zero-sized arrays


: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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]