Patchwork Fix alignment information in RTL for known misaligned addresses.

login
register
mail settings
Submitter Ramana Radhakrishnan
Date Dec. 24, 2010, 3:41 p.m.
Message ID <1293205276.21419.23.camel@e102325-lin.cambridge.arm.com>
Download mbox | patch
Permalink /patch/76632/
State New
Headers show

Comments

Ramana Radhakrishnan - Dec. 24, 2010, 3:41 p.m.
Hi,

We've been investigating a few failures in the regression testsuite with
Neon enabled and these show up here. 

http://gcc.gnu.org/ml/gcc-testresults/2010-12/msg02072.html

One of the failures is with the builtins test for memcpy. 

memcpy is called with the following values. 

 (dst=0x15360, src=0xbb7c, n=17)

It ends up with a bus error on a Cortex A9 because we have a vld1.8
instruction with an alignment specifier of 64 bits and the address we
pass (src) in this case is not 64 bit aligned. The alignment specifiers
are supposed to be hints to the memory system to accelerate memory
accesses.

If the address is not aligned as per the alignment specifier i.e. you've
specified more alignment than you actually have in the base address the
hardware will cause an unaligned access abort.  Thus it's important for
the compiler to track alignment information correctly. 

This attached patch by Richard attempts to fix this by setting the
correct alignment to the generated memory rtx before the generation of
the call to movmisalign_optab. By default the memory alignment is set to
be the natural alignment of the type rather than what the alignment was
set up to be at the gimple level. 

This fixes all the failures with respect to builtins.exp that were
observed in the test report and fixes runs for a number of benchmarks
that were failing with Neon turned on at O3. A full bootstrap and test
for armv7-a , neon and softfp is on currently. 

Ok to commit if no regressions ? 

cheers
Ramana

<DATE>  Richard Earnshaw  <rearnsha@arm.com>

	* expr.c (expand_assignment): Set MEM_ALIGN correctly for
	misaligned accesses.
	(expand_expr_real_1): Likewise.
Richard Guenther - Dec. 26, 2010, 10:13 p.m.
On Fri, Dec 24, 2010 at 4:41 PM, Ramana Radhakrishnan
<ramana.radhakrishnan@arm.com> wrote:
> Hi,
>
> We've been investigating a few failures in the regression testsuite with
> Neon enabled and these show up here.
>
> http://gcc.gnu.org/ml/gcc-testresults/2010-12/msg02072.html
>
> One of the failures is with the builtins test for memcpy.
>
> memcpy is called with the following values.
>
>  (dst=0x15360, src=0xbb7c, n=17)
>
> It ends up with a bus error on a Cortex A9 because we have a vld1.8
> instruction with an alignment specifier of 64 bits and the address we
> pass (src) in this case is not 64 bit aligned. The alignment specifiers
> are supposed to be hints to the memory system to accelerate memory
> accesses.
>
> If the address is not aligned as per the alignment specifier i.e. you've
> specified more alignment than you actually have in the base address the
> hardware will cause an unaligned access abort.  Thus it's important for
> the compiler to track alignment information correctly.
>
> This attached patch by Richard attempts to fix this by setting the
> correct alignment to the generated memory rtx before the generation of
> the call to movmisalign_optab. By default the memory alignment is set to
> be the natural alignment of the type rather than what the alignment was
> set up to be at the gimple level.
>
> This fixes all the failures with respect to builtins.exp that were
> observed in the test report and fixes runs for a number of benchmarks
> that were failing with Neon turned on at O3. A full bootstrap and test
> for armv7-a , neon and softfp is on currently.
>
> Ok to commit if no regressions ?

The correct way to fix this is IMHO to make set_mem_attributes_minus_bitpos
not get initial alignment from the mode (but assert the mem-attrs are not
set yet in that function).  At least if I understand the problem correctly.

Ulrich promised to do this a while back ...

Richard.

> cheers
> Ramana
>
> <DATE>  Richard Earnshaw  <rearnsha@arm.com>
>
>        * expr.c (expand_assignment): Set MEM_ALIGN correctly for
>        misaligned accesses.
>        (expand_expr_real_1): Likewise.
>

Patch

*** expr.c	(revision 168190)
--- expr.c	(local)
*************** expand_assignment (tree to, tree from, b
*** 4191,4196 ****
--- 4191,4201 ----
  	  && op_mode1 != VOIDmode)
  	reg = copy_to_mode_reg (op_mode1, reg);
  
+       /* The aligment on the memory is currently the natural alignment
+ 	 for the type, not that for an unaligned load, so force it
+ 	 down to the known alignment.  */
+       set_mem_align (mem, align);
+ 
        insn = GEN_FCN (icode) (mem, reg);
        /* The movmisalign<mode> pattern cannot fail, else the assignment would
           silently be omitted.  */
*************** expand_expr_real_1 (tree exp, rtx target
*** 8656,8661 ****
--- 8661,8671 ----
  	  {
  	    rtx reg, insn;
  
+ 	    /* The aligment on the memory is currently the natural
+ 	       alignment for the type, not that for an unaligned load,
+ 	       so force it down to the known alignment.  */
+ 	    set_mem_align (temp, align);
+ 
  	    /* We've already validated the memory, and we're creating a
  	       new pseudo destination.  The predicates really can't fail.  */
  	    reg = gen_reg_rtx (mode);
*************** expand_expr_real_1 (tree exp, rtx target
*** 8753,8758 ****
--- 8763,8773 ----
  	  {
  	    rtx reg, insn;
  
+ 	    /* The aligment on the memory is currently the natural
+ 	       alignment for the type, not that for an unaligned load,
+ 	       so force it down to the known alignment.  */
+ 	    set_mem_align (temp, align);
+ 
  	    /* We've already validated the memory, and we're creating a
  	       new pseudo destination.  The predicates really can't fail.  */
  	    reg = gen_reg_rtx (mode);