| Message ID | 1293205276.21419.23.camel@e102325-lin.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
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. >
*** 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);