Re: [PATCH, ARM] Misaligned access support for ARM Neon

[Ira Rosen <IRAR at il dot ibm dot com>]

gcc-patches-owner@gcc.gnu.org wrote on 17/11/2009 19:19:31:

> Julian Brown <julian@codesourcery.com>
...
>
> Neon vld1/vst1 can't be used for *arbitrary* alignments: the
> minimum alignment is the element size for the access in question.
> Another target hook (vector_min_alignment) is defined to teach the
> vectorizer this. Correspondingly for the testsuite,
> check_effective_target_vect_element_align has been added to
> target-supports.exp.
>
> The latter unfortunately is fairly redundant with
> check_effective_target_vect_hw_misalign, which has been introduced
> since this patch was written. *_vect_element_align provides a weaker
> promise (i.e. element alignment vs. arbitrary alignment), though one
> which is probably sufficient in most cases. I'm not sure if it makes
> sense to keep both predicates: at present, I've updated quite a few
> tests which previously used *_vect_hw_misalign to use
> *_vect_element_align instead when it improves test results for ARM, but
> not exhaustively.

I think, vect_element_align can replace vect_hw_misalign in all the
testcases except gcc.dg/vect/vect-align-1.c and gcc.dg/vect/vect-align-2.c.
These tests contain packed structures.


>
> Test results (cross to ARM Linux) show some additional failures. I
> think these are just showing up missing features elsewhere in the Neon
> support appearing now because of the dejagnu tweaks, rather than
> problems with this patch as such.
>
> OK to apply?
>



> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -796,7 +796,8 @@ vect_compute_data_ref_alignment (struct
data_reference *dr)
>    /* At this point we assume that the base is aligned.  */
>    gcc_assert (base_aligned
>           || (TREE_CODE (base) == VAR_DECL
> -           && DECL_ALIGN (base) >= TYPE_ALIGN (vectype)));
> +           && (DECL_ALIGN (base)
> +               >= targetm.vectorize.vector_min_alignment (vectype))));

Looks like you forgot to multiply by BITS_PER_UNIT here.


>    * tree-vect-stmts.c (vectorizable_store): Honour
>     targetm.vectorize.always_misalign.
>     (vectorizable_load): Ditto.

I would prefer to have all the alignment queries in
vect_supportable_dr_alignment(). Maybe you could add another enumeration
value to enum dr_alignment_support?

Otherwise, the vectorizer part is OK with me.

Thanks,
Ira

> Julian
>