diff mbox

PATCH: PR middle-end/48608: Alignment adjust of local variables is lost

Message ID 20110414133453.GA18805@intel.com
State New
Headers show

Commit Message

H.J. Lu April 14, 2011, 1:34 p.m. UTC
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?

Thanks.

H.J.
---
2011-04-14  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/48608
	* cfgexpand.c (get_decl_align_unit): Update DECL_ALIGN if needed.

Comments

Richard Biener April 14, 2011, 1:57 p.m. UTC | #1
On Thu, Apr 14, 2011 at 3:34 PM, H.J. Lu <hongjiu.lu@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.
Why does it matter that DECL_ALIGN is updated?

> Thanks.
>
> H.J.
> ---
> 2011-04-14  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR middle-end/48608
>        * cfgexpand.c (get_decl_align_unit): Update DECL_ALIGN if needed.
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index cc1382f..e79d50c 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -212,6 +212,8 @@ static unsigned int
>  get_decl_align_unit (tree decl)
>  {
>   unsigned int align = LOCAL_DECL_ALIGNMENT (decl);
> +  if (align > DECL_ALIGN (decl))
> +    DECL_ALIGN (decl) = align;
>   return align / BITS_PER_UNIT;
>  }
>
>
H.J. Lu April 14, 2011, 2:01 p.m. UTC | #2
On Thu, Apr 14, 2011 at 6:57 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Apr 14, 2011 at 3:34 PM, H.J. Lu <hongjiu.lu@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.

> Why does it matter that DECL_ALIGN is updated?
>

My port needs accurate alignment information on local variables.
Richard Biener April 14, 2011, 2:09 p.m. UTC | #3
On Thu, Apr 14, 2011 at 4:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Apr 14, 2011 at 6:57 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, Apr 14, 2011 at 3:34 PM, H.J. Lu <hongjiu.lu@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.

Richard.

> --
> H.J.
>
diff mbox

Patch

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index cc1382f..e79d50c 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -212,6 +212,8 @@  static unsigned int
 get_decl_align_unit (tree decl)
 {
   unsigned int align = LOCAL_DECL_ALIGNMENT (decl);
+  if (align > DECL_ALIGN (decl))
+    DECL_ALIGN (decl) = align;
   return align / BITS_PER_UNIT;
 }