Hi Richard

An arm testcase that can reproduce this bug is attached.

2014-10-20  Guozhi Wei  <car...@google.com>

        PR tree-optimization/63530
        gcc.target/arm/pr63530.c: New testcase.


Index: pr63530.c
===================================================================
--- pr63530.c (revision 0)
+++ pr63530.c (revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon } */
+/* { dg-options "-march=armv7-a -mfloat-abi=hard -mfpu=neon -marm -O2
-ftree-vectorize -funroll-loops --param
\"max-completely-peeled-insns=400\"" } */
+
+typedef struct {
+  unsigned char map[256];
+  int i;
+} A, *AP;
+
+void* calloc(int, int);
+
+AP foo (int n)
+{
+  AP b = (AP)calloc (1, sizeof (A));
+  int i;
+  for (i = n; i < 256; i++)
+    b->map[i] = i;
+  return b;
+}
+
+/* { dg-final { scan-assembler-not "vst1.64" } } */

On Mon, Oct 20, 2014 at 1:19 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Fri, Oct 17, 2014 at 7:58 PM, Carrot Wei <car...@google.com> wrote:
>
> I miss a testcase.  I also miss a comment before this code explaining
> why DR_MISALIGNMENT if not -1 is valid and why it is not valid if

DR_MISALIGNMENT (dr) == -1 means some unknown misalignment, otherwise
it means some known misalignment.
See the usage in file tree-vect-stmts.c.

> 'offset' is supplied (what about 'byte_offset' btw?).  Also if peeling

It is for conservative, so it doesn't change the logic when offset is supplied.
I've checked that most of the passed in offset are caused by negative
step, its impact to DR_MISALIGNMENT should have already be considered
in function vect_update_misalignment_for_peel, but the comments of
vect_create_addr_base_for_vector_ref does not guarantee this usage of
offset.

The usage of byte_offset is quite broken, many direct or indirect
callers don't provide the parameters. So only the author can comment
this.

> for alignment aligned this ref (misalign == 0) you don't set the alignment.
>
I assume if no misalignment is specified, the natural alignment of the
vector type is used, and caused the wrong code in our case, is it
right?

> Thus you may fix a bug (not sure without a testcase) but the new code
> certainly doesn't look 100% correct.
>
> That said, I would have expected that we can unconditionally do
>
>  set_ptr_info_alignment (..., align, misalign)
>
> if misalign is != -1 and if we adjust misalign by offset * step + byte_offset
> (usually both are constants).
>
> Also we can still trust the alignment copied from addr_base modulo
> vector element size even if DR_MISALIGN is -1.  This may matter
> for targets that require element-alignment for vector accesses.
>

Reply via email to