On 11/2/20 3:07 PM, Richard Biener wrote:
> On Mon, 2 Nov 2020, Bernd Edlinger wrote:
>
>> Hi,
>>
>> this makes sure that stack allocated SSA_NAMEs are
>> at least MODE_ALIGNED. Also increase the MEM_ALIGN
>> for the corresponding rtl objects.
>>
>>
>> Tested on x86_64-pc-linux-gnu and arm-none-eabi.
>>
>> OK for trunk?
>
>
> @@ -1022,6 +1030,14 @@ expand_one_stack_var_at (tree decl, rtx base,
> unsigned base_align,
> }
>
> set_rtl (decl, x);
> +
> + if (TREE_CODE (decl) == SSA_NAME
> + && GET_MODE (x) != BLKmode
> + && MEM_ALIGN (x) < GET_MODE_ALIGNMENT (GET_MODE (x)))
> + {
> + gcc_checking_assert (GET_MODE_ALIGNMENT (GET_MODE (x)) <=
> base_align);
> + set_mem_align (x, GET_MODE_ALIGNMENT (GET_MODE (x)));
> + }
> }
>
>
> I wonder whether we cannot "fix" set_rtl to not call
> set_mem_attributes in this path, maybe directly call
> set_mem_align there instead? That is, the preceeding
> code for ! SSA_NAME already tries to adjust alignment
> to honor that of the actual stack slot - IMHO the
> non-SSA and SSA cases should be merged after this
> patch, but maybe simply by calling set_mem_align
> instead of doing the DECL_ALIGN frobbing and do
> the alignment compute also for SSA_NAMEs?
>
> The other pieces look OK but the above is a bit ugly
> at the moment.
>
Hmm, how about this?
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1007,20 +1007,21 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned
base_align,
x = plus_constant (Pmode, base, offset);
x = gen_rtx_MEM (TREE_CODE (decl) == SSA_NAME
? TYPE_MODE (TREE_TYPE (decl))
- : DECL_MODE (SSAVAR (decl)), x);
+ : DECL_MODE (decl), x);
+
+ /* Set alignment we actually gave this decl if it isn't an SSA name.
+ If it is we generate stack slots only accidentally so it isn't as
+ important, we'll simply set the alignment directly on the MEM_P. */
+
+ if (base == virtual_stack_vars_rtx)
+ offset -= frame_phase;
+ align = known_alignment (offset);
+ align *= BITS_PER_UNIT;
+ if (align == 0 || align > base_align)
+ align = base_align;
if (TREE_CODE (decl) != SSA_NAME)
{
- /* Set alignment we actually gave this decl if it isn't an SSA name.
- If it is we generate stack slots only accidentally so it isn't as
- important, we'll simply use the alignment that is already set. */
- if (base == virtual_stack_vars_rtx)
- offset -= frame_phase;
- align = known_alignment (offset);
- align *= BITS_PER_UNIT;
- if (align == 0 || align > base_align)
- align = base_align;
-
/* One would think that we could assert that we're not decreasing
alignment here, but (at least) the i386 port does exactly this
via the MINIMUM_ALIGNMENT hook. */
@@ -1031,13 +1032,7 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned
base_align,
set_rtl (decl, x);
- if (TREE_CODE (decl) == SSA_NAME
- && GET_MODE (x) != BLKmode
- && MEM_ALIGN (x) < GET_MODE_ALIGNMENT (GET_MODE (x)))
- {
- gcc_checking_assert (GET_MODE_ALIGNMENT (GET_MODE (x)) <= base_align);
- set_mem_align (x, GET_MODE_ALIGNMENT (GET_MODE (x)));
- }
+ set_mem_align (x, align);
}
class stack_vars_data
Is it OK if it passes bootstrap and regtesting ?
Thanks
Bernd.
> Thanks,
> Richard,
>
>>
>>
>> Thanks
>> Bernd.
>>
>
