This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix unaligned access generated by IVOPTS


[Sorry for dropping the ball here]

> I think that may_be_unaligned_p is just seriously out-dated ... shouldn't it
> be sth like
> 
>   get_object_alignment_1 (ref, &align, &bitpos);
>   if step * BITS_PER_UNIT + bitpos is misaligned
> ...
> 
> or rather all this may_be_unaligned_p stuff should be dropped and IVOPTs
> should finally generate proper [TARGET_]MEM_REFs instead?  That is,
> we already handle aliasing fine:
> 
>   ref = create_mem_ref (&bsi, TREE_TYPE (*use->op_p), &aff,
>                         reference_alias_ptr_type (*use->op_p),
>                         iv, base_hint, data->speed);
> 
> so just also handle alignment properly by passing down
> get_object_alignment (*use->op_p) and in create_mem_ref_raw
> do at the end do the
> 
>   if (TYPE_MODE (type) != BLKmode
>       && GET_MODE_ALIGNMENT (TYPE_MODE (type)) > align)
>     type = build_aligned_type (type, align);
> 
> for BLKmode we already look at TYPE_ALIGN and as we do not change
> the access type(?) either the previous code was already wrong or it was
> fine, so there is nothing to do.
> 
> So - if you want to give it a try...?

After a bit of pondering, I'm not really thrilled, as this would mean changing 
TARGET_MEM_REF to accept invalid (unaligned) memory references for the target.  
But I agree that may_be_unaligned_p is seriously outdated, so the attached 
patch entirely rewrites it, fixing the bug in the process.

Tested on SPARC, SPARC64, IA-64 and ARM, OK for the mainline?


2014-01-10  Eric Botcazou  <ebotcazou@adacore.com>

	* builtins.c (get_object_alignment_2): Minor tweak.
	* tree-ssa-loop-ivopts.c (may_be_unaligned_p): Rewrite.


-- 
Eric Botcazou
Index: builtins.c
===================================================================
--- builtins.c	(revision 206455)
+++ builtins.c	(working copy)
@@ -434,7 +434,7 @@ get_object_alignment_2 (tree exp, unsign
      alignment that can prevail.  */
   if (offset)
     {
-      int trailing_zeros = tree_ctz (offset);
+      unsigned int trailing_zeros = tree_ctz (offset);
       if (trailing_zeros < HOST_BITS_PER_INT)
 	{
 	  unsigned int inner = (1U << trailing_zeros) * BITS_PER_UNIT;
Index: tree-ssa-loop-ivopts.c
===================================================================
--- tree-ssa-loop-ivopts.c	(revision 206455)
+++ tree-ssa-loop-ivopts.c	(working copy)
@@ -1668,50 +1668,30 @@ constant_multiple_of (tree top, tree bot
     }
 }
 
-/* Returns true if memory reference REF with step STEP may be unaligned.  */
+/* Return true if memory reference REF with step STEP may be unaligned.  */
 
 static bool
 may_be_unaligned_p (tree ref, tree step)
 {
-  tree base;
-  tree base_type;
-  HOST_WIDE_INT bitsize;
-  HOST_WIDE_INT bitpos;
-  tree toffset;
-  enum machine_mode mode;
-  int unsignedp, volatilep;
-  unsigned base_align;
-
   /* TARGET_MEM_REFs are translated directly to valid MEMs on the target,
      thus they are not misaligned.  */
   if (TREE_CODE (ref) == TARGET_MEM_REF)
     return false;
 
-  /* The test below is basically copy of what expr.c:normal_inner_ref
-     does to check whether the object must be loaded by parts when
-     STRICT_ALIGNMENT is true.  */
-  base = get_inner_reference (ref, &bitsize, &bitpos, &toffset, &mode,
-			      &unsignedp, &volatilep, true);
-  base_type = TREE_TYPE (base);
-  base_align = get_object_alignment (base);
-  base_align = MAX (base_align, TYPE_ALIGN (base_type));
-
-  if (mode != BLKmode)
-    {
-      unsigned mode_align = GET_MODE_ALIGNMENT (mode);
-
-      if (base_align < mode_align
-	  || (bitpos % mode_align) != 0
-	  || (bitpos % BITS_PER_UNIT) != 0)
-	return true;
-
-      if (toffset
-	  && (highest_pow2_factor (toffset) * BITS_PER_UNIT) < mode_align)
-	return true;
+  unsigned int align = TYPE_ALIGN (TREE_TYPE (ref));
 
-      if ((highest_pow2_factor (step) * BITS_PER_UNIT) < mode_align)
-	return true;
-    }
+  unsigned HOST_WIDE_INT bitpos;
+  unsigned int ref_align;
+  get_object_alignment_1 (ref, &ref_align, &bitpos);
+  if (ref_align < align
+      || (bitpos % align) != 0
+      || (bitpos % BITS_PER_UNIT) != 0)
+    return true;
+
+  unsigned int trailing_zeros = tree_ctz (step);
+  if (trailing_zeros < HOST_BITS_PER_INT
+      && (1U << trailing_zeros) * BITS_PER_UNIT < align)
+    return true;
 
   return false;
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]