1. Patchew
  2. QEMU
  3. [RFC PATCH v2 00/81] tcg: Merge *_i32 and *_i64 opcodes
  4. View patch

[PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec

Richard Henderson posted 81 patches 2 months, 4 weeks ago
[PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Richard Henderson 2 months, 4 weeks ago 
Rely on tcg-op-vec.c to expand the opcode if missing.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-sve.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
index 49d32fabc9..732453db6f 100644
--- a/target/arm/tcg/translate-sve.c
+++ b/target/arm/tcg/translate-sve.c
@@ -596,14 +596,8 @@ static void gen_bsl1n_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m, TCGv_i64 k)
 static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
                           TCGv_vec m, TCGv_vec k)
 {
-    if (TCG_TARGET_HAS_bitsel_vec) {
-        tcg_gen_not_vec(vece, n, n);
-        tcg_gen_bitsel_vec(vece, d, k, n, m);
-    } else {
-        tcg_gen_andc_vec(vece, n, k, n);
-        tcg_gen_andc_vec(vece, m, m, k);
-        tcg_gen_or_vec(vece, d, n, m);
-    }
+    tcg_gen_not_vec(vece, n, n);
+    tcg_gen_bitsel_vec(vece, d, k, n, m);
 }
 
 static void gen_bsl1n(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
@@ -640,14 +634,8 @@ static void gen_bsl2n_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m, TCGv_i64 k)
 static void gen_bsl2n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
                           TCGv_vec m, TCGv_vec k)
 {
-    if (TCG_TARGET_HAS_bitsel_vec) {
-        tcg_gen_not_vec(vece, m, m);
-        tcg_gen_bitsel_vec(vece, d, k, n, m);
-    } else {
-        tcg_gen_and_vec(vece, n, n, k);
-        tcg_gen_or_vec(vece, m, m, k);
-        tcg_gen_orc_vec(vece, d, n, m);
-    }
+    tcg_gen_not_vec(vece, m, m);
+    tcg_gen_bitsel_vec(vece, d, k, n, m);
 }
 
 static void gen_bsl2n(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
-- 
2.43.0
Re: [PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Philippe Mathieu-Daudé 2 months, 4 weeks ago 
On 7/1/25 08:59, Richard Henderson wrote:
> Rely on tcg-op-vec.c to expand the opcode if missing.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/tcg/translate-sve.c | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
> index 49d32fabc9..732453db6f 100644
> --- a/target/arm/tcg/translate-sve.c
> +++ b/target/arm/tcg/translate-sve.c
> @@ -596,14 +596,8 @@ static void gen_bsl1n_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m, TCGv_i64 k)
>   static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>                             TCGv_vec m, TCGv_vec k)
>   {
> -    if (TCG_TARGET_HAS_bitsel_vec) {
> -        tcg_gen_not_vec(vece, n, n);
> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
> -    } else {

Why aren't we doing the NOT n operation here?

> -        tcg_gen_andc_vec(vece, n, k, n);
> -        tcg_gen_andc_vec(vece, m, m, k);
> -        tcg_gen_or_vec(vece, d, n, m);
> -    }
> +    tcg_gen_not_vec(vece, n, n);
> +    tcg_gen_bitsel_vec(vece, d, k, n, m);
>   }
>   
>   static void gen_bsl1n(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
> @@ -640,14 +634,8 @@ static void gen_bsl2n_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m, TCGv_i64 k)
>   static void gen_bsl2n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>                             TCGv_vec m, TCGv_vec k)
>   {
> -    if (TCG_TARGET_HAS_bitsel_vec) {
> -        tcg_gen_not_vec(vece, m, m);
> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
> -    } else {
> -        tcg_gen_and_vec(vece, n, n, k);
> -        tcg_gen_or_vec(vece, m, m, k);
> -        tcg_gen_orc_vec(vece, d, n, m);
> -    }
> +    tcg_gen_not_vec(vece, m, m);
> +    tcg_gen_bitsel_vec(vece, d, k, n, m);
>   }
>   
>   static void gen_bsl2n(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
Re: [PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Richard Henderson 2 months, 4 weeks ago 
On 1/8/25 09:46, Philippe Mathieu-Daudé wrote:
> On 7/1/25 08:59, Richard Henderson wrote:
>> Rely on tcg-op-vec.c to expand the opcode if missing.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/tcg/translate-sve.c | 20 ++++----------------
>>   1 file changed, 4 insertions(+), 16 deletions(-)
>>
>> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
>> index 49d32fabc9..732453db6f 100644
>> --- a/target/arm/tcg/translate-sve.c
>> +++ b/target/arm/tcg/translate-sve.c
>> @@ -596,14 +596,8 @@ static void gen_bsl1n_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m, 
>> TCGv_i64 k)
>>   static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>>                             TCGv_vec m, TCGv_vec k)
>>   {
>> -    if (TCG_TARGET_HAS_bitsel_vec) {
>> -        tcg_gen_not_vec(vece, n, n);
>> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
>> -    } else {
> 
> Why aren't we doing the NOT n operation here?
> 
>> -        tcg_gen_andc_vec(vece, n, k, n);
>> -        tcg_gen_andc_vec(vece, m, m, k);
>> -        tcg_gen_or_vec(vece, d, n, m);
>> -    }
>> +    tcg_gen_not_vec(vece, n, n);
>> +    tcg_gen_bitsel_vec(vece, d, k, n, m);

Pardon?  It's right there, unindented.
Anyway, maybe I'll keep this, as it's still used on pre-avx512 x86 hosts.


r~
Re: [PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Philippe Mathieu-Daudé 2 months, 4 weeks ago 
On 8/1/25 22:38, Richard Henderson wrote:
> On 1/8/25 09:46, Philippe Mathieu-Daudé wrote:
>> On 7/1/25 08:59, Richard Henderson wrote:
>>> Rely on tcg-op-vec.c to expand the opcode if missing.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/arm/tcg/translate-sve.c | 20 ++++----------------
>>>   1 file changed, 4 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/ 
>>> translate-sve.c
>>> index 49d32fabc9..732453db6f 100644
>>> --- a/target/arm/tcg/translate-sve.c
>>> +++ b/target/arm/tcg/translate-sve.c
>>> @@ -596,14 +596,8 @@ static void gen_bsl1n_i64(TCGv_i64 d, TCGv_i64 
>>> n, TCGv_i64 m, TCGv_i64 k)
>>>   static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>>>                             TCGv_vec m, TCGv_vec k)
>>>   {
>>> -    if (TCG_TARGET_HAS_bitsel_vec) {
>>> -        tcg_gen_not_vec(vece, n, n);
>>> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
>>> -    } else {
>>
>> Why aren't we doing the NOT n operation here?
>>
>>> -        tcg_gen_andc_vec(vece, n, k, n);
>>> -        tcg_gen_andc_vec(vece, m, m, k);
>>> -        tcg_gen_or_vec(vece, d, n, m);
>>> -    }
>>> +    tcg_gen_not_vec(vece, n, n);
>>> +    tcg_gen_bitsel_vec(vece, d, k, n, m);
> 
> Pardon?  It's right there, unindented.

Sorry I'm not clear. Previous to your change, in the
TCG_TARGET_HAS_bitsel_vec side we use the NOT opcode,
but not in the other side where we expand, why?

> Anyway, maybe I'll keep this, as it's still used on pre-avx512 x86 hosts.
> 
> 
> r~
Re: [PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Richard Henderson 2 months, 3 weeks ago 
On 1/8/25 14:14, Philippe Mathieu-Daudé wrote:
>>>>   static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>>>>                             TCGv_vec m, TCGv_vec k)
>>>>   {
>>>> -    if (TCG_TARGET_HAS_bitsel_vec) {
>>>> -        tcg_gen_not_vec(vece, n, n);
>>>> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
>>>> -    } else {
>>>
>>> Why aren't we doing the NOT n operation here?
>>>
>>>> -        tcg_gen_andc_vec(vece, n, k, n);
>>>> -        tcg_gen_andc_vec(vece, m, m, k);
>>>> -        tcg_gen_or_vec(vece, d, n, m);
>>>> -    }
>>>> +    tcg_gen_not_vec(vece, n, n);
>>>> +    tcg_gen_bitsel_vec(vece, d, k, n, m);
>>
>> Pardon?  It's right there, unindented.
> 
> Sorry I'm not clear. Previous to your change, in the
> TCG_TARGET_HAS_bitsel_vec side we use the NOT opcode,
> but not in the other side where we expand, why?

Are you asking about the code being removed?

Recall that bitsel = (n & k) | (m & ~k).

Passing n = ~n' we get (~n & k) | (m & ~k),
                      = (k & ~n) | (m & ~k).

which is the two andc + or operations above.


r~
Re: [PATCH v2 07/81] target/arm: Do not test TCG_TARGET_HAS_bitsel_vec
Posted by Philippe Mathieu-Daudé 2 months, 3 weeks ago 
On 8/1/25 23:30, Richard Henderson wrote:
> On 1/8/25 14:14, Philippe Mathieu-Daudé wrote:
>>>>>   static void gen_bsl1n_vec(unsigned vece, TCGv_vec d, TCGv_vec n,
>>>>>                             TCGv_vec m, TCGv_vec k)
>>>>>   {
>>>>> -    if (TCG_TARGET_HAS_bitsel_vec) {
>>>>> -        tcg_gen_not_vec(vece, n, n);
>>>>> -        tcg_gen_bitsel_vec(vece, d, k, n, m);
>>>>> -    } else {
>>>>
>>>> Why aren't we doing the NOT n operation here?
>>>>
>>>>> -        tcg_gen_andc_vec(vece, n, k, n);

[*]

>>>>> -        tcg_gen_andc_vec(vece, m, m, k);
>>>>> -        tcg_gen_or_vec(vece, d, n, m);
>>>>> -    }
>>>>> +    tcg_gen_not_vec(vece, n, n);
>>>>> +    tcg_gen_bitsel_vec(vece, d, k, n, m);
>>>
>>> Pardon?  It's right there, unindented.
>>
>> Sorry I'm not clear. Previous to your change, in the
>> TCG_TARGET_HAS_bitsel_vec side we use the NOT opcode,
>> but not in the other side where we expand, why?
> 
> Are you asking about the code being removed?
> 
> Recall that bitsel = (n & k) | (m & ~k).
> 
> Passing n = ~n' we get (~n & k) | (m & ~k),
>                       = (k & ~n) | (m & ~k).
> 
> which is the two andc + or operations above.

Sorry, I misread the first ANDC [*] as AND...

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.