diff mbox

Fix PR tree-optimization/51315

Message ID 201201031226.53674.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Jan. 3, 2012, 11:26 a.m. UTC 
> Shouldn't we go back and unconditionally return false if lhs and (or?) rhs
> are BLKmode?  I suppose they are, in this case.  Or should we truncate
> the alignment from get_object_alignment to TYPE_ALIGN if it is bigger
> than that to avoid this situation?

Sorry, I didn't read your message before sending mine.  Yes, the alignment cap 
appears to be the best solution, but of course TYPE_ALIGN (access->type) won't 
work (that was the original code), so TYPE_ALIGN (TREE_TYPE (access->base)):

Comments

Richard Biener Jan. 3, 2012, 11:37 a.m. UTC | #1 
On Tue, Jan 3, 2012 at 12:26 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Shouldn't we go back and unconditionally return false if lhs and (or?) rhs
>> are BLKmode?  I suppose they are, in this case.  Or should we truncate
>> the alignment from get_object_alignment to TYPE_ALIGN if it is bigger
>> than that to avoid this situation?
>
> Sorry, I didn't read your message before sending mine.  Yes, the alignment cap
> appears to be the best solution, but of course TYPE_ALIGN (access->type) won't
> work (that was the original code), so TYPE_ALIGN (TREE_TYPE (access->base)):
>
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c  (revision 182780)
> +++ tree-sra.c  (working copy)
> @@ -1124,7 +1124,9 @@ build_accesses_from_assign (gimple stmt)
>     {
>       lacc->grp_assignment_write = 1;
>       if (STRICT_ALIGNMENT
> -         && tree_non_aligned_mem_p (rhs, get_object_alignment (lhs)))
> +         && tree_non_aligned_mem_p (rhs,
> +                                    MIN (TYPE_ALIGN (TREE_TYPE (lacc->base)),
> +                                         get_object_alignment (lhs))))
>         lacc->grp_unscalarizable_region = 1;
>     }
>
> @@ -1135,7 +1137,9 @@ build_accesses_from_assign (gimple stmt)
>          && !is_gimple_reg_type (racc->type))
>        bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base));
>       if (STRICT_ALIGNMENT
> -         && tree_non_aligned_mem_p (lhs, get_object_alignment (rhs)))
> +         && tree_non_aligned_mem_p (lhs,
> +                                    MIN (TYPE_ALIGN (TREE_TYPE (racc->base)),
> +                                         get_object_alignment (rhs))))
>         racc->grp_unscalarizable_region = 1;

I suppose we should move this logic to tree_non_aligned_mem_p, maybe
renaming it to tree_non_matching_align_mems_p or so, thus passing
down both lhs and rhs.

But I agree - let's sort out correctness issues first, then try to address the
missed optimizations we caused by that.

Richard.

>     }
>
> --
> Eric Botcazou
diff mbox

Patch

Index: tree-sra.c
===================================================================
--- tree-sra.c  (revision 182780)
+++ tree-sra.c  (working copy)
@@ -1124,7 +1124,9 @@  build_accesses_from_assign (gimple stmt)
     {
       lacc->grp_assignment_write = 1;
       if (STRICT_ALIGNMENT
-         && tree_non_aligned_mem_p (rhs, get_object_alignment (lhs)))
+         && tree_non_aligned_mem_p (rhs,
+                                    MIN (TYPE_ALIGN (TREE_TYPE (lacc->base)),
+                                         get_object_alignment (lhs))))
         lacc->grp_unscalarizable_region = 1;
     }

@@ -1135,7 +1137,9 @@  build_accesses_from_assign (gimple stmt)
          && !is_gimple_reg_type (racc->type))
        bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base));
       if (STRICT_ALIGNMENT
-         && tree_non_aligned_mem_p (lhs, get_object_alignment (rhs)))
+         && tree_non_aligned_mem_p (lhs,
+                                    MIN (TYPE_ALIGN (TREE_TYPE (racc->base)),
+                                         get_object_alignment (rhs))))
         racc->grp_unscalarizable_region = 1;
     }