On Fri, Jun 26, 2015 at 08:14:55PM +0100, Charles Baylis wrote:
> Since the last ping, I've tweaked the test cases a bit...
>
> Since I've been working on doing the same changes for the ARM backend,
> I've moved the tests into the advsimd-intrinsics directory, marked as
> XFAIL for ARM targets for now. The gcc/ part of the patch is
> unchanged.
Hi Charles,
This patch looks OK to me, though please fix the whitespace nits called
out below:
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -3959,7 +3962,7 @@
> (define_insn "vec_store_lanesoi_lane<mode>"
> [(set (match_operand:<V_TWO_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
> (unspec:<V_TWO_ELEM> [(match_operand:OI 1 "register_operand" "w")
> - (unspec:VQ [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> + (unspec:VALLDIF [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> (match_operand:SI 2 "immediate_operand" "i")]
> UNSPEC_ST2_LANE))]
8 Spaces to tab.
> "TARGET_SIMD"
> @@ -3967,7 +3970,7 @@
> operands[2] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[2])));
> return "st2\\t{%S1.<Vetype> - %T1.<Vetype>}[%2], %0";
> }
> - [(set_attr "type" "neon_store3_one_lane<q>")]
> + [(set_attr "type" "neon_store2_one_lane<q>")]
I would prefer this in a separate patch as it is a separate logical
change. Consider it pre-approved (and obvious) to commit as a one-line
fix on its own.
> @@ -4054,7 +4060,7 @@
> (define_insn "vec_store_lanesci_lane<mode>"
> [(set (match_operand:<V_THREE_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
> (unspec:<V_THREE_ELEM> [(match_operand:CI 1 "register_operand" "w")
> - (unspec:VQ [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> + (unspec:VALLDIF [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> (match_operand:SI 2 "immediate_operand" "i")]
> UNSPEC_ST3_LANE))]
8 Spaces to tab.
> @@ -4149,7 +4158,7 @@
> (define_insn "vec_store_lanesxi_lane<mode>"
> [(set (match_operand:<V_FOUR_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
> (unspec:<V_FOUR_ELEM> [(match_operand:XI 1 "register_operand" "w")
> - (unspec:VQ [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> + (unspec:VALLDIF [(const_int 0)] UNSPEC_VSTRUCTDUMMY)
> (match_operand:SI 2 "immediate_operand" "i")]
8 Spaces to tab.
> diff --git
> a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vld2_lane_f32_indices_1.c
>
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vld2_lane_f32_indices_1.c
> new file mode 100644
> index 0000000..04be713
> --- /dev/null
> +++
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vld2_lane_f32_indices_1.c
> @@ -0,0 +1,16 @@
> +#include <arm_neon.h>
> +
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } } */
This seems an odd limitation, presumably this is a side effect of waiting
until expand time to throw an error... It does suggest that we're tackling
the problem in the wrong way by pushing this to so late in the compilation
pipeline. The property here is on a type itself, which must take a constant
value within a given range. That feels much more like the sort of thing
we should be detecting and bailing out on closer to the front-end - perhaps
with a more generic extension allowing you to annotate any type with an
expected/required range (both as a helping hand for VRP and as a way to
express programmer defined preconditions).
But, given that adding such an extension is likely more effort than needed
I think this is OK for now!
Cheers,
James
