On Thu, Apr 14, 2011 at 4:27 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Apr 14, 2011 at 7:09 AM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Thu, Apr 14, 2011 at 4:01 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>> On Thu, Apr 14, 2011 at 6:57 AM, Richard Guenther
>>> <richard.guent...@gmail.com> wrote:
>>>> On Thu, Apr 14, 2011 at 3:34 PM, H.J. Lu <hongjiu...@intel.com> wrote:
>>>>> We have
>>>>>
>>>>> static unsigned int
>>>>> get_decl_align_unit (tree decl)
>>>>> {
>>>>> unsigned int align = LOCAL_DECL_ALIGNMENT (decl);
>>>>> return align / BITS_PER_UNIT;
>>>>> }
>>>>>
>>>>> LOCAL_DECL_ALIGNMENT may increase alignment for local variable. But it is
>>>>> never saved. DECL_ALIGN (decl) returns the old alignment. This patch
>>>>> updates DECL_ALIGN if needed. OK for trunk if there are no regressions?
>>>>
>>>> A get_* function does not seem like a good place to do such things.
>>>
>>> Any suggestion to how to do it properly? I can rename
>>> get_decl_align_unit to align_local_variable.
>>
>> That works for me.
>>
>>>> Why does it matter that DECL_ALIGN is updated?
>>>>
>>>
>>> My port needs accurate alignment information on local variables.
>>
>> I see.
>
> Here is the updated patch. OK for trunk if there are no regressions?
>
> Thanks.
>
> --
> H.J.
> ---
> 2011-04-14 H.J. Lu <hongjiu...@intel.com>
>
> PR middle-end/48608
> * cfgexpand.c (get_decl_align_unit): Renamed to ...
> (align_local_variable): This. Update DECL_ALIGN if needed.
> (add_stack_var): Updated.
> (expand_one_stack_var): Likewise.
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index cc1382f..d38d2f9 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -205,13 +205,15 @@ static bool has_protected_decls;
> smaller than our cutoff threshold. Used for -Wstack-protector. */
> static bool has_short_buffer;
>
> -/* Discover the byte alignment to use for DECL. Ignore alignment
> +/* Compute the byte alignment to use for DECL. Ignore alignment
> we can't do with expected alignment of the stack boundary. */
>
> static unsigned int
> -get_decl_align_unit (tree decl)
> +align_local_variable (tree decl)
> {
> unsigned int align = LOCAL_DECL_ALIGNMENT (decl);
> + if (align > DECL_ALIGN (decl))
> + DECL_ALIGN (decl) = align;
Shouldn't this unconditionally set DECL_ALIGN in case
LOCAL_DECL_ALINGMENT returns something smaller?
Ok with that change.
Thanks,
Richard.
> return align / BITS_PER_UNIT;
> }
>
> @@ -273,7 +275,7 @@ add_stack_var (tree decl)
> variables that are simultaneously live. */
> if (v->size == 0)
> v->size = 1;
> - v->alignb = get_decl_align_unit (SSAVAR (decl));
> + v->alignb = align_local_variable (SSAVAR (decl));
>
> /* All variables are initially in their own partition. */
> v->representative = stack_vars_num;
> @@ -905,7 +907,7 @@ expand_one_stack_var (tree var)
> unsigned byte_align;
>
> size = tree_low_cst (DECL_SIZE_UNIT (SSAVAR (var)), 1);
> - byte_align = get_decl_align_unit (SSAVAR (var));
> + byte_align = align_local_variable (SSAVAR (var));
>
> /* We handle highly aligned variables in expand_stack_vars. */
> gcc_assert (byte_align * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT);
>
