* [PATCH 3/4] vstN_lane error message enhancements (Q register)
2014-12-09 15:28 [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store charles.baylis
@ 2014-12-09 15:28 ` charles.baylis
2014-12-09 15:28 ` [PATCH 4/4] vstN_lane error message enhancements (D registers) charles.baylis
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: charles.baylis @ 2014-12-09 15:28 UTC (permalink / raw)
To: rearnsha, gcc-patches, marcus.shawcroft, tejas.belagod, alan.lawrence
From: Charles Baylis <charles.baylis@linaro.org>
gcc/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* config/aarch64/aarch64-builtins.c
(aarch64_types_storestruct_lane_qualifiers): Mark last argument with
qualifier_struct_load_store_lane_index.
gcc/testsuite/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* gcc.target/aarch64/simd/vst4q_lane.c: New test.
Change-Id: If097c9d32eb6eb3d4c4e16db81f81e44a3154509
---
gcc/config/aarch64/aarch64-builtins.c | 2 +-
gcc/testsuite/gcc.target/aarch64/simd/vst4q_lane.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vst4q_lane.c
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 27046e2..f2fb939 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -258,7 +258,7 @@ aarch64_types_store1_qualifiers[SIMD_MAX_BUILTIN_ARGS]
static enum aarch64_type_qualifiers
aarch64_types_storestruct_lane_qualifiers[SIMD_MAX_BUILTIN_ARGS]
= { qualifier_void, qualifier_pointer_map_mode,
- qualifier_none, qualifier_none };
+ qualifier_none, qualifier_struct_load_store_lane_index };
#define TYPES_STORESTRUCT_LANE (aarch64_types_storestruct_lane_qualifiers)
#define CF0(N, X) CODE_FOR_aarch64_##N##X
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vst4q_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vst4q_lane.c
new file mode 100644
index 0000000..849f07a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vst4q_lane.c
@@ -0,0 +1,15 @@
+/* Test error message when passing an invalid value as a lane index. */
+
+/* { dg-do compile } */
+
+#include <arm_neon.h>
+
+void
+f_vst4q_lane (int8_t * p, int8x16x4_t v)
+{
+ vst4q_lane_s8 (p, v, 16);
+/* { dg-error "lane 16 out of range 0 - 15" "" { target *-*-* } 0 } */
+ return;
+}
+
+
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] vstN_lane error message enhancements (D registers)
2014-12-09 15:28 [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store charles.baylis
2014-12-09 15:28 ` [PATCH 3/4] vstN_lane error message enhancements (Q register) charles.baylis
@ 2014-12-09 15:28 ` charles.baylis
2014-12-09 15:28 ` [PATCH 2/4] vldN_lane " charles.baylis
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: charles.baylis @ 2014-12-09 15:28 UTC (permalink / raw)
To: rearnsha, gcc-patches, marcus.shawcroft, tejas.belagod, alan.lawrence
From: Charles Baylis <charles.baylis@linaro.org>
gcc/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* config/aarch64/arm_neon.h (__ST2_LANE_FUNC): Add explicit lane bounds
check.
(__ST3_LANE_FUNC): Likewise.
(__ST4_LANE_FUNC): Likewise.
gcc/testsuite/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* gcc.target/aarch64/simd/vst4_lane.c: New test.
Change-Id: I6bceaeb7773bf20860daca4013ea1c4d2c06afa6
---
gcc/config/aarch64/arm_neon.h | 6 ++++++
gcc/testsuite/gcc.target/aarch64/simd/vst4_lane.c | 14 ++++++++++++++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vst4_lane.c
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index 22df564..22c6d06 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -11181,6 +11181,8 @@ vst2_lane_ ## funcsuffix (ptrtype *__ptr, \
__temp.val[1] \
= vcombine_##funcsuffix (__b.val[1], \
vcreate_##funcsuffix (__AARCH64_UINT64_C (0))); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (__b.val[0]) / sizeof (*__ptr)); \
__o = __builtin_aarch64_set_qregoi##mode (__o, \
(signedtype) __temp.val[0], 0); \
__o = __builtin_aarch64_set_qregoi##mode (__o, \
@@ -11258,6 +11260,8 @@ vst3_lane_ ## funcsuffix (ptrtype *__ptr, \
(signedtype) __temp.val[1], 1); \
__o = __builtin_aarch64_set_qregci##mode (__o, \
(signedtype) __temp.val[2], 2); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (__b.val[0]) / sizeof (*__ptr)); \
__builtin_aarch64_st3_lane##mode ((__builtin_aarch64_simd_ ## ptr_mode *) \
__ptr, __o, __c); \
}
@@ -11336,6 +11340,8 @@ vst4_lane_ ## funcsuffix (ptrtype *__ptr, \
(signedtype) __temp.val[2], 2); \
__o = __builtin_aarch64_set_qregxi##mode (__o, \
(signedtype) __temp.val[3], 3); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (__b.val[0]) / sizeof (*__ptr)); \
__builtin_aarch64_st4_lane##mode ((__builtin_aarch64_simd_ ## ptr_mode *) \
__ptr, __o, __c); \
}
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vst4_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vst4_lane.c
new file mode 100644
index 0000000..6627ecf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vst4_lane.c
@@ -0,0 +1,14 @@
+/* Test error message when passing an invalid value as a lane index. */
+
+/* { dg-do compile } */
+
+#include <arm_neon.h>
+
+void
+f_vst4_lane (int8_t * p, int8x8x4_t v)
+{
+ /* { dg-error "lane 8 out of range 0 - 7" "" { target *-*-* } 0 } */
+ vst4_lane_s8 (p, v, 8);
+ return;
+}
+
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] vldN_lane error message enhancements (D registers)
2014-12-09 15:28 [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store charles.baylis
2014-12-09 15:28 ` [PATCH 3/4] vstN_lane error message enhancements (Q register) charles.baylis
2014-12-09 15:28 ` [PATCH 4/4] vstN_lane error message enhancements (D registers) charles.baylis
@ 2014-12-09 15:28 ` charles.baylis
2014-12-10 9:23 ` Christophe Lyon
2014-12-09 15:28 ` [PATCH 1/4] vldN_lane error message enhancements (Q registers) charles.baylis
2014-12-10 10:34 ` [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store Alan Lawrence
4 siblings, 1 reply; 11+ messages in thread
From: charles.baylis @ 2014-12-09 15:28 UTC (permalink / raw)
To: rearnsha, gcc-patches, marcus.shawcroft, tejas.belagod, alan.lawrence
From: Charles Baylis <charles.baylis@linaro.org>
gcc/ChangeLog
<DATE> Charles Baylis <charles.baylis@linaro.org>
* config/aarch64/arm_neon.h (__LD2_LANE_FUNC): Add explicit lane
bounds check.
(__LD3_LANE_FUNC): Likewise.
(__LD4_LANE_FUNC): Likewise
gcc/testsuite/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* gcc.target/aarch64/simd/vld4_lane.c: New test.
Change-Id: Ia95fbed34b50cf710ea9032ff3428a5f1432e0aa
---
gcc/config/aarch64/arm_neon.h | 6 ++++++
gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c | 15 +++++++++++++++
2 files changed, 21 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index 8cff719..22df564 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -17901,6 +17901,8 @@ vld2_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
__o = __builtin_aarch64_set_qregoi##mode (__o, \
(signedtype) __temp.val[1], \
1); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (vectype) / sizeof (*__ptr)); \
__o = __builtin_aarch64_ld2_lane##mode ( \
(__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
__b.val[0] = (vectype) __builtin_aarch64_get_dregoidi (__o, 0); \
@@ -17991,6 +17993,8 @@ vld3_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
__o = __builtin_aarch64_set_qregci##mode (__o, \
(signedtype) __temp.val[2], \
2); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (vectype) / sizeof (*__ptr)); \
__o = __builtin_aarch64_ld3_lane##mode ( \
(__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
__b.val[0] = (vectype) __builtin_aarch64_get_dregcidi (__o, 0); \
@@ -18089,6 +18093,8 @@ vld4_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
__o = __builtin_aarch64_set_qregxi##mode (__o, \
(signedtype) __temp.val[3], \
3); \
+ __builtin_aarch64_im_lane_boundsi (__c, \
+ sizeof (vectype) / sizeof (*__ptr)); \
__o = __builtin_aarch64_ld4_lane##mode ( \
(__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
__b.val[0] = (vectype) __builtin_aarch64_get_dregxidi (__o, 0); \
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
new file mode 100644
index 0000000..d14e6c1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
@@ -0,0 +1,15 @@
+/* Test error message when passing an invalid value as a lane index. */
+
+/* { dg-do compile } */
+
+#include <arm_neon.h>
+
+int8x8x4_t
+f_vld4_lane (int8_t * p, int8x8x4_t v)
+{
+ int8x8x4_t res;
+ /* { dg-error "lane 8 out of range 0 - 7" "" { target *-*-* } 0 } */
+ res = vld4_lane_s8 (p, v, 8);
+ return res;
+}
+
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] vldN_lane error message enhancements (D registers)
2014-12-09 15:28 ` [PATCH 2/4] vldN_lane " charles.baylis
@ 2014-12-10 9:23 ` Christophe Lyon
2014-12-10 10:26 ` Alan Lawrence
0 siblings, 1 reply; 11+ messages in thread
From: Christophe Lyon @ 2014-12-10 9:23 UTC (permalink / raw)
To: Charles Baylis
Cc: Richard Earnshaw, gcc-patches, Marcus Shawcroft, Tejas Belagod,
Alan Lawrence
On 9 December 2014 at 16:27, <charles.baylis@linaro.org> wrote:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> gcc/ChangeLog
>
> <DATE> Charles Baylis <charles.baylis@linaro.org>
>
> * config/aarch64/arm_neon.h (__LD2_LANE_FUNC): Add explicit lane
> bounds check.
> (__LD3_LANE_FUNC): Likewise.
> (__LD4_LANE_FUNC): Likewise
>
> gcc/testsuite/ChangeLog:
>
> <DATE> Charles Baylis <charles.baylis@linaro.org>
>
> * gcc.target/aarch64/simd/vld4_lane.c: New test.
>
> Change-Id: Ia95fbed34b50cf710ea9032ff3428a5f1432e0aa
> ---
> gcc/config/aarch64/arm_neon.h | 6 ++++++
> gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c | 15 +++++++++++++++
> 2 files changed, 21 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
>
> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
> index 8cff719..22df564 100644
> --- a/gcc/config/aarch64/arm_neon.h
> +++ b/gcc/config/aarch64/arm_neon.h
> @@ -17901,6 +17901,8 @@ vld2_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
> __o = __builtin_aarch64_set_qregoi##mode (__o, \
> (signedtype) __temp.val[1], \
> 1); \
> + __builtin_aarch64_im_lane_boundsi (__c, \
> + sizeof (vectype) / sizeof (*__ptr)); \
Shouldn't the arguments be reversed? (I'm looking at
__AARCH64_LANE_CHECK: the lane index is the 2nd parameter)
> __o = __builtin_aarch64_ld2_lane##mode ( \
> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
> __b.val[0] = (vectype) __builtin_aarch64_get_dregoidi (__o, 0); \
> @@ -17991,6 +17993,8 @@ vld3_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
> __o = __builtin_aarch64_set_qregci##mode (__o, \
> (signedtype) __temp.val[2], \
> 2); \
> + __builtin_aarch64_im_lane_boundsi (__c, \
> + sizeof (vectype) / sizeof (*__ptr)); \
> __o = __builtin_aarch64_ld3_lane##mode ( \
> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
> __b.val[0] = (vectype) __builtin_aarch64_get_dregcidi (__o, 0); \
> @@ -18089,6 +18093,8 @@ vld4_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
> __o = __builtin_aarch64_set_qregxi##mode (__o, \
> (signedtype) __temp.val[3], \
> 3); \
> + __builtin_aarch64_im_lane_boundsi (__c, \
> + sizeof (vectype) / sizeof (*__ptr)); \
> __o = __builtin_aarch64_ld4_lane##mode ( \
> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
> __b.val[0] = (vectype) __builtin_aarch64_get_dregxidi (__o, 0); \
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
> new file mode 100644
> index 0000000..d14e6c1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
> @@ -0,0 +1,15 @@
> +/* Test error message when passing an invalid value as a lane index. */
> +
> +/* { dg-do compile } */
> +
> +#include <arm_neon.h>
> +
> +int8x8x4_t
> +f_vld4_lane (int8_t * p, int8x8x4_t v)
> +{
> + int8x8x4_t res;
> + /* { dg-error "lane 8 out of range 0 - 7" "" { target *-*-* } 0 } */
> + res = vld4_lane_s8 (p, v, 8);
> + return res;
> +}
> +
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] vldN_lane error message enhancements (D registers)
2014-12-10 9:23 ` Christophe Lyon
@ 2014-12-10 10:26 ` Alan Lawrence
0 siblings, 0 replies; 11+ messages in thread
From: Alan Lawrence @ 2014-12-10 10:26 UTC (permalink / raw)
To: Christophe Lyon
Cc: Charles Baylis, Richard Earnshaw, gcc-patches, Marcus Shawcroft,
Tejas Belagod
Hmmm. Yes I think I may have switched that in the patch introducing
__AARCH64_LANE_CHECK, and it was correct at time of Charles' writing. However,
maybe we could (now) use __AARCH64_LANE_CHECK directly? (Referencing one of the
component vectors in the blahLxMxN_t struct?)
--Alan
Christophe Lyon wrote:
> On 9 December 2014 at 16:27, <charles.baylis@linaro.org> wrote:
>> From: Charles Baylis <charles.baylis@linaro.org>
>>
>> gcc/ChangeLog
>>
>> <DATE> Charles Baylis <charles.baylis@linaro.org>
>>
>> * config/aarch64/arm_neon.h (__LD2_LANE_FUNC): Add explicit lane
>> bounds check.
>> (__LD3_LANE_FUNC): Likewise.
>> (__LD4_LANE_FUNC): Likewise
>>
>> gcc/testsuite/ChangeLog:
>>
>> <DATE> Charles Baylis <charles.baylis@linaro.org>
>>
>> * gcc.target/aarch64/simd/vld4_lane.c: New test.
>>
>> Change-Id: Ia95fbed34b50cf710ea9032ff3428a5f1432e0aa
>> ---
>> gcc/config/aarch64/arm_neon.h | 6 ++++++
>> gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c | 15 +++++++++++++++
>> 2 files changed, 21 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
>>
>> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
>> index 8cff719..22df564 100644
>> --- a/gcc/config/aarch64/arm_neon.h
>> +++ b/gcc/config/aarch64/arm_neon.h
>> @@ -17901,6 +17901,8 @@ vld2_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
>> __o = __builtin_aarch64_set_qregoi##mode (__o, \
>> (signedtype) __temp.val[1], \
>> 1); \
>> + __builtin_aarch64_im_lane_boundsi (__c, \
>> + sizeof (vectype) / sizeof (*__ptr)); \
>
> Shouldn't the arguments be reversed? (I'm looking at
> __AARCH64_LANE_CHECK: the lane index is the 2nd parameter)
>
>> __o = __builtin_aarch64_ld2_lane##mode ( \
>> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
>> __b.val[0] = (vectype) __builtin_aarch64_get_dregoidi (__o, 0); \
>> @@ -17991,6 +17993,8 @@ vld3_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
>> __o = __builtin_aarch64_set_qregci##mode (__o, \
>> (signedtype) __temp.val[2], \
>> 2); \
>> + __builtin_aarch64_im_lane_boundsi (__c, \
>> + sizeof (vectype) / sizeof (*__ptr)); \
>> __o = __builtin_aarch64_ld3_lane##mode ( \
>> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
>> __b.val[0] = (vectype) __builtin_aarch64_get_dregcidi (__o, 0); \
>> @@ -18089,6 +18093,8 @@ vld4_lane_##funcsuffix (const ptrtype * __ptr, intype __b, const int __c) \
>> __o = __builtin_aarch64_set_qregxi##mode (__o, \
>> (signedtype) __temp.val[3], \
>> 3); \
>> + __builtin_aarch64_im_lane_boundsi (__c, \
>> + sizeof (vectype) / sizeof (*__ptr)); \
>> __o = __builtin_aarch64_ld4_lane##mode ( \
>> (__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c); \
>> __b.val[0] = (vectype) __builtin_aarch64_get_dregxidi (__o, 0); \
>> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
>> new file mode 100644
>> index 0000000..d14e6c1
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c
>> @@ -0,0 +1,15 @@
>> +/* Test error message when passing an invalid value as a lane index. */
>> +
>> +/* { dg-do compile } */
>> +
>> +#include <arm_neon.h>
>> +
>> +int8x8x4_t
>> +f_vld4_lane (int8_t * p, int8x8x4_t v)
>> +{
>> + int8x8x4_t res;
>> + /* { dg-error "lane 8 out of range 0 - 7" "" { target *-*-* } 0 } */
>> + res = vld4_lane_s8 (p, v, 8);
>> + return res;
>> +}
>> +
>> --
>> 1.9.1
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] vldN_lane error message enhancements (Q registers)
2014-12-09 15:28 [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store charles.baylis
` (2 preceding siblings ...)
2014-12-09 15:28 ` [PATCH 2/4] vldN_lane " charles.baylis
@ 2014-12-09 15:28 ` charles.baylis
[not found] ` <CAOckXuMSZyHU80A6-VjSzZ7bHrtRikTKwn-45DZ9oPPuRX_0HQ@mail.gmail.com>
2014-12-10 10:34 ` [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store Alan Lawrence
4 siblings, 1 reply; 11+ messages in thread
From: charles.baylis @ 2014-12-09 15:28 UTC (permalink / raw)
To: rearnsha, gcc-patches, marcus.shawcroft, tejas.belagod, alan.lawrence
From: Charles Baylis <charles.baylis@linaro.org>
gcc/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
PR target/63870
* config/aarch64/aarch64-builtins.c (enum aarch64_type_qualifiers):
Add qualifier_struct_load_store_lane_index.
(aarch64_types_loadstruct_lane_qualifiers): Use
qualifier_struct_load_store_lane_index for lane index argument for
last argument.
(builtin_simd_arg): Add SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX.
(aarch64_simd_expand_args): Add new argument describing mode of
builtin. Check lane bounds for arguments with
SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX.
(aarch64_simd_expand_builtin): Emit error for incorrect lane indices
if marked with SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX.
(aarch64_simd_expand_builtin): Pass machine mode of builtin to
aarch64_simd_expand_args.
* config/aarch64/aarch64-simd.md: (aarch64_ld2_lane<mode>): Remove
lane bounds check. Adjust lane numbers for big-endian.
(aarch64_ld3_lane<mode>): Likewise.
(aarch64_ld4_lane<mode>): Likewise.
gcc/testsuite/ChangeLog:
<DATE> Charles Baylis <charles.baylis@linaro.org>
* gcc.target/aarch64/simd/vld4q_lane.c: New test.
Change-Id: Ib17adaf64e631cf8d00a1a1a6c12409d2d7f4239
---
gcc/config/aarch64/aarch64-builtins.c | 30 +++++++++++++++++++---
gcc/config/aarch64/aarch64-simd.md | 12 ++++-----
gcc/testsuite/gcc.target/aarch64/simd/vld4q_lane.c | 16 ++++++++++++
3 files changed, 48 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vld4q_lane.c
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index aac7269..27046e2 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -116,7 +116,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
};
typedef struct
@@ -224,7 +226,7 @@ aarch64_types_load1_qualifiers[SIMD_MAX_BUILTIN_ARGS]
static enum aarch64_type_qualifiers
aarch64_types_loadstruct_lane_qualifiers[SIMD_MAX_BUILTIN_ARGS]
= { qualifier_none, qualifier_const_pointer_map_mode,
- qualifier_none, qualifier_none };
+ qualifier_none, qualifier_struct_load_store_lane_index };
#define TYPES_LOADSTRUCT_LANE (aarch64_types_loadstruct_lane_qualifiers)
static enum aarch64_type_qualifiers
@@ -859,12 +861,14 @@ typedef enum
SIMD_ARG_COPY_TO_REG,
SIMD_ARG_CONSTANT,
SIMD_ARG_LANE_INDEX,
+ SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX,
SIMD_ARG_STOP
} builtin_simd_arg;
static rtx
aarch64_simd_expand_args (rtx target, int icode, int have_retval,
- tree exp, builtin_simd_arg *args)
+ tree exp, builtin_simd_arg *args,
+ enum machine_mode builtin_mode)
{
rtx pat;
rtx op[SIMD_MAX_BUILTIN_ARGS + 1]; /* First element for result operand. */
@@ -903,6 +907,21 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval,
op[opc] = copy_to_mode_reg (mode, op[opc]);
break;
+ case SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX:
+ /* we expect arguments in order (ptr, array_of_vector, lane), and
+ we have to grub around in the ptr to find the lane size */
+ gcc_assert (opc > 1);
+ if (CONST_INT_P (op[opc]))
+ {
+ aarch64_simd_lane_bounds (op[opc], 0,
+ GET_MODE_NUNITS (builtin_mode),
+ exp);
+ /* Keep to GCC-vector-extension lane indices in the RTL. */
+ op[opc] =
+ GEN_INT (ENDIAN_LANE_N (builtin_mode, INTVAL (op[opc])));
+ }
+ goto constant_arg;
+
case SIMD_ARG_LANE_INDEX:
/* Must be a previous operand into which this is an index. */
gcc_assert (opc > 0);
@@ -917,6 +936,7 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval,
/* Fall through - if the lane index isn't a constant then
the next case will error. */
case SIMD_ARG_CONSTANT:
+constant_arg:
if (!(*insn_data[icode].operand[opc].predicate)
(op[opc], mode))
{
@@ -1003,6 +1023,8 @@ aarch64_simd_expand_builtin (int fcode, tree exp, rtx target)
if (d->qualifiers[qualifiers_k] & qualifier_lane_index)
args[k] = SIMD_ARG_LANE_INDEX;
+ else if (d->qualifiers[qualifiers_k] & qualifier_struct_load_store_lane_index)
+ args[k] = SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX;
else if (d->qualifiers[qualifiers_k] & qualifier_immediate)
args[k] = SIMD_ARG_CONSTANT;
else if (d->qualifiers[qualifiers_k] & qualifier_maybe_immediate)
@@ -1026,7 +1048,7 @@ aarch64_simd_expand_builtin (int fcode, tree exp, rtx target)
/* The interface to aarch64_simd_expand_args expects a 0 if
the function is void, and a 1 if it is not. */
return aarch64_simd_expand_args
- (target, icode, !is_void, exp, &args[1]);
+ (target, icode, !is_void, exp, &args[1], d->mode);
}
rtx
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 0ec1323..beac497 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -4397,8 +4397,8 @@
machine_mode mode = <V_TWO_ELEM>mode;
rtx mem = gen_rtx_MEM (mode, operands[1]);
- aarch64_simd_lane_bounds (operands[3], 0, GET_MODE_NUNITS (<VCONQ>mode),
- NULL);
+ operands[3] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[3])));
+
emit_insn (gen_aarch64_vec_load_lanesoi_lane<mode> (operands[0],
mem,
operands[2],
@@ -4417,8 +4417,8 @@
machine_mode mode = <V_THREE_ELEM>mode;
rtx mem = gen_rtx_MEM (mode, operands[1]);
- aarch64_simd_lane_bounds (operands[3], 0, GET_MODE_NUNITS (<VCONQ>mode),
- NULL);
+ operands[3] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[3])));
+
emit_insn (gen_aarch64_vec_load_lanesci_lane<mode> (operands[0],
mem,
operands[2],
@@ -4437,8 +4437,8 @@
machine_mode mode = <V_FOUR_ELEM>mode;
rtx mem = gen_rtx_MEM (mode, operands[1]);
- aarch64_simd_lane_bounds (operands[3], 0, GET_MODE_NUNITS (<VCONQ>mode),
- NULL);
+ operands[3] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[3])));
+
emit_insn (gen_aarch64_vec_load_lanesxi_lane<mode> (operands[0],
mem,
operands[2],
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vld4q_lane.c b/gcc/testsuite/gcc.target/aarch64/simd/vld4q_lane.c
new file mode 100644
index 0000000..c673ded
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vld4q_lane.c
@@ -0,0 +1,16 @@
+/* Test error message when passing an invalid value as a lane index. */
+
+/* { dg-do compile } */
+
+#include <arm_neon.h>
+
+/* { dg-error "lane 8 out of range 0 - 7" "" { target *-*-* } 0 } */
+int16x8x4_t
+f_vld4_lane (int16_t * p, int16x8x4_t v)
+{
+ int16x8x4_t res;
+ res = vld4q_lane_s16 (p, v, 8);
+ return res;
+}
+
+
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store
2014-12-09 15:28 [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store charles.baylis
` (3 preceding siblings ...)
2014-12-09 15:28 ` [PATCH 1/4] vldN_lane error message enhancements (Q registers) charles.baylis
@ 2014-12-10 10:34 ` Alan Lawrence
2014-12-12 14:12 ` Charles Baylis
4 siblings, 1 reply; 11+ messages in thread
From: Alan Lawrence @ 2014-12-10 10:34 UTC (permalink / raw)
To: charles.baylis
Cc: Richard Earnshaw, gcc-patches, Marcus Shawcroft, Tejas Belagod
Thanks, Charles. A couple of thoughts.
I think the approach in patches 2+3+4 of using __builtin_aarch64_im_lane_boundsi
is justified and works quite neatly. Modulo the question of argument ordering
and __AARCH64_LANE_CHECK, those patches look good.
However, the SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX, seems a lot of
infrastructure to introduce if we are only going to use it in one place, and I
think I might argue in favour of using ...__im_lane_bound or AARCH64_LANE_CHECK
there also. Of course all of this palaver stems from using the same builtins for
both D- and Q-reg intrinsics, and I suspect some cleanup may be due to those
intrinsics *at some point*, but probably not in time for gcc 5.0. However, this
does mean that if I use a D-reg intrinsic with a lane index that's out of bounds
for the Q-reg too, I get a double error message: e.g. for testcase
int8x8x4_t
f_vld4_lane (int8_t * p, int8x8x4_t v)
{
int8x8x4_t res;
return vld4_lane_s8 (p, v, 18);
}
I get output:
In file included from gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c:5:0:
.../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h: In function
'f_vld4_lane':
.../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h:18123:1: error:
lane 18 out of range 0 - 7
__LD4_LANE_FUNC (int8x8x4_t, int8x8_t, int8x16x4_t, int8_t, v16qi, qi, s8,
^
In function 'vld4_lane_s8',
inlined from 'f_vld4_lane' at
gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c:12:7:
.../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h:18123:1: error:
lane 18 out of range 0 - 15
__LD4_LANE_FUNC (int8x8x4_t, int8x8_t, int8x16x4_t, int8_t, v16qi, qi, s8,
^
which (although not serious) could be mildly confusing.
--Alan
charles.baylis@linaro.org wrote:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> This patch series moves the checking of lane indices for vld[234](q?)_lane and
> vst[234](q?)_lane intrinsics so that it occurs during builtin expansion.
>
> The q register variants are checked directly, but since the d register variants
> use the same intrinsics, these are checked in arm_neon.h using
> __builtin_aarch64_im_land_boundsi().
>
> Tested with make check-gcc on aarch64-oe-linux, with no regressions.
>
> Charles Baylis (4):
> vldN_lane error message enhancements (Q registers)
> vldN_lane error message enhancements (D registers)
> vstN_lane error message enhancements (Q register)
> vstN_lane error message enhancements (D registers)
>
> gcc/config/aarch64/aarch64-builtins.c | 32 +++++++++++++++++++++++++++-----
> gcc/config/aarch64/aarch64-simd.md | 12 ++++++------
> gcc/config/aarch64/arm_neon.h | 12 ++++++++++++
> 3 files changed, 45 insertions(+), 11 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store
2014-12-10 10:34 ` [PATCH 0/4] [AARCH64,SIMD] PR63870 Improve error messages for single lane load/store Alan Lawrence
@ 2014-12-12 14:12 ` Charles Baylis
0 siblings, 0 replies; 11+ messages in thread
From: Charles Baylis @ 2014-12-12 14:12 UTC (permalink / raw)
To: Alan Lawrence
Cc: Richard Earnshaw, gcc-patches, Marcus Shawcroft, Tejas Belagod
On 10 December 2014 at 10:34, Alan Lawrence <alan.lawrence@arm.com> wrote:
> Thanks, Charles. A couple of thoughts.
>
> I think the approach in patches 2+3+4 of using
> __builtin_aarch64_im_lane_boundsi is justified and works quite neatly.
> Modulo the question of argument ordering and __AARCH64_LANE_CHECK, those
> patches look good.
>
> However, the SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX, seems a lot of
> infrastructure to introduce if we are only going to use it in one place, and
> I think I might argue in favour of using ...__im_lane_bound or
> AARCH64_LANE_CHECK there also. Of course all of this palaver stems from
> using the same builtins for both D- and Q-reg intrinsics, and I suspect some
> cleanup may be due to those intrinsics *at some point*, but probably not in
> time for gcc 5.0.
>
> However, this does mean that if I use a D-reg intrinsic
> with a lane index that's out of bounds for the Q-reg too, I get a double
> error message: e.g. for testcase
>
> int8x8x4_t
> f_vld4_lane (int8_t * p, int8x8x4_t v)
> {
> int8x8x4_t res;
> return vld4_lane_s8 (p, v, 18);
> }
>
> I get output:
>
> In file included from gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c:5:0:
> .../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h: In function
> 'f_vld4_lane':
> .../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h:18123:1:
> error: lane 18 out of range 0 - 7
> __LD4_LANE_FUNC (int8x8x4_t, int8x8_t, int8x16x4_t, int8_t, v16qi, qi, s8,
> ^
> In function 'vld4_lane_s8',
> inlined from 'f_vld4_lane' at
> gcc/testsuite/gcc.target/aarch64/simd/vld4_lane.c:12:7:
> .../install/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h:18123:1:
> error: lane 18 out of range 0 - 15
> __LD4_LANE_FUNC (int8x8x4_t, int8x8_t, int8x16x4_t, int8_t, v16qi, qi, s8,
> ^
>
> which (although not serious) could be mildly confusing.
Oh dear, this is rather sad. Aesthetically, I think the builtins
should protect themselves from direct misuse, but I can't think of a
clean way to prevent this.
It could be done like this, but I don't think the end result really
justifies it.
__o = __builtin_aarch64_ld4_lane##mode
((__builtin_aarch64_simd_##ptrmode *) __ptr, __o, __c &
(__NUMBER_OF_LANES(__b.val[0]) - 1));
^ permalink raw reply [flat|nested] 11+ messages in thread