Re: [PATCH] [AArch64] PR63870 Improve error messages for NEON single lane memory access intrinsics

[Charles Baylis <charles.baylis@linaro.org>]

On 8 June 2015 at 10:33, Alan Lawrence <alan.lawrence@arm.com> wrote:
> Thanks for working on this!
>
> I'd been fiddling around with a patch with some similar elements to this,
> but many trials with union types, subregs, etc., all worsened the register
> allocation and led to more unnecessary shuffling / moves.

Kugan has been looking into this at Linaro. We should avoid
duplicating effort here.

> The only real
> thing I tried which you don't do here, was to introduce a set_dreg expander
> to clean up some of those macro definitions in arm_neon.h. That could easily
> follow in a separate patch if desired!

I'd prefer that to be a separate step.

> So your patch looks good to me.
>
> A couple of style nits:
>
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -128,7 +128,9 @@ enum aarch64_type_qualifiers
>    /* Polynomial types.  */
>    qualifier_poly = 0x100,
>    /* Lane indices - must be in range, and flipped for bigendian.  */
> -  qualifier_lane_index = 0x200
> +  qualifier_lane_index = 0x200,
> +  /* Lane indices for single lane structure loads and stores */
> +  qualifier_struct_load_store_lane_index = 0x400
>  };
>
> should be ...'loads and stores.  */'
>
> also the dg-error messages in the testsuite, do not need to be on the same
> line as the statement generating the error, because the trailing 0 tells dg
> that the position/line number doesn't matter (i.e. dg should allow the error
> to be reported at any line); so these could be brought under 80 chars.

OK, thanks. I'll re-spin once I've tested on big endian.

> Oh, have you tested bigendian?

I have started a bigendian build on our validation infrastructure here.

Thanks for the review
Charles