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