[PATCH] Fix PR97205

Bernd Edlinger bernd.edlinger@hotmail.de
Mon Nov 2 19:36:55 GMT 2020


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.
>>
> 


More information about the Gcc-patches mailing list