[PATCH] target/i386: Fix BZHI instruction

Richard Henderson posted 1 patch 1 year, 2 months ago
tests/tcg/i386/test-i386-bmi2.c |  3 +++
target/i386/tcg/emit.c.inc      | 14 +++++++-------
2 files changed, 10 insertions(+), 7 deletions(-)
[PATCH] target/i386: Fix BZHI instruction
Posted by Richard Henderson 1 year, 2 months ago
We did not correctly handle N >= operand size.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1374
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/i386/test-i386-bmi2.c |  3 +++
 target/i386/tcg/emit.c.inc      | 14 +++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
index 982d4abda4..0244df7987 100644
--- a/tests/tcg/i386/test-i386-bmi2.c
+++ b/tests/tcg/i386/test-i386-bmi2.c
@@ -123,6 +123,9 @@ int main(int argc, char *argv[]) {
     result = bzhiq(mask, 0x1f);
     assert(result == (mask & ~(-1 << 30)));
 
+    result = bzhiq(mask, 0x40);
+    assert(result == mask);
+
     result = rorxq(0x2132435465768798, 8);
     assert(result == 0x9821324354657687);
 
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 4d7702c106..1eace1231a 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1143,20 +1143,20 @@ static void gen_BLSR(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
 static void gen_BZHI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[0].ot;
-    TCGv bound;
+    TCGv bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
+    TCGv zero = tcg_constant_tl(0);
+    TCGv mone = tcg_constant_tl(-1);
 
-    tcg_gen_ext8u_tl(s->T1, cpu_regs[s->vex_v]);
-    bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
+    tcg_gen_ext8u_tl(s->T1, s->T1);
 
     /*
      * Note that since we're using BMILG (in order to get O
      * cleared) we need to store the inverse into C.
      */
-    tcg_gen_setcond_tl(TCG_COND_LT, cpu_cc_src, s->T1, bound);
-    tcg_gen_movcond_tl(TCG_COND_GT, s->T1, s->T1, bound, bound, s->T1);
+    tcg_gen_setcond_tl(TCG_COND_LEU, cpu_cc_src, s->T1, bound);
 
-    tcg_gen_movi_tl(s->A0, -1);
-    tcg_gen_shl_tl(s->A0, s->A0, s->T1);
+    tcg_gen_shl_tl(s->A0, mone, s->T1);
+    tcg_gen_movcond_tl(TCG_COND_LEU, s->A0, s->T1, bound, s->A0, zero);
     tcg_gen_andc_tl(s->T0, s->T0, s->A0);
 
     gen_op_update1_cc(s);
-- 
2.34.1
Re: [PATCH] target/i386: Fix BZHI instruction
Posted by Paolo Bonzini 1 year, 1 month ago
Queued, thanks.

Paolo
Re: [PATCH] target/i386: Fix BZHI instruction
Posted by Richard Henderson 1 year, 1 month ago
Ping.

r~

On 1/14/23 13:32, Richard Henderson wrote:
> We did not correctly handle N >= operand size.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1374
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tests/tcg/i386/test-i386-bmi2.c |  3 +++
>   target/i386/tcg/emit.c.inc      | 14 +++++++-------
>   2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
> index 982d4abda4..0244df7987 100644
> --- a/tests/tcg/i386/test-i386-bmi2.c
> +++ b/tests/tcg/i386/test-i386-bmi2.c
> @@ -123,6 +123,9 @@ int main(int argc, char *argv[]) {
>       result = bzhiq(mask, 0x1f);
>       assert(result == (mask & ~(-1 << 30)));
>   
> +    result = bzhiq(mask, 0x40);
> +    assert(result == mask);
> +
>       result = rorxq(0x2132435465768798, 8);
>       assert(result == 0x9821324354657687);
>   
> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
> index 4d7702c106..1eace1231a 100644
> --- a/target/i386/tcg/emit.c.inc
> +++ b/target/i386/tcg/emit.c.inc
> @@ -1143,20 +1143,20 @@ static void gen_BLSR(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
>   static void gen_BZHI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
>   {
>       MemOp ot = decode->op[0].ot;
> -    TCGv bound;
> +    TCGv bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
> +    TCGv zero = tcg_constant_tl(0);
> +    TCGv mone = tcg_constant_tl(-1);
>   
> -    tcg_gen_ext8u_tl(s->T1, cpu_regs[s->vex_v]);
> -    bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
> +    tcg_gen_ext8u_tl(s->T1, s->T1);
>   
>       /*
>        * Note that since we're using BMILG (in order to get O
>        * cleared) we need to store the inverse into C.
>        */
> -    tcg_gen_setcond_tl(TCG_COND_LT, cpu_cc_src, s->T1, bound);
> -    tcg_gen_movcond_tl(TCG_COND_GT, s->T1, s->T1, bound, bound, s->T1);
> +    tcg_gen_setcond_tl(TCG_COND_LEU, cpu_cc_src, s->T1, bound);
>   
> -    tcg_gen_movi_tl(s->A0, -1);
> -    tcg_gen_shl_tl(s->A0, s->A0, s->T1);
> +    tcg_gen_shl_tl(s->A0, mone, s->T1);
> +    tcg_gen_movcond_tl(TCG_COND_LEU, s->A0, s->T1, bound, s->A0, zero);
>       tcg_gen_andc_tl(s->T0, s->T0, s->A0);
>   
>       gen_op_update1_cc(s);
Re: [PATCH] target/i386: Fix BZHI instruction
Posted by Richard Henderson 1 year, 1 month ago
Ping 2.

r~

On 2/15/23 20:50, Richard Henderson wrote:
> Ping.
> 
> r~
> 
> On 1/14/23 13:32, Richard Henderson wrote:
>> We did not correctly handle N >= operand size.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1374
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   tests/tcg/i386/test-i386-bmi2.c |  3 +++
>>   target/i386/tcg/emit.c.inc      | 14 +++++++-------
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
>> index 982d4abda4..0244df7987 100644
>> --- a/tests/tcg/i386/test-i386-bmi2.c
>> +++ b/tests/tcg/i386/test-i386-bmi2.c
>> @@ -123,6 +123,9 @@ int main(int argc, char *argv[]) {
>>       result = bzhiq(mask, 0x1f);
>>       assert(result == (mask & ~(-1 << 30)));
>> +    result = bzhiq(mask, 0x40);
>> +    assert(result == mask);
>> +
>>       result = rorxq(0x2132435465768798, 8);
>>       assert(result == 0x9821324354657687);
>> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
>> index 4d7702c106..1eace1231a 100644
>> --- a/target/i386/tcg/emit.c.inc
>> +++ b/target/i386/tcg/emit.c.inc
>> @@ -1143,20 +1143,20 @@ static void gen_BLSR(DisasContext *s, CPUX86State *env, 
>> X86DecodedInsn *decode)
>>   static void gen_BZHI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
>>   {
>>       MemOp ot = decode->op[0].ot;
>> -    TCGv bound;
>> +    TCGv bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
>> +    TCGv zero = tcg_constant_tl(0);
>> +    TCGv mone = tcg_constant_tl(-1);
>> -    tcg_gen_ext8u_tl(s->T1, cpu_regs[s->vex_v]);
>> -    bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
>> +    tcg_gen_ext8u_tl(s->T1, s->T1);
>>       /*
>>        * Note that since we're using BMILG (in order to get O
>>        * cleared) we need to store the inverse into C.
>>        */
>> -    tcg_gen_setcond_tl(TCG_COND_LT, cpu_cc_src, s->T1, bound);
>> -    tcg_gen_movcond_tl(TCG_COND_GT, s->T1, s->T1, bound, bound, s->T1);
>> +    tcg_gen_setcond_tl(TCG_COND_LEU, cpu_cc_src, s->T1, bound);
>> -    tcg_gen_movi_tl(s->A0, -1);
>> -    tcg_gen_shl_tl(s->A0, s->A0, s->T1);
>> +    tcg_gen_shl_tl(s->A0, mone, s->T1);
>> +    tcg_gen_movcond_tl(TCG_COND_LEU, s->A0, s->T1, bound, s->A0, zero);
>>       tcg_gen_andc_tl(s->T0, s->T0, s->A0);
>>       gen_op_update1_cc(s);
>