Patchwork [ggc] fix ggc_alloc_rtvec_resized

login
register
mail settings
Submitter Liang Wang
Date Aug. 16, 2011, 9:35 a.m.
Message ID <CAO+NnCZE-X52ynUknfLkxuE6FtXd9N0NRzZ1UCwLA4=-4UW=8g@mail.gmail.com>
Download mbox | patch
Permalink /patch/110168/
State New
Headers show

Comments

Liang Wang - Aug. 16, 2011, 9:35 a.m.
Current implementation of ggc_alloc_rtvec_resized allocates more
spaces for rtvec.  This patch uses original formula to compute size
for rtvec.  Bootstrap on x86_64 successfully.

OK for trunk?

By the way, I don't have write access to SVN repository yet.  Could
you please help commit it after approval?

Liang.


2011-08-16  Liang Wang  <lwang1@marvell.com>

	* ggc.h (ggc_alloc_rtvec_sized): Change arguments of
	ggc_alloc_zone_vec_rtvec_def.
Richard Guenther - Aug. 16, 2011, 9:47 a.m.
On Tue, Aug 16, 2011 at 11:35 AM, Liang Wang <netcasper@gmail.com> wrote:
> Current implementation of ggc_alloc_rtvec_resized allocates more
> spaces for rtvec.  This patch uses original formula to compute size
> for rtvec.  Bootstrap on x86_64 successfully.
>
> OK for trunk?
>
> By the way, I don't have write access to SVN repository yet.  Could
> you please help commit it after approval?

Ok for trunk and 4.6 branch.  I'll take care of committing it.

Thanks,
Richard.

> Liang.
>
>
> 2011-08-16  Liang Wang  <lwang1@marvell.com>
>
>        * ggc.h (ggc_alloc_rtvec_sized): Change arguments of
>        ggc_alloc_zone_vec_rtvec_def.
>
>
> diff --git a/gcc/ggc.h b/gcc/ggc.h
> index 7f2144c..07f0dda 100644
> --- a/gcc/ggc.h
> +++ b/gcc/ggc.h
> @@ -266,8 +266,9 @@ extern struct alloc_zone tree_zone;
>  extern struct alloc_zone tree_id_zone;
>
>  #define ggc_alloc_rtvec_sized(NELT)                                     \
> -    (ggc_alloc_zone_vec_rtvec_def (sizeof (rtx),                        \
> -                                   sizeof (struct rtvec_def) + ((NELT) - 1), \
> +    (ggc_alloc_zone_vec_rtvec_def (1,                                   \
> +                                   sizeof (struct rtvec_def)            \
> +                                  + ((NELT) - 1) * sizeof (rtx),       \
>                                    &rtl_zone))
>
>  #if defined (GGC_ZONE) && !defined (GENERATOR_FILE)
>
Andreas Schwab - Aug. 16, 2011, 11:17 a.m.
Liang Wang <netcasper@gmail.com> writes:

>  #define ggc_alloc_rtvec_sized(NELT)                                     \
> -    (ggc_alloc_zone_vec_rtvec_def (sizeof (rtx),                        \
> -                                   sizeof (struct rtvec_def) + ((NELT) - 1), \
> +    (ggc_alloc_zone_vec_rtvec_def (1,                                   \
> +                                   sizeof (struct rtvec_def)            \
> +				   + ((NELT) - 1) * sizeof (rtx),	\

ggc_alloc_zone_vec_rtvec_def is for allocating an array of rtvec_def,
but you want a single (variable sized) rtvec_def, so
ggc_alloc_zone_rtvec_def is the correct function to call.

Andreas.
Richard Guenther - Aug. 16, 2011, 11:32 a.m.
On Tue, Aug 16, 2011 at 1:17 PM, Andreas Schwab <schwab@redhat.com> wrote:
> Liang Wang <netcasper@gmail.com> writes:
>
>>  #define ggc_alloc_rtvec_sized(NELT)                                     \
>> -    (ggc_alloc_zone_vec_rtvec_def (sizeof (rtx),                        \
>> -                                   sizeof (struct rtvec_def) + ((NELT) - 1), \
>> +    (ggc_alloc_zone_vec_rtvec_def (1,                                   \
>> +                                   sizeof (struct rtvec_def)            \
>> +                                + ((NELT) - 1) * sizeof (rtx),       \
>
> ggc_alloc_zone_vec_rtvec_def is for allocating an array of rtvec_def,
> but you want a single (variable sized) rtvec_def, so
> ggc_alloc_zone_rtvec_def is the correct function to call.

I'm not so sure about that given the more strongly typing of our allocators.
Laurynas?

Thanks,
Richard.

> Andreas.
>
> --
> Andreas Schwab, schwab@redhat.com
> GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
> "And now for something completely different."
>
Laurynas Biveinis - Aug. 16, 2011, 1:52 p.m.
2011/8/16 Richard Guenther <richard.guenther@gmail.com>:
>> ggc_alloc_zone_vec_rtvec_def is for allocating an array of rtvec_def,
>> but you want a single (variable sized) rtvec_def, so
>> ggc_alloc_zone_rtvec_def is the correct function to call.
>
> I'm not so sure about that given the more strongly typing of our allocators.
> Laurynas?

Yes, in size calculations I totally forgot to check the allocator
itself. Andreas is right, the non-vector version is the one to use
here. It will happen to work with non-vector version right now too, as
currently both non-vector and vector allocators cast the result to the
same type, use the same underlying allocator and basically expand to
the same code.

The patch to adjust the definition is obvious/pre-approved if anyone's
inclined, or I'll just note in my TODO to fix this the next time I
commit something.

Thanks for pointing this out.

Patch

diff --git a/gcc/ggc.h b/gcc/ggc.h
index 7f2144c..07f0dda 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -266,8 +266,9 @@  extern struct alloc_zone tree_zone;
 extern struct alloc_zone tree_id_zone;

 #define ggc_alloc_rtvec_sized(NELT)                                     \
-    (ggc_alloc_zone_vec_rtvec_def (sizeof (rtx),                        \
-                                   sizeof (struct rtvec_def) + ((NELT) - 1), \
+    (ggc_alloc_zone_vec_rtvec_def (1,                                   \
+                                   sizeof (struct rtvec_def)            \
+				   + ((NELT) - 1) * sizeof (rtx),	\
                                    &rtl_zone))

 #if defined (GGC_ZONE) && !defined (GENERATOR_FILE)