[PATCH PR43513, 1/3] Replace vla with array - Implementation.
Richard Guenther
rguenther@suse.de
Wed Jul 27 14:52:00 GMT 2011
On Wed, 27 Jul 2011, Tom de Vries wrote:
> On 07/27/2011 02:12 PM, Richard Guenther wrote:
> > On Wed, 27 Jul 2011, Tom de Vries wrote:
> >
> >> On 07/27/2011 01:50 PM, Tom de Vries wrote:
> >>> Hi Richard,
> >>>
> >>> I have a patch set for bug 43513 - The stack pointer is adjusted twice.
> >>>
> >>> 01_pr43513.3.patch
> >>> 02_pr43513.3.test.patch
> >>> 03_pr43513.3.mudflap.patch
> >>>
> >>> The patch set has been bootstrapped and reg-tested on x86_64.
> >>>
> >>> I will sent out the patches individually.
> >>>
> >>
> >> The patch replaces a vla __builtin_alloca that has a constant argument with an
> >> array declaration.
> >>
> >> OK for trunk?
> >
> > I don't think it is safe to try to get at the VLA type the way you do.
>
> I don't understand in what way it's not safe. Do you mean I don't manage to find
> the type always, or that I find the wrong type, or something else?
I think you might get the wrong type, you also do not transform code
like
int *p = alloca(4);
*p = 3;
as there is no array type involved here.
> > In fact I would simply do sth like
> >
> > elem_type = build_nonstandard_integer_type (BITS_PER_UNIT, 1);
> > n_elem = size * 8 / BITS_PER_UNIT;
> > array_type = build_array_type_nelts (elem_type, n_elem);
> > var = create_tmp_var (array_type, NULL);
> > return fold_convert (TREE_TYPE (lhs), build_fold_addr_expr (var));
> >
>
> I tried this code on the example, and it works, but the newly declared type has
> an 8-bit alignment, while the vla base type has a 32 bit alignment. This make
> the memory access in the example potentially unaligned, which prohibits an
> ivopts optimization, so the resulting text size is 68 instead of the 64 achieved
> with my current patch.
Ok, so then set DECL_ALIGN of the variable to something reasonable
like MIN (size * 8, GET_MODE_PRECISION (word_mode)). Basically the
alignment that the targets alloca function would guarantee.
> > And obviously you lose the optimization we arrange with inserting
> > __builtin_stack_save/restore pairs that way - stack space will no
> > longer be shared for subsequent VLAs. Which means that you'd
> > better limit the size you allow this promotion.
> >
>
> Right, I could introduce a parameter for this.
I would think you could use PARAM_LARGE_STACK_FRAME for now and say,
allow a size of PARAM_LARGE_STACK_FRAME / 10?
> > Alternatively this promotion could happen alongsize
> > optimize_stack_restore using more global knowledge of the effects
> > on the maximum stack size this folding produces.
> >
>
> OK, I'll look into this.
Thanks,
Richard.
More information about the Gcc-patches
mailing list