* [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
@ 2020-04-23 16:42 Stephen Long
2020-04-24 21:37 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Long @ 2020-04-23 16:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, richard.henderson, apazos
Signed-off-by: Stephen Long <steplong@quicinc.com>
These insns don't show up under any SVE2 categories in the manual. But
if you lookup each insn, you'll find they have SVE2 variants.
---
target/arm/helper-sve.h | 10 +++++++
target/arm/sve.decode | 5 ++++
target/arm/sve_helper.c | 53 ++++++++++++++++++++++++++++++++++++++
target/arm/translate-sve.c | 20 ++++++++++++++
4 files changed, 88 insertions(+)
diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index f6ae814021..54d20575e8 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2687,3 +2687,13 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_s, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_d, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_5(sve2_tbl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_tbl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_tbl_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_tbl_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_tbx_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(sve2_tbx_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(sve2_tbx_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(sve2_tbx_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 3a2a4a7f1c..483fbf0dcc 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1387,3 +1387,8 @@ UMLSLT_zzzw 01000100 .. 0 ..... 010 111 ..... ..... @rda_rn_rm
CMLA_zzzz 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
SQRDCMLAH_zzzz 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5 ra=%reg_movprfx
+
+### SVE2 Table Lookup (three sources)
+
+TBL_zzz 00000101 .. 1 ..... 00101 0 ..... ..... @rd_rn_rm
+TBX_zzz 00000101 .. 1 ..... 00101 1 ..... ..... @rd_rn_rm
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 55e2c32f03..d1e91da02a 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -2968,6 +2968,59 @@ DO_TBL(sve_tbl_d, uint64_t, )
#undef TBL
+#define DO_SVE2_TBL(NAME, TYPE, H) \
+void HELPER(NAME)(void *vd, void *vn1, void *vm, void *vn2, uint32_t desc) \
+{ \
+ intptr_t i, opr_sz = simd_oprsz(desc); \
+ uintptr_t elem = opr_sz / sizeof(TYPE); \
+ TYPE *d = vd, *n1 = vn1, *n2 = vn2, *m = vm; \
+ ARMVectorReg tmp1, tmp2; \
+ if (unlikely(vd == vn1)) { \
+ n1 = memcpy(&tmp1, vn1, opr_sz); \
+ } \
+ if (unlikely(vd == vn2)) { \
+ n2 = memcpy(&tmp2, vn2, opr_sz); \
+ } \
+ for (i = 0; i < elem; i++) { \
+ TYPE j = m[H(i)]; \
+ d[H(i)] = j < (elem * 2) ? n1[H(j)] : 0; \
+ \
+ TYPE k = m[H(elem + i)]; \
+ d[H(elem + i)] = k < (elem * 2) ? n2[H(k)] : 0; \
+ } \
+}
+
+DO_SVE2_TBL(sve2_tbl_b, uint8_t, H1)
+DO_SVE2_TBL(sve2_tbl_h, uint16_t, H2)
+DO_SVE2_TBL(sve2_tbl_s, uint32_t, H4)
+DO_SVE2_TBL(sve2_tbl_d, uint64_t, )
+
+#define DO_SVE2_TBX(NAME, TYPE, H) \
+void HELPER(NAME)(void *vd, void *vn, void *vm, uint32_t desc) \
+{ \
+ intptr_t i, opr_sz = simd_oprsz(desc); \
+ uintptr_t elem = opr_sz / sizeof(TYPE); \
+ TYPE *d = vd, *n = vn, *m = vm; \
+ ARMVectorReg tmp; \
+ if (unlikely(vd == vn)) { \
+ n = memcpy(&tmp, vn, opr_sz); \
+ } \
+ for (i = 0; i < elem; i++) { \
+ TYPE j = m[H(i)]; \
+ if (j < elem) { \
+ d[H(i)] = n[H(j)]; \
+ } \
+ } \
+}
+
+DO_SVE2_TBX(sve2_tbx_b, uint8_t, H1)
+DO_SVE2_TBX(sve2_tbx_h, uint16_t, H2)
+DO_SVE2_TBX(sve2_tbx_s, uint32_t, H4)
+DO_SVE2_TBX(sve2_tbx_d, uint64_t, )
+
+#undef DO_SVE2_TBX
+#undef DO_SVE2_TBL
+
#define DO_UNPK(NAME, TYPED, TYPES, HD, HS) \
void HELPER(NAME)(void *vd, void *vn, uint32_t desc) \
{ \
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 20eb588cb3..c2694d92dd 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -7882,3 +7882,23 @@ static bool trans_SQRDCMLAH_zzzz(DisasContext *s, arg_CMLA_zzzz *a)
};
return do_sve2_zzzz_fn(s, a->rd, a->rn, a->rm, a->ra, fns[a->esz], a->rot);
}
+
+static bool trans_TBL_zzz(DisasContext *s, arg_rrr_esz *a)
+{
+ static gen_helper_gvec_4 * const fns[] = {
+ gen_helper_sve2_tbl_b, gen_helper_sve2_tbl_h,
+ gen_helper_sve2_tbl_s, gen_helper_sve2_tbl_d,
+ };
+ int rn1 = a->rn;
+ int rn2 = (a->rn + 1) % 32;
+ return do_sve2_zzzz_fn(s, a->rd, rn1, a->rm, rn2, fns[a->esz], 0);
+}
+
+static bool trans_TBX_zzz(DisasContext *s, arg_rrr_esz *a)
+{
+ static gen_helper_gvec_3 * const fns[] = {
+ gen_helper_sve2_tbx_b, gen_helper_sve2_tbx_h,
+ gen_helper_sve2_tbx_s, gen_helper_sve2_tbx_d,
+ };
+ return do_sve2_zzz_ool(s, a, fns[a->esz]);
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
2020-04-23 16:42 [PATCH RFC] target/arm: Implement SVE2 TBL, TBX Stephen Long
@ 2020-04-24 21:37 ` Richard Henderson
2020-04-24 22:47 ` Stephen Long
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2020-04-24 21:37 UTC (permalink / raw)
To: Stephen Long, qemu-devel; +Cc: qemu-arm, apazos
On 4/23/20 9:42 AM, Stephen Long wrote:
> Signed-off-by: Stephen Long <steplong@quicinc.com>
>
> These insns don't show up under any SVE2 categories in the manual. But
> if you lookup each insn, you'll find they have SVE2 variants.
> ---
> target/arm/helper-sve.h | 10 +++++++
> target/arm/sve.decode | 5 ++++
> target/arm/sve_helper.c | 53 ++++++++++++++++++++++++++++++++++++++
> target/arm/translate-sve.c | 20 ++++++++++++++
> 4 files changed, 88 insertions(+)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index f6ae814021..54d20575e8 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -2687,3 +2687,13 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_s, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
> DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_d, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_5(sve2_tbl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_5(sve2_tbl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_5(sve2_tbl_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_5(sve2_tbl_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_4(sve2_tbx_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_4(sve2_tbx_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_4(sve2_tbx_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
> +DEF_HELPER_FLAGS_4(sve2_tbx_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
> diff --git a/target/arm/sve.decode b/target/arm/sve.decode
> index 3a2a4a7f1c..483fbf0dcc 100644
> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -1387,3 +1387,8 @@ UMLSLT_zzzw 01000100 .. 0 ..... 010 111 ..... ..... @rda_rn_rm
>
> CMLA_zzzz 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
> SQRDCMLAH_zzzz 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5 ra=%reg_movprfx
> +
> +### SVE2 Table Lookup (three sources)
> +
> +TBL_zzz 00000101 .. 1 ..... 00101 0 ..... ..... @rd_rn_rm
> +TBX_zzz 00000101 .. 1 ..... 00101 1 ..... ..... @rd_rn_rm
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 55e2c32f03..d1e91da02a 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2968,6 +2968,59 @@ DO_TBL(sve_tbl_d, uint64_t, )
>
> #undef TBL
>
> +#define DO_SVE2_TBL(NAME, TYPE, H) \
> +void HELPER(NAME)(void *vd, void *vn1, void *vm, void *vn2, uint32_t desc) \
> +{ \
> + intptr_t i, opr_sz = simd_oprsz(desc); \
> + uintptr_t elem = opr_sz / sizeof(TYPE); \
> + TYPE *d = vd, *n1 = vn1, *n2 = vn2, *m = vm; \
> + ARMVectorReg tmp1, tmp2; \
Only one temp needed.
> + if (unlikely(vd == vn1)) { \
> + n1 = memcpy(&tmp1, vn1, opr_sz); \
> + } \
> + if (unlikely(vd == vn2)) { \
> + n2 = memcpy(&tmp2, vn2, opr_sz); \
> + }
Better with else if here.
Because vd cannot overlap both vn1 or vn2, only one of them.
\
> + for (i = 0; i < elem; i++) { \
> + TYPE j = m[H(i)]; \
> + d[H(i)] = j < (elem * 2) ? n1[H(j)] : 0; \
> + \
> + TYPE k = m[H(elem + i)]; \
> + d[H(elem + i)] = k < (elem * 2) ? n2[H(k)] : 0; \
> + }
First, the indexing is wrong.
Note that you're range checking vs elem * 2, but only indexing a single vector.
Thus you must be indexing into the next vector.
This should look more like
TYPE j = m[H(i)];
TYPE r = 0;
if (j < elem) {
r = n1[H(j)];
} else if (j < 2 * elem) {
r = n2[H(j - elem)];
}
d[H(i)] = r;
Second, this is one case where I'd prefer to share code with AArch64. It would
be worthwhile to rearrange both sve1 and advsimd to use a common set of helpers.
> +static bool trans_TBL_zzz(DisasContext *s, arg_rrr_esz *a)
_zzz is not helpful here. The SVE1 insn also operates on 3 registers, and thus
could logically be _zzz too.
Better might be _double, after double_table = TRUE, or maybe just _2 just in
case SVE3 adds a variant with more table registers.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
2020-04-24 21:37 ` Richard Henderson
@ 2020-04-24 22:47 ` Stephen Long
2020-04-25 0:25 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Long @ 2020-04-24 22:47 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Ana Pazos
Oh, maybe I misread the manual description for SVE2 TBL, but I thought Zm was the indexes register and the loop compares the index from Zm with the total number of elems, table_elems.
-----Original Message-----
From: Richard Henderson <richard.henderson@linaro.org>
Sent: Friday, April 24, 2020 2:37 PM
To: Stephen Long <steplong@quicinc.com>; qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org; Ana Pazos <apazos@quicinc.com>
Subject: [EXT] Re: [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
On 4/23/20 9:42 AM, Stephen Long wrote:
> Signed-off-by: Stephen Long <steplong@quicinc.com>
>
> These insns don't show up under any SVE2 categories in the manual. But
> if you lookup each insn, you'll find they have SVE2 variants.
> ---
> target/arm/helper-sve.h | 10 +++++++
> target/arm/sve.decode | 5 ++++
> target/arm/sve_helper.c | 53 ++++++++++++++++++++++++++++++++++++++
> target/arm/translate-sve.c | 20 ++++++++++++++
> 4 files changed, 88 insertions(+)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h index
> f6ae814021..54d20575e8 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -2687,3 +2687,13 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_s, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
> DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_d, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_5(sve2_tbl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr,
> +ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_h, TCG_CALL_NO_RWG, void, ptr,
> +ptr, ptr, ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_s, TCG_CALL_NO_RWG,
> +void, ptr, ptr, ptr, ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_d,
> +TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_4(sve2_tbx_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr,
> +i32) DEF_HELPER_FLAGS_4(sve2_tbx_h, TCG_CALL_NO_RWG, void, ptr, ptr,
> +ptr, i32) DEF_HELPER_FLAGS_4(sve2_tbx_s, TCG_CALL_NO_RWG, void, ptr,
> +ptr, ptr, i32) DEF_HELPER_FLAGS_4(sve2_tbx_d, TCG_CALL_NO_RWG, void,
> +ptr, ptr, ptr, i32)
> diff --git a/target/arm/sve.decode b/target/arm/sve.decode index
> 3a2a4a7f1c..483fbf0dcc 100644
> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -1387,3 +1387,8 @@ UMLSLT_zzzw 01000100 .. 0 ..... 010 111 ..... ..... @rda_rn_rm
>
> CMLA_zzzz 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
> SQRDCMLAH_zzzz 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5
> ra=%reg_movprfx
> +
> +### SVE2 Table Lookup (three sources)
> +
> +TBL_zzz 00000101 .. 1 ..... 00101 0 ..... ..... @rd_rn_rm
> +TBX_zzz 00000101 .. 1 ..... 00101 1 ..... ..... @rd_rn_rm
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c index
> 55e2c32f03..d1e91da02a 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2968,6 +2968,59 @@ DO_TBL(sve_tbl_d, uint64_t, )
>
> #undef TBL
>
> +#define DO_SVE2_TBL(NAME, TYPE, H) \
> +void HELPER(NAME)(void *vd, void *vn1, void *vm, void *vn2, uint32_t desc) \
> +{ \
> + intptr_t i, opr_sz = simd_oprsz(desc); \
> + uintptr_t elem = opr_sz / sizeof(TYPE); \
> + TYPE *d = vd, *n1 = vn1, *n2 = vn2, *m = vm; \
> + ARMVectorReg tmp1, tmp2; \
Only one temp needed.
> + if (unlikely(vd == vn1)) { \
> + n1 = memcpy(&tmp1, vn1, opr_sz); \
> + } \
> + if (unlikely(vd == vn2)) { \
> + n2 = memcpy(&tmp2, vn2, opr_sz); \
> + }
Better with else if here.
Because vd cannot overlap both vn1 or vn2, only one of them.
\
> + for (i = 0; i < elem; i++) { \
> + TYPE j = m[H(i)]; \
> + d[H(i)] = j < (elem * 2) ? n1[H(j)] : 0; \
> + \
> + TYPE k = m[H(elem + i)]; \
> + d[H(elem + i)] = k < (elem * 2) ? n2[H(k)] : 0; \
> + }
First, the indexing is wrong.
Note that you're range checking vs elem * 2, but only indexing a single vector.
Thus you must be indexing into the next vector.
This should look more like
TYPE j = m[H(i)];
TYPE r = 0;
if (j < elem) {
r = n1[H(j)];
} else if (j < 2 * elem) {
r = n2[H(j - elem)];
}
d[H(i)] = r;
Second, this is one case where I'd prefer to share code with AArch64. It would be worthwhile to rearrange both sve1 and advsimd to use a common set of helpers.
> +static bool trans_TBL_zzz(DisasContext *s, arg_rrr_esz *a)
_zzz is not helpful here. The SVE1 insn also operates on 3 registers, and thus could logically be _zzz too.
Better might be _double, after double_table = TRUE, or maybe just _2 just in case SVE3 adds a variant with more table registers.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
2020-04-24 22:47 ` Stephen Long
@ 2020-04-25 0:25 ` Richard Henderson
0 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2020-04-25 0:25 UTC (permalink / raw)
To: Stephen Long, qemu-devel; +Cc: qemu-arm, Ana Pazos
On 4/24/20 3:47 PM, Stephen Long wrote:
> Oh, maybe I misread the manual description for SVE2 TBL, but I thought Zm was the indexes register and the loop compares the index from Zm with the total number of elems, table_elems.
That's right. You take the index from Zm just fine, but fail to apply that
index properly across Zn and Zn+1.
r~
>
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, April 24, 2020 2:37 PM
> To: Stephen Long <steplong@quicinc.com>; qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org; Ana Pazos <apazos@quicinc.com>
> Subject: [EXT] Re: [PATCH RFC] target/arm: Implement SVE2 TBL, TBX
>
> On 4/23/20 9:42 AM, Stephen Long wrote:
>> Signed-off-by: Stephen Long <steplong@quicinc.com>
>>
>> These insns don't show up under any SVE2 categories in the manual. But
>> if you lookup each insn, you'll find they have SVE2 variants.
>> ---
>> target/arm/helper-sve.h | 10 +++++++
>> target/arm/sve.decode | 5 ++++
>> target/arm/sve_helper.c | 53 ++++++++++++++++++++++++++++++++++++++
>> target/arm/translate-sve.c | 20 ++++++++++++++
>> 4 files changed, 88 insertions(+)
>>
>> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h index
>> f6ae814021..54d20575e8 100644
>> --- a/target/arm/helper-sve.h
>> +++ b/target/arm/helper-sve.h
>> @@ -2687,3 +2687,13 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_s, TCG_CALL_NO_RWG,
>> void, ptr, ptr, ptr, ptr, i32)
>> DEF_HELPER_FLAGS_5(sve2_sqrdcmlah_zzzz_d, TCG_CALL_NO_RWG,
>> void, ptr, ptr, ptr, ptr, i32)
>> +
>> +DEF_HELPER_FLAGS_5(sve2_tbl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr,
>> +ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_h, TCG_CALL_NO_RWG, void, ptr,
>> +ptr, ptr, ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_s, TCG_CALL_NO_RWG,
>> +void, ptr, ptr, ptr, ptr, i32) DEF_HELPER_FLAGS_5(sve2_tbl_d,
>> +TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
>> +
>> +DEF_HELPER_FLAGS_4(sve2_tbx_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr,
>> +i32) DEF_HELPER_FLAGS_4(sve2_tbx_h, TCG_CALL_NO_RWG, void, ptr, ptr,
>> +ptr, i32) DEF_HELPER_FLAGS_4(sve2_tbx_s, TCG_CALL_NO_RWG, void, ptr,
>> +ptr, ptr, i32) DEF_HELPER_FLAGS_4(sve2_tbx_d, TCG_CALL_NO_RWG, void,
>> +ptr, ptr, ptr, i32)
>> diff --git a/target/arm/sve.decode b/target/arm/sve.decode index
>> 3a2a4a7f1c..483fbf0dcc 100644
>> --- a/target/arm/sve.decode
>> +++ b/target/arm/sve.decode
>> @@ -1387,3 +1387,8 @@ UMLSLT_zzzw 01000100 .. 0 ..... 010 111 ..... ..... @rda_rn_rm
>>
>> CMLA_zzzz 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
>> SQRDCMLAH_zzzz 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5
>> ra=%reg_movprfx
>> +
>> +### SVE2 Table Lookup (three sources)
>> +
>> +TBL_zzz 00000101 .. 1 ..... 00101 0 ..... ..... @rd_rn_rm
>> +TBX_zzz 00000101 .. 1 ..... 00101 1 ..... ..... @rd_rn_rm
>> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c index
>> 55e2c32f03..d1e91da02a 100644
>> --- a/target/arm/sve_helper.c
>> +++ b/target/arm/sve_helper.c
>> @@ -2968,6 +2968,59 @@ DO_TBL(sve_tbl_d, uint64_t, )
>>
>> #undef TBL
>>
>> +#define DO_SVE2_TBL(NAME, TYPE, H) \
>> +void HELPER(NAME)(void *vd, void *vn1, void *vm, void *vn2, uint32_t desc) \
>> +{ \
>> + intptr_t i, opr_sz = simd_oprsz(desc); \
>> + uintptr_t elem = opr_sz / sizeof(TYPE); \
>> + TYPE *d = vd, *n1 = vn1, *n2 = vn2, *m = vm; \
>> + ARMVectorReg tmp1, tmp2; \
>
> Only one temp needed.
>
>> + if (unlikely(vd == vn1)) { \
>> + n1 = memcpy(&tmp1, vn1, opr_sz); \
>> + } \
>> + if (unlikely(vd == vn2)) { \
>> + n2 = memcpy(&tmp2, vn2, opr_sz); \
>> + }
>
> Better with else if here.
> Because vd cannot overlap both vn1 or vn2, only one of them.
> \
>> + for (i = 0; i < elem; i++) { \
>> + TYPE j = m[H(i)]; \
>> + d[H(i)] = j < (elem * 2) ? n1[H(j)] : 0; \
>> + \
>> + TYPE k = m[H(elem + i)]; \
>> + d[H(elem + i)] = k < (elem * 2) ? n2[H(k)] : 0; \
>> + }
>
> First, the indexing is wrong.
>
> Note that you're range checking vs elem * 2, but only indexing a single vector.
> Thus you must be indexing into the next vector.
>
> This should look more like
>
> TYPE j = m[H(i)];
> TYPE r = 0;
>
> if (j < elem) {
> r = n1[H(j)];
> } else if (j < 2 * elem) {
> r = n2[H(j - elem)];
> }
> d[H(i)] = r;
>
> Second, this is one case where I'd prefer to share code with AArch64. It would be worthwhile to rearrange both sve1 and advsimd to use a common set of helpers.
>
>> +static bool trans_TBL_zzz(DisasContext *s, arg_rrr_esz *a)
>
> _zzz is not helpful here. The SVE1 insn also operates on 3 registers, and thus could logically be _zzz too.
>
> Better might be _double, after double_table = TRUE, or maybe just _2 just in case SVE3 adds a variant with more table registers.
>
>
> r~
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-25 0:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 16:42 [PATCH RFC] target/arm: Implement SVE2 TBL, TBX Stephen Long
2020-04-24 21:37 ` Richard Henderson
2020-04-24 22:47 ` Stephen Long
2020-04-25 0:25 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.