* [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel
@ 2019-06-03 16:57 Richard Henderson
2019-06-03 19:41 ` David Hildenbrand
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2019-06-03 16:57 UTC (permalink / raw)
To: qemu-devel; +Cc: cohuck, david
This replaces the target-specific implementations for VSEL.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/translate_vx.inc.c | 38 ++++++---------------------------
1 file changed, 6 insertions(+), 32 deletions(-)
diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 7e0bfcb190..a8603cbfd6 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -233,6 +233,9 @@ static void get_vec_element_ptr_i64(TCGv_ptr ptr, uint8_t reg, TCGv_i64 enr,
#define gen_gvec_fn_3(fn, es, v1, v2, v3) \
tcg_gen_gvec_##fn(es, vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
vec_full_reg_offset(v3), 16, 16)
+#define gen_gvec_fn_4(fn, es, v1, v2, v3, v4) \
+ tcg_gen_gvec_##fn(es, vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
+ vec_full_reg_offset(v3), vec_full_reg_offset(v4), 16, 16)
/*
* Helper to carry out a 128 bit vector computation using 2 i64 values per
@@ -903,40 +906,11 @@ static DisasJumpType op_vsce(DisasContext *s, DisasOps *o)
return DISAS_NEXT;
}
-static void gen_sel_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b, TCGv_i64 c)
-{
- TCGv_i64 t = tcg_temp_new_i64();
-
- /* bit in c not set -> copy bit from b */
- tcg_gen_andc_i64(t, b, c);
- /* bit in c set -> copy bit from a */
- tcg_gen_and_i64(d, a, c);
- /* merge the results */
- tcg_gen_or_i64(d, d, t);
- tcg_temp_free_i64(t);
-}
-
-static void gen_sel_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b,
- TCGv_vec c)
-{
- TCGv_vec t = tcg_temp_new_vec_matching(d);
-
- tcg_gen_andc_vec(vece, t, b, c);
- tcg_gen_and_vec(vece, d, a, c);
- tcg_gen_or_vec(vece, d, d, t);
- tcg_temp_free_vec(t);
-}
-
static DisasJumpType op_vsel(DisasContext *s, DisasOps *o)
{
- static const GVecGen4 gvec_op = {
- .fni8 = gen_sel_i64,
- .fniv = gen_sel_vec,
- .prefer_i64 = TCG_TARGET_REG_BITS == 64,
- };
-
- gen_gvec_4(get_field(s->fields, v1), get_field(s->fields, v2),
- get_field(s->fields, v3), get_field(s->fields, v4), &gvec_op);
+ gen_gvec_fn_4(bitsel, ES_8, get_field(s->fields, v1),
+ get_field(s->fields, v4), get_field(s->fields, v2),
+ get_field(s->fields, v3));
return DISAS_NEXT;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel
2019-06-03 16:57 [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel Richard Henderson
@ 2019-06-03 19:41 ` David Hildenbrand
2019-06-03 21:59 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: David Hildenbrand @ 2019-06-03 19:41 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: cohuck
On 03.06.19 18:57, Richard Henderson wrote:
> This replaces the target-specific implementations for VSEL.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/translate_vx.inc.c | 38 ++++++---------------------------
> 1 file changed, 6 insertions(+), 32 deletions(-)
>
> diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
> index 7e0bfcb190..a8603cbfd6 100644
> --- a/target/s390x/translate_vx.inc.c
> +++ b/target/s390x/translate_vx.inc.c
> @@ -233,6 +233,9 @@ static void get_vec_element_ptr_i64(TCGv_ptr ptr, uint8_t reg, TCGv_i64 enr,
> #define gen_gvec_fn_3(fn, es, v1, v2, v3) \
> tcg_gen_gvec_##fn(es, vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
> vec_full_reg_offset(v3), 16, 16)
> +#define gen_gvec_fn_4(fn, es, v1, v2, v3, v4) \
> + tcg_gen_gvec_##fn(es, vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
> + vec_full_reg_offset(v3), vec_full_reg_offset(v4), 16, 16)
>
> /*
> * Helper to carry out a 128 bit vector computation using 2 i64 values per
> @@ -903,40 +906,11 @@ static DisasJumpType op_vsce(DisasContext *s, DisasOps *o)
> return DISAS_NEXT;
> }
>
> -static void gen_sel_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b, TCGv_i64 c)
> -{
> - TCGv_i64 t = tcg_temp_new_i64();
> -
> - /* bit in c not set -> copy bit from b */
> - tcg_gen_andc_i64(t, b, c);
> - /* bit in c set -> copy bit from a */
> - tcg_gen_and_i64(d, a, c);
> - /* merge the results */
> - tcg_gen_or_i64(d, d, t);
> - tcg_temp_free_i64(t);
> -}
> -
> -static void gen_sel_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b,
> - TCGv_vec c)
> -{
> - TCGv_vec t = tcg_temp_new_vec_matching(d);
> -
> - tcg_gen_andc_vec(vece, t, b, c);
> - tcg_gen_and_vec(vece, d, a, c);
> - tcg_gen_or_vec(vece, d, d, t);
> - tcg_temp_free_vec(t);
> -}
> -
Comparing against tcg_gen_bitsel_i64()
1. a and c are switched
2. b is _not_ switched (and() and andc() are switched)
Shouldn't this be
gen_gvec_fn_4(bitsel, ES_8, get_field(s->fields, v1),
get_field(s->fields, v4), get_field(s->fields, v3),
get_field(s->fields, v2));
?
Maybe I am missing something. It was a long day :)
Should I send this patch with the next s390x/tcg pull request?
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel
2019-06-03 19:41 ` David Hildenbrand
@ 2019-06-03 21:59 ` Richard Henderson
2019-06-04 7:44 ` David Hildenbrand
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2019-06-03 21:59 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel; +Cc: cohuck
On 6/3/19 2:41 PM, David Hildenbrand wrote:
>> -static void gen_sel_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b,
>> - TCGv_vec c)
>> -{
>> - TCGv_vec t = tcg_temp_new_vec_matching(d);
>> -
>> - tcg_gen_andc_vec(vece, t, b, c);
>> - tcg_gen_and_vec(vece, d, a, c);
>> - tcg_gen_or_vec(vece, d, d, t);
>> - tcg_temp_free_vec(t);
>> -}
>> -
>
> Comparing against tcg_gen_bitsel_i64()
>
> 1. a and c are switched
> 2. b is _not_ switched (and() and andc() are switched)
Not quite. {a,b,c} from your s390 implementation becomes {c,a,b} for tcg.
Running tests would show for sure; I guess you have those later in your vx
patch set?
> Should I send this patch with the next s390x/tcg pull request?
Yes please.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel
2019-06-03 21:59 ` Richard Henderson
@ 2019-06-04 7:44 ` David Hildenbrand
0 siblings, 0 replies; 4+ messages in thread
From: David Hildenbrand @ 2019-06-04 7:44 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: cohuck
On 03.06.19 23:59, Richard Henderson wrote:
> On 6/3/19 2:41 PM, David Hildenbrand wrote:
>>> -static void gen_sel_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b,
>>> - TCGv_vec c)
>>> -{
>>> - TCGv_vec t = tcg_temp_new_vec_matching(d);
>>> -
>>> - tcg_gen_andc_vec(vece, t, b, c);
>>> - tcg_gen_and_vec(vece, d, a, c);
>>> - tcg_gen_or_vec(vece, d, d, t);
>>> - tcg_temp_free_vec(t);
>>> -}
>>> -
>>
>> Comparing against tcg_gen_bitsel_i64()
>>
>> 1. a and c are switched
>> 2. b is _not_ switched (and() and andc() are switched)
>
> Not quite. {a,b,c} from your s390 implementation becomes {c,a,b} for tcg.
>
> Running tests would show for sure; I guess you have those later in your vx
> patch set?
I only have a small selection of tests for now
84fa6254e4 tests/tcg: target/s390x: Test VECTOR ADD *
9e9a9e5246 tests/tcg: target/s390x: Test VECTOR UNPACK *
9d2b7184c6 tests/tcg: target/s390x: Test VECTOR PACK *
4872c1b6bd tests/tcg: target/s390x: Test VECTOR MERGE (HIGH|LOW)
914502d1d1 tests/tcg: target/s390x: Test VECTOR LOAD AND REPLICATE
7d01bbca17 tests/tcg: target/s390x: Test VECTOR GENERATE MASK
dc107ebdf7 tests/tcg: target/s390x: Test VECTOR GENERATE BYTE MASK
17212a732d tests/tcg: target/s390x: Test VECTOR LOAD GR FROM VR ELEMENT
f120e93c15 tests/tcg: target/s390x: Test VECTOR GATHER ELEMENT
But I just added a simple
6c43fe6a8c tests/tcg: target/s390x: Test VECTOR SELECT
That test confirmed that your implementation works correctly. :)
>
>> Should I send this patch with the next s390x/tcg pull request?
>
> Yes please.
I'll change the subject to "s390x/tcg: Use tcg_gen_gvec_bitsel for
VECTOR SELECT" if you don't object. Thanks!
>
>
> r~
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-04 7:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 16:57 [Qemu-devel] [PATCH] target/s390x: Use tcg_gen_gvec_bitsel Richard Henderson
2019-06-03 19:41 ` David Hildenbrand
2019-06-03 21:59 ` Richard Henderson
2019-06-04 7:44 ` David Hildenbrand
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.