* [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372]
@ 2023-01-14 23:05 Richard Henderson
2023-01-14 23:05 ` [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Richard Henderson @ 2023-01-14 23:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Fix the instruction, and improve the test harness to better
handle 32-bit insns with 64-bit inputs.
r~
Richard Henderson (2):
tests/tcg/i386: Introduce and use reg_t consistently
target/i386: Fix BEXTR instruction
tests/tcg/i386/test-i386-bmi2.c | 194 +++++++++++++++++---------------
target/i386/tcg/emit.c.inc | 22 ++--
2 files changed, 116 insertions(+), 100 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently
2023-01-14 23:05 [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Richard Henderson
@ 2023-01-14 23:05 ` Richard Henderson
2023-01-16 8:23 ` Philippe Mathieu-Daudé
2023-01-14 23:05 ` [PATCH 2/2] target/i386: Fix BEXTR instruction Richard Henderson
2023-01-17 8:09 ` [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Paolo Bonzini
2 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2023-01-14 23:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Define reg_t based on the actual register width.
Define the inlines using that type. This will allow
input registers to 32-bit insns to be set to 64-bit
values on x86-64, which allows testing various edge cases.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/tcg/i386/test-i386-bmi2.c | 182 ++++++++++++++++----------------
1 file changed, 93 insertions(+), 89 deletions(-)
diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
index 5fadf47510..3c3ef85513 100644
--- a/tests/tcg/i386/test-i386-bmi2.c
+++ b/tests/tcg/i386/test-i386-bmi2.c
@@ -3,34 +3,40 @@
#include <stdint.h>
#include <stdio.h>
+#ifdef __x86_64
+typedef uint64_t reg_t;
+#else
+typedef uint32_t reg_t;
+#endif
+
#define insn1q(name, arg0) \
-static inline uint64_t name##q(uint64_t arg0) \
+static inline reg_t name##q(reg_t arg0) \
{ \
- uint64_t result64; \
+ reg_t result64; \
asm volatile (#name "q %1, %0" : "=r"(result64) : "rm"(arg0)); \
return result64; \
}
#define insn1l(name, arg0) \
-static inline uint32_t name##l(uint32_t arg0) \
+static inline reg_t name##l(reg_t arg0) \
{ \
- uint32_t result32; \
+ reg_t result32; \
asm volatile (#name "l %k1, %k0" : "=r"(result32) : "rm"(arg0)); \
return result32; \
}
#define insn2q(name, arg0, c0, arg1, c1) \
-static inline uint64_t name##q(uint64_t arg0, uint64_t arg1) \
+static inline reg_t name##q(reg_t arg0, reg_t arg1) \
{ \
- uint64_t result64; \
+ reg_t result64; \
asm volatile (#name "q %2, %1, %0" : "=r"(result64) : c0(arg0), c1(arg1)); \
return result64; \
}
#define insn2l(name, arg0, c0, arg1, c1) \
-static inline uint32_t name##l(uint32_t arg0, uint32_t arg1) \
+static inline reg_t name##l(reg_t arg0, reg_t arg1) \
{ \
- uint32_t result32; \
+ reg_t result32; \
asm volatile (#name "l %k2, %k1, %k0" : "=r"(result32) : c0(arg0), c1(arg1)); \
return result32; \
}
@@ -65,130 +71,128 @@ insn1l(blsr, src)
int main(int argc, char *argv[]) {
uint64_t ehlo = 0x202020204f4c4845ull;
uint64_t mask = 0xa080800302020001ull;
- uint32_t result32;
+ reg_t result;
#ifdef __x86_64
- uint64_t result64;
-
/* 64 bits */
- result64 = andnq(mask, ehlo);
- assert(result64 == 0x002020204d4c4844);
+ result = andnq(mask, ehlo);
+ assert(result == 0x002020204d4c4844);
- result64 = pextq(ehlo, mask);
- assert(result64 == 133);
+ result = pextq(ehlo, mask);
+ assert(result == 133);
- result64 = pdepq(result64, mask);
- assert(result64 == (ehlo & mask));
+ result = pdepq(result, mask);
+ assert(result == (ehlo & mask));
- result64 = pextq(-1ull, mask);
- assert(result64 == 511); /* mask has 9 bits set */
+ result = pextq(-1ull, mask);
+ assert(result == 511); /* mask has 9 bits set */
- result64 = pdepq(-1ull, mask);
- assert(result64 == mask);
+ result = pdepq(-1ull, mask);
+ assert(result == mask);
- result64 = bextrq(mask, 0x3f00);
- assert(result64 == (mask & ~INT64_MIN));
+ result = bextrq(mask, 0x3f00);
+ assert(result == (mask & ~INT64_MIN));
- result64 = bextrq(mask, 0x1038);
- assert(result64 == 0xa0);
+ result = bextrq(mask, 0x1038);
+ assert(result == 0xa0);
- result64 = bextrq(mask, 0x10f8);
- assert(result64 == 0);
+ result = bextrq(mask, 0x10f8);
+ assert(result == 0);
- result64 = blsiq(0x30);
- assert(result64 == 0x10);
+ result = blsiq(0x30);
+ assert(result == 0x10);
- result64 = blsiq(0x30ull << 32);
- assert(result64 == 0x10ull << 32);
+ result = blsiq(0x30ull << 32);
+ assert(result == 0x10ull << 32);
- result64 = blsmskq(0x30);
- assert(result64 == 0x1f);
+ result = blsmskq(0x30);
+ assert(result == 0x1f);
- result64 = blsrq(0x30);
- assert(result64 == 0x20);
+ result = blsrq(0x30);
+ assert(result == 0x20);
- result64 = blsrq(0x30ull << 32);
- assert(result64 == 0x20ull << 32);
+ result = blsrq(0x30ull << 32);
+ assert(result == 0x20ull << 32);
- result64 = bzhiq(mask, 0x3f);
- assert(result64 == (mask & ~INT64_MIN));
+ result = bzhiq(mask, 0x3f);
+ assert(result == (mask & ~INT64_MIN));
- result64 = bzhiq(mask, 0x1f);
- assert(result64 == (mask & ~(-1 << 30)));
+ result = bzhiq(mask, 0x1f);
+ assert(result == (mask & ~(-1 << 30)));
- result64 = rorxq(0x2132435465768798, 8);
- assert(result64 == 0x9821324354657687);
+ result = rorxq(0x2132435465768798, 8);
+ assert(result == 0x9821324354657687);
- result64 = sarxq(0xffeeddccbbaa9988, 8);
- assert(result64 == 0xffffeeddccbbaa99);
+ result = sarxq(0xffeeddccbbaa9988, 8);
+ assert(result == 0xffffeeddccbbaa99);
- result64 = sarxq(0x77eeddccbbaa9988, 8 | 64);
- assert(result64 == 0x0077eeddccbbaa99);
+ result = sarxq(0x77eeddccbbaa9988, 8 | 64);
+ assert(result == 0x0077eeddccbbaa99);
- result64 = shrxq(0xffeeddccbbaa9988, 8);
- assert(result64 == 0x00ffeeddccbbaa99);
+ result = shrxq(0xffeeddccbbaa9988, 8);
+ assert(result == 0x00ffeeddccbbaa99);
- result64 = shrxq(0x77eeddccbbaa9988, 8 | 192);
- assert(result64 == 0x0077eeddccbbaa99);
+ result = shrxq(0x77eeddccbbaa9988, 8 | 192);
+ assert(result == 0x0077eeddccbbaa99);
- result64 = shlxq(0xffeeddccbbaa9988, 8);
- assert(result64 == 0xeeddccbbaa998800);
+ result = shlxq(0xffeeddccbbaa9988, 8);
+ assert(result == 0xeeddccbbaa998800);
#endif
/* 32 bits */
- result32 = andnl(mask, ehlo);
- assert(result32 == 0x04d4c4844);
+ result = andnl(mask, ehlo);
+ assert(result == 0x04d4c4844);
- result32 = pextl((uint32_t) ehlo, mask);
- assert(result32 == 5);
+ result = pextl((uint32_t) ehlo, mask);
+ assert(result == 5);
- result32 = pdepl(result32, mask);
- assert(result32 == (uint32_t)(ehlo & mask));
+ result = pdepl(result, mask);
+ assert(result == (uint32_t)(ehlo & mask));
- result32 = pextl(-1u, mask);
- assert(result32 == 7); /* mask has 3 bits set */
+ result = pextl(-1u, mask);
+ assert(result == 7); /* mask has 3 bits set */
- result32 = pdepl(-1u, mask);
- assert(result32 == (uint32_t)mask);
+ result = pdepl(-1u, mask);
+ assert(result == (uint32_t)mask);
- result32 = bextrl(mask, 0x1f00);
- assert(result32 == (mask & ~INT32_MIN));
+ result = bextrl(mask, 0x1f00);
+ assert(result == (mask & ~INT32_MIN));
- result32 = bextrl(ehlo, 0x1018);
- assert(result32 == 0x4f);
+ result = bextrl(ehlo, 0x1018);
+ assert(result == 0x4f);
- result32 = bextrl(mask, 0x1038);
- assert(result32 == 0);
+ result = bextrl(mask, 0x1038);
+ assert(result == 0);
- result32 = blsil(0xffff);
- assert(result32 == 1);
+ result = blsil(0xffff);
+ assert(result == 1);
- result32 = blsmskl(0x300);
- assert(result32 == 0x1ff);
+ result = blsmskl(0x300);
+ assert(result == 0x1ff);
- result32 = blsrl(0xffc);
- assert(result32 == 0xff8);
+ result = blsrl(0xffc);
+ assert(result == 0xff8);
- result32 = bzhil(mask, 0xf);
- assert(result32 == 1);
+ result = bzhil(mask, 0xf);
+ assert(result == 1);
- result32 = rorxl(0x65768798, 8);
- assert(result32 == 0x98657687);
+ result = rorxl(0x65768798, 8);
+ assert(result == 0x98657687);
- result32 = sarxl(0xffeeddcc, 8);
- assert(result32 == 0xffffeedd);
+ result = sarxl(0xffeeddcc, 8);
+ assert(result == 0xffffeedd);
- result32 = sarxl(0x77eeddcc, 8 | 32);
- assert(result32 == 0x0077eedd);
+ result = sarxl(0x77eeddcc, 8 | 32);
+ assert(result == 0x0077eedd);
- result32 = shrxl(0xffeeddcc, 8);
- assert(result32 == 0x00ffeedd);
+ result = shrxl(0xffeeddcc, 8);
+ assert(result == 0x00ffeedd);
- result32 = shrxl(0x77eeddcc, 8 | 128);
- assert(result32 == 0x0077eedd);
+ result = shrxl(0x77eeddcc, 8 | 128);
+ assert(result == 0x0077eedd);
- result32 = shlxl(0xffeeddcc, 8);
- assert(result32 == 0xeeddcc00);
+ result = shlxl(0xffeeddcc, 8);
+ assert(result == 0xeeddcc00);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] target/i386: Fix BEXTR instruction
2023-01-14 23:05 [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Richard Henderson
2023-01-14 23:05 ` [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently Richard Henderson
@ 2023-01-14 23:05 ` Richard Henderson
2023-01-17 8:09 ` [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2023-01-14 23:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
There were two problems here: not limiting the input to operand bits,
and not correctly handling large extraction length.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1372
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/tcg/i386/test-i386-bmi2.c | 12 ++++++++++++
target/i386/tcg/emit.c.inc | 22 +++++++++++-----------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
index 3c3ef85513..982d4abda4 100644
--- a/tests/tcg/i386/test-i386-bmi2.c
+++ b/tests/tcg/i386/test-i386-bmi2.c
@@ -99,6 +99,9 @@ int main(int argc, char *argv[]) {
result = bextrq(mask, 0x10f8);
assert(result == 0);
+ result = bextrq(0xfedcba9876543210ull, 0x7f00);
+ assert(result == 0xfedcba9876543210ull);
+
result = blsiq(0x30);
assert(result == 0x10);
@@ -164,6 +167,15 @@ int main(int argc, char *argv[]) {
result = bextrl(mask, 0x1038);
assert(result == 0);
+ result = bextrl((reg_t)0x8f635a775ad3b9b4ull, 0x3018);
+ assert(result == 0x5a);
+
+ result = bextrl((reg_t)0xfedcba9876543210ull, 0x7f00);
+ assert(result == 0x76543210u);
+
+ result = bextrl(-1, 0);
+ assert(result == 0);
+
result = blsil(0xffff);
assert(result == 1);
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 9d610de8c2..4d7702c106 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1078,30 +1078,30 @@ static void gen_ANDN(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
static void gen_BEXTR(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[0].ot;
- TCGv bound, zero;
+ TCGv bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
+ TCGv zero = tcg_constant_tl(0);
+ TCGv mone = tcg_constant_tl(-1);
/*
* Extract START, and shift the operand.
* Shifts larger than operand size get zeros.
*/
tcg_gen_ext8u_tl(s->A0, s->T1);
+ if (TARGET_LONG_BITS == 64 && ot == MO_32) {
+ tcg_gen_ext32u_tl(s->T0, s->T0);
+ }
tcg_gen_shr_tl(s->T0, s->T0, s->A0);
- bound = tcg_constant_tl(ot == MO_64 ? 63 : 31);
- zero = tcg_constant_tl(0);
tcg_gen_movcond_tl(TCG_COND_LEU, s->T0, s->A0, bound, s->T0, zero);
/*
- * Extract the LEN into a mask. Lengths larger than
- * operand size get all ones.
+ * Extract the LEN into an inverse mask. Lengths larger than
+ * operand size get all zeros, length 0 gets all ones.
*/
tcg_gen_extract_tl(s->A0, s->T1, 8, 8);
- tcg_gen_movcond_tl(TCG_COND_LEU, s->A0, s->A0, bound, s->A0, bound);
-
- tcg_gen_movi_tl(s->T1, 1);
- tcg_gen_shl_tl(s->T1, s->T1, s->A0);
- tcg_gen_subi_tl(s->T1, s->T1, 1);
- tcg_gen_and_tl(s->T0, s->T0, s->T1);
+ tcg_gen_shl_tl(s->T1, mone, s->A0);
+ tcg_gen_movcond_tl(TCG_COND_LEU, s->T1, s->A0, bound, s->T1, zero);
+ tcg_gen_andc_tl(s->T0, s->T0, s->T1);
gen_op_update1_cc(s);
set_cc_op(s, CC_OP_LOGICB + ot);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently
2023-01-14 23:05 ` [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently Richard Henderson
@ 2023-01-16 8:23 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-16 8:23 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini
On 15/1/23 00:05, Richard Henderson wrote:
> Define reg_t based on the actual register width.
> Define the inlines using that type. This will allow
> input registers to 32-bit insns to be set to 64-bit
> values on x86-64, which allows testing various edge cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> tests/tcg/i386/test-i386-bmi2.c | 182 ++++++++++++++++----------------
> 1 file changed, 93 insertions(+), 89 deletions(-)
>
> diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
> index 5fadf47510..3c3ef85513 100644
> --- a/tests/tcg/i386/test-i386-bmi2.c
> +++ b/tests/tcg/i386/test-i386-bmi2.c
> @@ -3,34 +3,40 @@
> #include <stdint.h>
> #include <stdio.h>
>
> +#ifdef __x86_64
> +typedef uint64_t reg_t;
> +#else
> +typedef uint32_t reg_t;
> +#endif
> +
> #define insn1q(name, arg0) \
> -static inline uint64_t name##q(uint64_t arg0) \
> +static inline reg_t name##q(reg_t arg0) \
> { \
> - uint64_t result64; \
> + reg_t result64; \
> asm volatile (#name "q %1, %0" : "=r"(result64) : "rm"(arg0)); \
> return result64; \
> }
>
> #define insn1l(name, arg0) \
> -static inline uint32_t name##l(uint32_t arg0) \
> +static inline reg_t name##l(reg_t arg0) \
> { \
> - uint32_t result32; \
> + reg_t result32; \
> asm volatile (#name "l %k1, %k0" : "=r"(result32) : "rm"(arg0)); \
> return result32; \
> }
Are the names 'result64/result32' still appropriate?
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372]
2023-01-14 23:05 [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Richard Henderson
2023-01-14 23:05 ` [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently Richard Henderson
2023-01-14 23:05 ` [PATCH 2/2] target/i386: Fix BEXTR instruction Richard Henderson
@ 2023-01-17 8:09 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2023-01-17 8:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, pbonzini
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-17 8:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-14 23:05 [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Richard Henderson
2023-01-14 23:05 ` [PATCH 1/2] tests/tcg/i386: Introduce and use reg_t consistently Richard Henderson
2023-01-16 8:23 ` Philippe Mathieu-Daudé
2023-01-14 23:05 ` [PATCH 2/2] target/i386: Fix BEXTR instruction Richard Henderson
2023-01-17 8:09 ` [PATCH 0/2] target/i386: Fix BEXTR instruction [#1372] Paolo Bonzini
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.