On Thu, 6 Dec 2012, Martin Jambor wrote:
> Hi,
>
> On Thu, Dec 06, 2012 at 10:35:04AM +0100, Richard Guenther wrote:
> > Don't you want to use get_object_alignment_1 to record whether
> > 'base' is for example at offset 3 of a 16-byte aligned object
> > so that if 'offset' then makes the result aligned you use
> > that alignment instead of the conservative 1-byte alignment
> > you'd otherwise get from get_object_alignment?
> >
> > That is, how do 'offset' and 'base' relate? As far as I understand
> > align should be computed as the alignment of the access to
> > 'base' plus offset 'offset', no?
>
>
> You're right, such situation can occur too, even though probably only
> rarely. Below is a patch that uses get_object_alignment_1, it has
> passed bootstrap and testing on x86_64-linux, and powerpc64-linux, the
> same on sparc64-linux is underway. OK for trunk if it passes there
> too?
Ok.
Thanks,
Richard.
> Thanks,
>
> Martin
>
>
> 2012-12-06 Martin Jambor <mjam...@suse.cz>
>
> PR tree-optimization/55590
> * tree-sra.c (build_ref_for_offset): Use get_object_alignment_1 to
> get base alignment.
>
> * testsuite/gcc.target/i386/pr55590-1.c: New test.
> * testsuite/gcc.target/i386/pr55590-2.c: Likewise.
>
> *** /dev/null Thu Oct 25 13:49:30 2012
> --- gcc/testsuite/gcc.target/i386/pr55590-1.c Tue Dec 4 11:36:54 2012
> ***************
> *** 0 ****
> --- 1,27 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -mavx" } */
> +
> + #include <immintrin.h>
> +
> + struct S
> + {
> + __m128 a, b;
> + };
> +
> + struct T
> + {
> + int a;
> + struct S s;
> + };
> +
> +
> + void foo (struct T *p, __m128 v)
> + {
> + struct S s;
> +
> + s = p->s;
> + s.b = _mm_add_ps(s.b, v);
> + p->s = s;
> + }
> +
> + /* { dg-final { scan-assembler-not "vmovups" } } */
> *** /dev/null Thu Oct 25 13:49:30 2012
> --- gcc/testsuite/gcc.target/i386/pr55590-2.c Tue Dec 4 11:37:02 2012
> ***************
> *** 0 ****
> --- 1,27 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -mavx" } */
> +
> + #include <immintrin.h>
> +
> + struct S
> + {
> + __m128 a, b;
> + };
> +
> + struct T
> + {
> + int a;
> + struct S s[8];
> + };
> +
> +
> + void foo (struct T *p, int i, __m128 v)
> + {
> + struct S s;
> +
> + s = p->s[i];
> + s.b = _mm_add_ps(s.b, v);
> + p->s[i] = s;
> + }
> +
> + /* { dg-final { scan-assembler-not "vmovups" } } */
> *** /tmp/sOn2fd_tree-sra.c Thu Dec 6 15:49:23 2012
> --- gcc/tree-sra.c Thu Dec 6 11:46:23 2012
> *************** make_fancy_name (tree expr)
> *** 1423,1429 ****
> EXP_TYPE at the given OFFSET. If BASE is something for which
> get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is
> used
> to insert new statements either before or below the current one as
> specified
> ! by INSERT_AFTER. This function is not capable of handling bitfields. */
>
> tree
> build_ref_for_offset (location_t loc, tree base, HOST_WIDE_INT offset,
> --- 1423,1432 ----
> EXP_TYPE at the given OFFSET. If BASE is something for which
> get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is
> used
> to insert new statements either before or below the current one as
> specified
> ! by INSERT_AFTER. This function is not capable of handling bitfields.
> !
> ! BASE must be either a declaration or a memory reference that has correct
> ! alignment ifformation embeded in it (e.g. a pre-existing one in SRA). */
>
> tree
> build_ref_for_offset (location_t loc, tree base, HOST_WIDE_INT offset,
> *************** build_ref_for_offset (location_t loc, tr
> *** 1437,1443 ****
> unsigned int align;
>
> gcc_checking_assert (offset % BITS_PER_UNIT == 0);
> !
> base = get_addr_base_and_unit_offset (base, &base_offset);
>
> /* get_addr_base_and_unit_offset returns NULL for references with a
> variable
> --- 1440,1446 ----
> unsigned int align;
>
> gcc_checking_assert (offset % BITS_PER_UNIT == 0);
> ! get_object_alignment_1 (base, &align, &misalign);
> base = get_addr_base_and_unit_offset (base, &base_offset);
>
> /* get_addr_base_and_unit_offset returns NULL for references with a
> variable
> *************** build_ref_for_offset (location_t loc, tr
> *** 1476,1497 ****
> base = build_fold_addr_expr (unshare_expr (base));
> }
>
> ! /* If prev_base were always an originally performed access
> ! we can extract more optimistic alignment information
> ! by looking at the access mode. That would constrain the
> ! alignment of base + base_offset which we would need to
> ! adjust according to offset. */
> ! if (!get_pointer_alignment_1 (base, &align, &misalign))
> ! {
> ! gcc_assert (misalign == 0);
> ! if (TREE_CODE (prev_base) == MEM_REF
> ! || TREE_CODE (prev_base) == TARGET_MEM_REF)
> ! align = TYPE_ALIGN (TREE_TYPE (prev_base));
> ! }
> ! misalign += (tree_to_double_int (off)
> ! .sext (TYPE_PRECISION (TREE_TYPE (off))).low
> ! * BITS_PER_UNIT);
> ! misalign = misalign & (align - 1);
> if (misalign != 0)
> align = (misalign & -misalign);
> if (align < TYPE_ALIGN (exp_type))
> --- 1479,1485 ----
> base = build_fold_addr_expr (unshare_expr (base));
> }
>
> ! misalign = (misalign + offset) & (align - 1);
> if (misalign != 0)
> align = (misalign & -misalign);
> if (align < TYPE_ALIGN (exp_type))
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
