* [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions
@ 2017-07-11 21:21 Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 1/2] target/i386: Decode AMD XOP prefix Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Richard Henderson @ 2017-07-11 21:21 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, ricardo.ribalda
These are general purpose bit manipulation instructions akin
to the BMI1 and BMI2 instructions. This is an AMD extension
and uses the XOP instruction prefix.
I am in the process of trying to run the gcc testsuite with -mtbm,
with and without the patchset, to see (1) if the new insns get used
and (2) that they run ok.
Please review.
r~
Richard Henderson (2):
target/i386: Decode AMD XOP prefix
target/i386: Implement all TBM instructions
target/i386/cc_helper_template.h | 18 +++++
target/i386/cpu.h | 7 +-
target/i386/cc_helper.c | 28 ++++++-
target/i386/cpu.c | 3 +-
target/i386/translate.c | 163 +++++++++++++++++++++++++++++++++++----
5 files changed, 199 insertions(+), 20 deletions(-)
--
2.9.4
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/2] target/i386: Decode AMD XOP prefix
2017-07-11 21:21 [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions Richard Henderson
@ 2017-07-11 21:21 ` Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 2/2] target/i386: Implement all TBM instructions Richard Henderson
2017-07-12 4:04 ` [Qemu-devel] [PATCH 0/2] " Richard Henderson
2 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2017-07-11 21:21 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, ricardo.ribalda
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/i386/translate.c | 46 ++++++++++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index ed3b896..6082db2 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4500,8 +4500,9 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
#endif
case 0xc5: /* 2-byte VEX */
case 0xc4: /* 3-byte VEX */
+ case 0x8f: /* 3-byte XOP */
/* VEX prefixes cannot be used except in 32-bit mode.
- Otherwise the instruction is LES or LDS. */
+ Otherwise the instruction is LES, LDS, or POP. */
if (s->code32 && !s->vm86) {
static const int pp_prefix[4] = {
0, PREFIX_DATA, PREFIX_REPZ, PREFIX_REPNZ
@@ -4510,7 +4511,13 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
if (!CODE64(s) && (vex2 & 0xc0) != 0xc0) {
/* 4.1.4.6: In 32-bit mode, bits [7:6] must be 11b,
- otherwise the instruction is LES or LDS. */
+ otherwise the instruction is LES, LDS, or POP. */
+ break;
+ }
+ if (b == 0x8f && (vex2 & 0x1f) < 8) {
+ /* If the value of the XOP.map_select field is less than 8,
+ the first two bytes of the three-byte XOP are interpreted
+ as a form of the POP instruction. */
break;
}
s->pc++;
@@ -4536,18 +4543,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
#endif
vex3 = cpu_ldub_code(env, s->pc++);
rex_w = (vex3 >> 7) & 1;
- switch (vex2 & 0x1f) {
- case 0x01: /* Implied 0f leading opcode bytes. */
- b = cpu_ldub_code(env, s->pc++) | 0x100;
- break;
- case 0x02: /* Implied 0f 38 leading opcode bytes. */
- b = 0x138;
- break;
- case 0x03: /* Implied 0f 3a leading opcode bytes. */
- b = 0x13a;
- break;
- default: /* Reserved for future use. */
- goto unknown_op;
+ if (b == 0xc4) {
+ switch (vex2 & 0x1f) {
+ case 0x01: /* Implied 0f leading opcode bytes. */
+ b = cpu_ldub_code(env, s->pc++) | 0x100;
+ break;
+ case 0x02: /* Implied 0f 38 leading opcode bytes. */
+ b = 0x138;
+ break;
+ case 0x03: /* Implied 0f 3a leading opcode bytes. */
+ b = 0x13a;
+ break;
+ default: /* Reserved for future use. */
+ goto unknown_op;
+ }
+ } else {
+ /* Unlike VEX, XOP.map_select does not overlap the
+ base instruction set. Prepend the map_select to
+ the next opcode byte. */
+ b = cpu_ldub_code(env, s->pc++) + (vex2 & 0x1f) * 0x100;
}
}
s->vex_v = (~vex3 >> 3) & 0xf;
@@ -8276,6 +8290,10 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
case 0x1d0 ... 0x1fe:
gen_sse(env, s, b, pc_start, rex_r);
break;
+
+ case 0x800 ... 0x8ff: /* XOP opcode map 8 */
+ case 0x900 ... 0x9ff: /* XOP opcode map 9 */
+ case 0xa00 ... 0xaff: /* XOP opcode map 10 */
default:
goto unknown_op;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/2] target/i386: Implement all TBM instructions
2017-07-11 21:21 [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 1/2] target/i386: Decode AMD XOP prefix Richard Henderson
@ 2017-07-11 21:21 ` Richard Henderson
2017-07-12 4:04 ` [Qemu-devel] [PATCH 0/2] " Richard Henderson
2 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2017-07-11 21:21 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, ricardo.ribalda
Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/i386/cc_helper_template.h | 18 ++++++
target/i386/cpu.h | 7 ++-
target/i386/cc_helper.c | 28 +++++++--
target/i386/cpu.c | 3 +-
target/i386/translate.c | 123 ++++++++++++++++++++++++++++++++++++++-
5 files changed, 170 insertions(+), 9 deletions(-)
diff --git a/target/i386/cc_helper_template.h b/target/i386/cc_helper_template.h
index 607311f..6ce63b7 100644
--- a/target/i386/cc_helper_template.h
+++ b/target/i386/cc_helper_template.h
@@ -235,6 +235,24 @@ static int glue(compute_c_bmilg, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1)
return src1 == 0;
}
+static int glue(compute_all_tbmadd, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1)
+{
+ int cf, pf, af, zf, sf, of;
+
+ cf = (src1 == (DATA_TYPE)-1);
+ pf = 0; /* undefined */
+ af = 0; /* undefined */
+ zf = (dst == 0) * CC_Z;
+ sf = lshift(dst, 8 - DATA_BITS) & CC_S;
+ of = 0;
+ return cf | pf | af | zf | sf | of;
+}
+
+static int glue(compute_c_tbmadd, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1)
+{
+ return src1 == (DATA_TYPE)-1;
+}
+
#undef DATA_BITS
#undef SIGN_MASK
#undef DATA_TYPE
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7a228af..537f592 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -774,11 +774,16 @@ typedef enum {
CC_OP_SARL,
CC_OP_SARQ,
- CC_OP_BMILGB, /* Z,S via CC_DST, C = SRC==0; O=0; P,A undefined */
+ CC_OP_BMILGB, /* Z,S via DST, C = SRC==0; O=0; P,A undefined */
CC_OP_BMILGW,
CC_OP_BMILGL,
CC_OP_BMILGQ,
+ CC_OP_TBMADDB, /* Z,S via DST; C = SRC==-1; O=0; P,A undefined */
+ CC_OP_TBMADDW,
+ CC_OP_TBMADDL,
+ CC_OP_TBMADDQ,
+
CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest. */
CC_OP_ADOX, /* CC_DST = O, CC_SRC = rest. */
CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest. */
diff --git a/target/i386/cc_helper.c b/target/i386/cc_helper.c
index c9c90e1..2f12c3b 100644
--- a/target/i386/cc_helper.c
+++ b/target/i386/cc_helper.c
@@ -98,9 +98,6 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1,
target_ulong src2, int op)
{
switch (op) {
- default: /* should never happen */
- return 0;
-
case CC_OP_EFLAGS:
return src1;
case CC_OP_CLR:
@@ -185,6 +182,13 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1,
case CC_OP_BMILGL:
return compute_all_bmilgl(dst, src1);
+ case CC_OP_TBMADDB:
+ return compute_all_tbmaddb(dst, src1);
+ case CC_OP_TBMADDW:
+ return compute_all_tbmaddw(dst, src1);
+ case CC_OP_TBMADDL:
+ return compute_all_tbmaddl(dst, src1);
+
case CC_OP_ADCX:
return compute_all_adcx(dst, src1, src2);
case CC_OP_ADOX:
@@ -215,7 +219,12 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1,
return compute_all_sarq(dst, src1);
case CC_OP_BMILGQ:
return compute_all_bmilgq(dst, src1);
+ case CC_OP_TBMADDQ:
+ return compute_all_tbmaddq(dst, src1);
#endif
+
+ default:
+ g_assert_not_reached();
}
}
@@ -228,7 +237,6 @@ target_ulong helper_cc_compute_c(target_ulong dst, target_ulong src1,
target_ulong src2, int op)
{
switch (op) {
- default: /* should never happen */
case CC_OP_LOGICB:
case CC_OP_LOGICW:
case CC_OP_LOGICL:
@@ -307,6 +315,13 @@ target_ulong helper_cc_compute_c(target_ulong dst, target_ulong src1,
case CC_OP_BMILGL:
return compute_c_bmilgl(dst, src1);
+ case CC_OP_TBMADDB:
+ return compute_c_tbmaddb(dst, src1);
+ case CC_OP_TBMADDW:
+ return compute_c_tbmaddw(dst, src1);
+ case CC_OP_TBMADDL:
+ return compute_c_tbmaddl(dst, src1);
+
#ifdef TARGET_X86_64
case CC_OP_ADDQ:
return compute_c_addq(dst, src1);
@@ -320,7 +335,12 @@ target_ulong helper_cc_compute_c(target_ulong dst, target_ulong src1,
return compute_c_shlq(dst, src1);
case CC_OP_BMILGQ:
return compute_c_bmilgq(dst, src1);
+ case CC_OP_TBMADDQ:
+ return compute_c_tbmaddq(dst, src1);
#endif
+
+ default:
+ g_assert_not_reached();
}
}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c571772..34ab828 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -227,7 +227,8 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_PDPE1GB | \
TCG_EXT2_X86_64_FEATURES)
#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
- CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+ CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A | \
+ CPUID_EXT3_TBM)
#define TCG_EXT4_FEATURES 0
#define TCG_SVM_FEATURES 0
#define TCG_KVM_FEATURES 0
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 6082db2..2c64d2b 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -219,6 +219,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = {
[CC_OP_SHLB ... CC_OP_SHLQ] = USES_CC_DST | USES_CC_SRC,
[CC_OP_SARB ... CC_OP_SARQ] = USES_CC_DST | USES_CC_SRC,
[CC_OP_BMILGB ... CC_OP_BMILGQ] = USES_CC_DST | USES_CC_SRC,
+ [CC_OP_TBMADDB ... CC_OP_TBMADDQ] = USES_CC_DST | USES_CC_SRC,
[CC_OP_ADCX] = USES_CC_DST | USES_CC_SRC,
[CC_OP_ADOX] = USES_CC_SRC | USES_CC_SRC2,
[CC_OP_ADCOX] = USES_CC_DST | USES_CC_SRC | USES_CC_SRC2,
@@ -783,6 +784,12 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
t0 = gen_ext_tl(reg, cpu_cc_src, size, false);
return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0, .mask = -1 };
+ case CC_OP_TBMADDB ... CC_OP_TBMADDQ:
+ size = s->cc_op - CC_OP_TBMADDB;
+ t0 = gen_ext_tl(reg, cpu_cc_src, size, true);
+ return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0,
+ .mask = -1, .imm = -1 };
+
case CC_OP_ADCX:
case CC_OP_ADCOX:
return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_dst,
@@ -8291,9 +8298,119 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
gen_sse(env, s, b, pc_start, rex_r);
break;
- case 0x800 ... 0x8ff: /* XOP opcode map 8 */
- case 0x900 ... 0x9ff: /* XOP opcode map 9 */
- case 0xa00 ... 0xaff: /* XOP opcode map 10 */
+ case 0x901:
+ case 0x902: /* most tbm insns */
+ if (!(s->cpuid_ext3_features & CPUID_EXT3_TBM)
+ || s->vex_l != 0) {
+ goto illegal_op;
+ }
+ modrm = cpu_ldub_code(env, s->pc++);
+ mod = (modrm >> 6) & 3;
+ rm = (modrm & 7) | REX_B(s);
+ ot = rex_w > 0 ? MO_64 : MO_32;
+ if (mod != 3) {
+ gen_lea_modrm(env, s, modrm);
+ gen_op_ld_v(s, ot, cpu_T0, cpu_A0);
+ } else {
+ gen_op_mov_v_reg(ot, cpu_T0, rm);
+ }
+
+ tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
+ switch ((b & 2) * 4 + ((modrm >> 3) & 7)) {
+ case 1: /* blcfill */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
+ break;
+ case 2: /* blsfill */
+ op = CC_OP_BMILGB;
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_or_tl(cpu_T0, cpu_T0, cpu_T1);
+ break;
+ case 3: /* blcs */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_or_tl(cpu_T0, cpu_T0, cpu_T1);
+ break;
+ case 4: /* tzmsk */
+ op = CC_OP_BMILGB;
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_andc_tl(cpu_T0, cpu_T1, cpu_T0);
+ break;
+ case 5: /* blcic */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_andc_tl(cpu_T0, cpu_T1, cpu_T0);
+ break;
+ case 6: /* blsic */
+ op = CC_OP_BMILGB;
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_orc_tl(cpu_T0, cpu_T1, cpu_T0);
+ break;
+ case 7: /* t1mskc */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_orc_tl(cpu_T0, cpu_T1, cpu_T0);
+ break;
+ case 8 + 1: /* blcmsk */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
+ break;
+ case 8 + 6: /* blci */
+ op = CC_OP_TBMADDB;
+ tcg_gen_addi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_orc_tl(cpu_T0, cpu_T0, cpu_T1);
+ break;
+ default:
+ goto illegal_op;
+ }
+ gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
+ tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
+ set_cc_op(s, op + ot);
+ break;
+
+ case 0xa10: /* bextr Gy, Ey, imm4 */
+ {
+ int ofs, len, max;
+
+ if (!(s->cpuid_ext3_features & CPUID_EXT3_TBM)
+ || s->vex_l != 0) {
+ goto illegal_op;
+ }
+
+ s->rip_offset = 4;
+ modrm = cpu_ldub_code(env, s->pc++);
+ reg = ((modrm >> 3) & 7) | rex_r;
+ mod = (modrm >> 6) & 3;
+ rm = (modrm & 7) | REX_B(s);
+ ot = rex_w > 0 ? MO_64 : MO_32;
+ if (mod != 3) {
+ gen_lea_modrm(env, s, modrm);
+ gen_op_ld_v(s, ot, cpu_T0, cpu_A0);
+ } else {
+ gen_op_mov_v_reg(ot, cpu_T0, rm);
+ }
+ val = cpu_ldl_code(env, s->pc);
+ s->pc += 4;
+
+ ofs = extract32(val, 0, 8);
+ len = extract32(val, 8, 8);
+ max = 8 << ot;
+ if (len == 0 || ofs >= max) {
+ tcg_gen_movi_tl(cpu_T0, 0);
+ } else {
+ len = MIN(len, max - ofs);
+ tcg_gen_extract_tl(cpu_T0, cpu_T0, ofs, len);
+ }
+ tcg_gen_mov_tl(cpu_regs[reg], cpu_T0);
+ gen_op_update1_cc();
+ /* Z is set as per result, C/O = 0, S/A/P = undefined.
+ Which is less strict than LOGIC, but accurate. */
+ set_cc_op(s, CC_OP_LOGICB + ot);
+ }
+ break;
+
default:
goto unknown_op;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions
2017-07-11 21:21 [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 1/2] target/i386: Decode AMD XOP prefix Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 2/2] target/i386: Implement all TBM instructions Richard Henderson
@ 2017-07-12 4:04 ` Richard Henderson
2017-07-12 13:28 ` Ricardo Ribalda Delgado
2 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2017-07-12 4:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, ricardo.ribalda
On 07/11/2017 11:21 AM, Richard Henderson wrote:
> I am in the process of trying to run the gcc testsuite with -mtbm,
> with and without the patchset, to see (1) if the new insns get used
> and (2) that they run ok.
FWIW, make check-gcc RUNTESTFLAGS='--target_board=unix/-mtbm execute.exp' shows
204 failures on a host that does not support TBM, so the extension is being
used. A browse through exactly one of these used only bextr. Running the same
tests with dejagnu using qemu-x86_64 -cpu qemu64,+tbm shows zero failures.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions
2017-07-12 4:04 ` [Qemu-devel] [PATCH 0/2] " Richard Henderson
@ 2017-07-12 13:28 ` Ricardo Ribalda Delgado
2017-07-12 18:43 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-12 13:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, pbonzini, ehabkost
Hi Richard
Thanks for your patch! I have applied it to my tree, but i still get
SIGSEGV. I think that I might have found the problem. It seems to be
related to the bmi instruction blsr, which seems to be not properly
implemented.
On this example:
#include <stdio.h>
int test_blsr(int val){
return (val & (val - 1));
}
int main(int argc, char *argv) {
volatile int val = 4096;
fprintf(stdout, "%d\n", test_blsr(val));
return 0;
}
When it is compiled with -march=bdver4 -static -O3 test_blsr , the
compiler produces:
0000000000400af0 <test_blsr>:
400af0: c4 e2 78 f3 cf blsr %edi,%eax
400af5: c3 retq
400af6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
400afd: 00 00 00
If I run the emulator: /tmp/qemu/x86_64-linux-user/qemu-x86_64 -cpu
Haswell ./a.out
The function prints 4096
A fast look in the code shows that
https://github.com/qemu/qemu/blob/master/target/i386/translate.c#L4028
does not really match
https://en.wikipedia.org/wiki/Bit_Manipulation_Instruction_Sets#BMI1_.28Bit_Manipulation_Instruction_Set_1.29
It appears that case 1 and case 3 are swapped. I tried to fix it, but
with no results :(. Anyway, the wiki could also be wrong.
What is sure is that the code produces different results on qemu than
on the target, which is not good
Thanks again for your help!
On Wed, Jul 12, 2017 at 6:04 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 07/11/2017 11:21 AM, Richard Henderson wrote:
>>
>> I am in the process of trying to run the gcc testsuite with -mtbm,
>> with and without the patchset, to see (1) if the new insns get used
>> and (2) that they run ok.
>
>
> FWIW, make check-gcc RUNTESTFLAGS='--target_board=unix/-mtbm execute.exp'
> shows 204 failures on a host that does not support TBM, so the extension is
> being used. A browse through exactly one of these used only bextr. Running
> the same tests with dejagnu using qemu-x86_64 -cpu qemu64,+tbm shows zero
> failures.
>
>
> r~
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions
2017-07-12 13:28 ` Ricardo Ribalda Delgado
@ 2017-07-12 18:43 ` Richard Henderson
2017-07-12 18:45 ` [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI Richard Henderson
2017-07-12 19:29 ` [Qemu-devel] [PATCH v2] " Richard Henderson
2 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2017-07-12 18:43 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: qemu-devel, pbonzini, ehabkost
On 07/12/2017 03:28 AM, Ricardo Ribalda Delgado wrote:
> Hi Richard
>
> Thanks for your patch! I have applied it to my tree, but i still get
> SIGSEGV. I think that I might have found the problem. It seems to be
> related to the bmi instruction blsr, which seems to be not properly
> implemented.
You're absolutely right.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2017-07-12 13:28 ` Ricardo Ribalda Delgado
2017-07-12 18:43 ` Richard Henderson
@ 2017-07-12 18:45 ` Richard Henderson
2017-07-12 18:58 ` Ricardo Ribalda Delgado
2018-06-06 9:13 ` Ricardo Ribalda Delgado
2017-07-12 19:29 ` [Qemu-devel] [PATCH v2] " Richard Henderson
2 siblings, 2 replies; 16+ messages in thread
From: Richard Henderson @ 2017-07-12 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: ricardo.ribalda, ehabkost, pbonzini
The implementation of these two instructions was swapped.
At the same time, unify the setup of eflags for the insn group.
Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/i386/translate.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 8365a6d..087a2e6 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4029,36 +4029,27 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
goto illegal_op;
}
ot = mo_64_32(s->dflag);
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, ot, OR_TMP1, 0);
switch (reg & 7) {
case 1: /* blsr By,Ey */
- tcg_gen_neg_tl(cpu_T1, cpu_T0);
+ tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
- gen_op_update2_cc();
- set_cc_op(s, CC_OP_BMILGB + ot);
break;
-
case 2: /* blsmsk By,Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
+ tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
case 3: /* blsi By, Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_neg_tl(cpu_T0, cpu_T1);
+ tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
default:
goto unknown_op;
}
+ gen_op_update2_cc();
+ set_cc_op(s, CC_OP_BMILGB + ot);
break;
default:
--
2.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2017-07-12 18:45 ` [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI Richard Henderson
@ 2017-07-12 18:58 ` Ricardo Ribalda Delgado
2017-07-12 19:12 ` Richard Henderson
2018-06-06 9:13 ` Ricardo Ribalda Delgado
1 sibling, 1 reply; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-12 18:58 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
Hi Richard
Thanks again!, When I apply this patch I get the following error:
/tmp/qemu/tcg/tcg.c:2042: tcg fatal error
Regards!
On Wed, Jul 12, 2017 at 8:45 PM, Richard Henderson <rth@twiddle.net> wrote:
> The implementation of these two instructions was swapped.
> At the same time, unify the setup of eflags for the insn group.
>
> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/i386/translate.c | 25 ++++++++-----------------
> 1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 8365a6d..087a2e6 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -4029,36 +4029,27 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
> goto illegal_op;
> }
> ot = mo_64_32(s->dflag);
> - gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
> + gen_ldst_modrm(env, s, modrm, ot, OR_TMP1, 0);
>
> switch (reg & 7) {
> case 1: /* blsr By,Ey */
> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
> - gen_op_update2_cc();
> - set_cc_op(s, CC_OP_BMILGB + ot);
> break;
> -
> case 2: /* blsmsk By,Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> case 3: /* blsi By, Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_neg_tl(cpu_T0, cpu_T1);
> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> default:
> goto unknown_op;
> }
> + gen_op_update2_cc();
> + set_cc_op(s, CC_OP_BMILGB + ot);
> break;
>
> default:
> --
> 2.9.4
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2017-07-12 18:58 ` Ricardo Ribalda Delgado
@ 2017-07-12 19:12 ` Richard Henderson
2017-07-12 19:25 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2017-07-12 19:12 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: qemu-devel, ehabkost, pbonzini
On 07/12/2017 08:58 AM, Ricardo Ribalda Delgado wrote:
> Hi Richard
>
> Thanks again!, When I apply this patch I get the following error:
>
> /tmp/qemu/tcg/tcg.c:2042: tcg fatal error
Bah. I misremembered that OR_TMP1 is unusable in this context.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2017-07-12 19:12 ` Richard Henderson
@ 2017-07-12 19:25 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-12 19:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
This seems to work fine with the example. But my app still throughs sigsegv :(
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 2c64d2b71ec4..564b9c6057c2 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4033,32 +4033,23 @@ static void gen_sse(CPUX86State *env,
DisasContext *s, int b,
switch (reg & 7) {
case 1: /* blsr By,Ey */
- tcg_gen_neg_tl(cpu_T1, cpu_T0);
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
- gen_op_update2_cc();
- set_cc_op(s, CC_OP_BMILGB + ot);
break;
-
case 2: /* blsmsk By,Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
case 3: /* blsi By, Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_neg_tl(cpu_T1, cpu_T0);
+ tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
default:
goto unknown_op;
}
+ gen_op_update2_cc();
+ set_cc_op(s, CC_OP_BMILGB + ot);
break;
default:
On Wed, Jul 12, 2017 at 9:12 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 07/12/2017 08:58 AM, Ricardo Ribalda Delgado wrote:
>>
>> Hi Richard
>>
>> Thanks again!, When I apply this patch I get the following error:
>>
>> /tmp/qemu/tcg/tcg.c:2042: tcg fatal error
>
>
> Bah. I misremembered that OR_TMP1 is unusable in this context.
>
>
> r~
--
Ricardo Ribalda
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2] target/i386: Fix BLSR and BLSI
2017-07-12 13:28 ` Ricardo Ribalda Delgado
2017-07-12 18:43 ` Richard Henderson
2017-07-12 18:45 ` [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI Richard Henderson
@ 2017-07-12 19:29 ` Richard Henderson
2017-07-13 20:42 ` Ricardo Ribalda Delgado
2 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2017-07-12 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: ricardo.ribalda, ehabkost, pbonzini
The implementation of these two instructions was swapped.
At the same time, unify the setup of eflags for the insn group.
Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/i386/translate.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 9d5f1c3..69d3787 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4031,34 +4031,26 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
ot = mo_64_32(s->dflag);
gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
+ tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
switch (reg & 7) {
case 1: /* blsr By,Ey */
- tcg_gen_neg_tl(cpu_T1, cpu_T0);
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
- gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
- gen_op_update2_cc();
- set_cc_op(s, CC_OP_BMILGB + ot);
break;
-
case 2: /* blsmsk By,Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
+ tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
case 3: /* blsi By, Ey */
- tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
- tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
- tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
- tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
- set_cc_op(s, CC_OP_BMILGB + ot);
+ tcg_gen_neg_tl(cpu_T1, cpu_T0);
+ tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
break;
-
default:
goto unknown_op;
}
+ tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
+ gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
+ set_cc_op(s, CC_OP_BMILGB + ot);
break;
default:
--
2.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/i386: Fix BLSR and BLSI
2017-07-12 19:29 ` [Qemu-devel] [PATCH v2] " Richard Henderson
@ 2017-07-13 20:42 ` Ricardo Ribalda Delgado
2017-07-13 21:55 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-13 20:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
Hi Richard
The simple example works as expected, but my big application
(gobject-introspection) still crashes with sigsegv :(.
it seems to be something related to the bmi and tbm instructions. If I
disable them in gcc ( -mno-bmi -mno-tbm), the application
runs ok.
A look at qemu's code does not show anything obvious, but I am not
that familiar with qemu source yet to find something like this through
static analysis.
My plan (as soon as I have some time) is to create a small set of apps
to validate bmi/tbm/ (Are you aware of something already existing for
this?)
My stupid guess is that maybe the ops are switched, or the flags are
not properly modified.
If you want, I can share the application that crashes with you, just
be aware that the number of dependencies is considerable.
BTW I can only run the gdb stub on version 2.8.0. On git HEAD I am getting only:
Quit
(gdb) c
Continuing.
warning: Remote failure reply: E22
Program stopped.
0x00000040017bac07 in ?? ()
(gdb) c
Continuing.
Thanks for your help, it is greatly appreciated!
On Wed, Jul 12, 2017 at 9:29 PM, Richard Henderson <rth@twiddle.net> wrote:
> The implementation of these two instructions was swapped.
> At the same time, unify the setup of eflags for the insn group.
>
> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/i386/translate.c | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 9d5f1c3..69d3787 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -4031,34 +4031,26 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
> ot = mo_64_32(s->dflag);
> gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
>
> + tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> switch (reg & 7) {
> case 1: /* blsr By,Ey */
> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> - gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
> - gen_op_update2_cc();
> - set_cc_op(s, CC_OP_BMILGB + ot);
> break;
> -
> case 2: /* blsmsk By,Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> case 3: /* blsi By, Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_neg_tl(cpu_T1, cpu_T0);
> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> default:
> goto unknown_op;
> }
> + tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> + gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
> + set_cc_op(s, CC_OP_BMILGB + ot);
> break;
>
> default:
> --
> 2.9.4
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/i386: Fix BLSR and BLSI
2017-07-13 20:42 ` Ricardo Ribalda Delgado
@ 2017-07-13 21:55 ` Ricardo Ribalda Delgado
2017-07-13 22:49 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-13 21:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
Hi again
Some progress here, I think that I have found a bug in andn, I have
already send a patch.
I have made a rudimentary testcase for bmi. I will try tomorrow o
build something similar for tbm.
For reference, I am using this script:
for a in $(seq 0 255); do for b in $(seq 0 255); do for c in $(seq 0
255); do /tmp/qemu/x86_64-linux-user/qemu-x86_64 -cpu qemu64,+bmi1
./a.out $a $b $c >/tmp/res.qemu; ./pc $a $b $c > /tmp/res.pc; if !
diff /tmp/res.pc /tmp/res.qemu; then echo $a $b $c; fi ; done ; done
; done
with this build options
gcc kk.c -mbmi -O3 -Wall
gcc kk.c -march=native -O3 -Wall -o pc
and this code:
#include <stdio.h>
#include <stdlib.h>
#include <x86intrin.h>
long test_blsr(long val){
return (val & (val - 1));
}
long test_blsi(long val){
return (val & (-val));
}
long test_blmsk(long val){
return (val ^ (val -1));
}
long test_andn(long v1, long v2){
return (~v1 & v2);
}
long test_tzcnt(long val) {
#ifdef PC
return val ? __builtin_ctz(val) : 32;
#else
return __tzcnt_u32(val);
#endif
}
long test_bextr(long src, unsigned char start, unsigned char len) {
#ifdef PC
return (src >> start) & ((1 << len)-1);
#else
return __bextr_u32(src, start | len <<8);
#endif
}
int main(int argc, char *argv[]) {
long op1, op2, op3;
long ret;
if (argc < 4) {
fprintf(stderr, "use %s op1 op2 op3\n", argv[0]);
return -1;
}
op1 = strtoul(argv[1], NULL, 0);
op2 = strtoul(argv[2], NULL, 0);
op3 = strtoul(argv[3], NULL, 0);
fprintf(stdout, "op 1 %ld (0x%lx)\n", op1, op1);
fprintf(stdout, "op 2 %ld (0x%lx)\n", op2, op2);
ret = test_blsr(op1);
fprintf(stdout, "blsr %ld (0x%lx)\n",ret, ret);
ret = test_blsi(op1);
fprintf(stdout, "blsi %ld (0x%lx)\n",ret, ret);
ret = test_blmsk(op1);
fprintf(stdout, "blmsk %ld (0x%lx)\n",ret, ret);
ret = test_andn(op1,op2);
fprintf(stdout, "andn %ld (0x%lx)\n",ret, ret);
ret = test_tzcnt(op1);
fprintf(stdout, "tzcnt %ld (0x%lx)\n",ret, ret);
ret = test_bextr(op1, op2, op3);
fprintf(stdout, "bextr %ld (0x%lx)\n",ret, ret);
return 0;
}
On Thu, Jul 13, 2017 at 10:42 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hi Richard
>
> The simple example works as expected, but my big application
> (gobject-introspection) still crashes with sigsegv :(.
>
> it seems to be something related to the bmi and tbm instructions. If I
> disable them in gcc ( -mno-bmi -mno-tbm), the application
> runs ok.
>
> A look at qemu's code does not show anything obvious, but I am not
> that familiar with qemu source yet to find something like this through
> static analysis.
>
> My plan (as soon as I have some time) is to create a small set of apps
> to validate bmi/tbm/ (Are you aware of something already existing for
> this?)
> My stupid guess is that maybe the ops are switched, or the flags are
> not properly modified.
>
> If you want, I can share the application that crashes with you, just
> be aware that the number of dependencies is considerable.
>
> BTW I can only run the gdb stub on version 2.8.0. On git HEAD I am getting only:
>
> Quit
> (gdb) c
> Continuing.
> warning: Remote failure reply: E22
>
> Program stopped.
> 0x00000040017bac07 in ?? ()
> (gdb) c
> Continuing.
>
>
>
> Thanks for your help, it is greatly appreciated!
>
> On Wed, Jul 12, 2017 at 9:29 PM, Richard Henderson <rth@twiddle.net> wrote:
>> The implementation of these two instructions was swapped.
>> At the same time, unify the setup of eflags for the insn group.
>>
>> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>> target/i386/translate.c | 26 +++++++++-----------------
>> 1 file changed, 9 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 9d5f1c3..69d3787 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -4031,34 +4031,26 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>> ot = mo_64_32(s->dflag);
>> gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
>>
>> + tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>> switch (reg & 7) {
>> case 1: /* blsr By,Ey */
>> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
>> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
>> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>> - gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
>> - gen_op_update2_cc();
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> break;
>> -
>> case 2: /* blsmsk By,Ey */
>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
>> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
>> break;
>> -
>> case 3: /* blsi By, Ey */
>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> + tcg_gen_neg_tl(cpu_T1, cpu_T0);
>> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>> break;
>> -
>> default:
>> goto unknown_op;
>> }
>> + tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>> + gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
>> + set_cc_op(s, CC_OP_BMILGB + ot);
>> break;
>>
>> default:
>> --
>> 2.9.4
>>
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/i386: Fix BLSR and BLSI
2017-07-13 21:55 ` Ricardo Ribalda Delgado
@ 2017-07-13 22:49 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2017-07-13 22:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
Hi
For completion. This is my poor man tbm test. It has run for 5 minutes
with no errors
gcc tbm.c -O3 -march=native -o pc
gcc tbm.c -mtbm -O3
for a in $(seq 0 65535); do /tmp/qemu/x86_64-linux-user/qemu-x86_64
-cpu qemu64,+tbm ./a.out $a >/tmp/res.qemu ; ./pc $a > /tmp/res.pc ;
if ! diff /tmp/res.pc /tmp/res.qemu; then echo $a ; fi ; done
#include <stdio.h>
#include <stdlib.h>
#include <x86intrin.h>
long test_bextr(long src) {
unsigned char start =1, len=3;
return (src >> start) & ((1 << len)-1);
}
long test_blcfill(long val){
return val & (val + 1);
}
long test_blci(long val){
return val | ~(val + 1);
}
long test_blcic(long val){
return ~val & (val + 1);
}
long test_blcmsk(long val){
return val ^ (val + 1);
}
long test_blcs(long val){
return val | (val + 1);
}
long test_blsfill(long val){
return val | (val - 1);
}
long test_blsic(long val){
return ~val | (val - 1);
}
long test_t1mskc(long val){
return ~val | (val + 1);
}
long test_tzmsk(long val){
return ~val & (val - 1);
}
int main(int argc, char *argv[]) {
long op1;
long ret;
if (argc < 2) {
fprintf(stderr, "use %s op1 \n", argv[0]);
return -1;
}
op1 = strtoul(argv[1], NULL, 0);
fprintf(stdout, "op 1 %ld (0x%lx)\n", op1, op1);
ret = test_bextr(op1);
fprintf(stdout, "bextr %ld (0x%lx)\n",ret, ret);
ret = test_blcfill(op1);
fprintf(stdout, "blcfill %ld (0x%lx)\n",ret, ret);
ret = test_blci(op1);
fprintf(stdout, "blci %ld (0x%lx)\n",ret, ret);
ret = test_blcic(op1);
fprintf(stdout, "blcic %ld (0x%lx)\n",ret, ret);
ret = test_blcmsk(op1);
fprintf(stdout, "blcmsk %ld (0x%lx)\n",ret, ret);
ret = test_blcs(op1);
fprintf(stdout, "blcs %ld (0x%lx)\n",ret, ret);
ret = test_blsfill(op1);
fprintf(stdout, "blsfill %ld (0x%lx)\n",ret, ret);
ret = test_blsic(op1);
fprintf(stdout, "blsic %ld (0x%lx)\n",ret, ret);
ret = test_t1mskc(op1);
fprintf(stdout, "t1mskc %ld (0x%lx)\n",ret, ret);
ret = test_tzmsk(op1);
fprintf(stdout, "tzmsk %ld (0x%lx)\n",ret, ret);
return 0;
}
On Thu, Jul 13, 2017 at 11:55 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hi again
>
> Some progress here, I think that I have found a bug in andn, I have
> already send a patch.
>
> I have made a rudimentary testcase for bmi. I will try tomorrow o
> build something similar for tbm.
>
> For reference, I am using this script:
>
> for a in $(seq 0 255); do for b in $(seq 0 255); do for c in $(seq 0
> 255); do /tmp/qemu/x86_64-linux-user/qemu-x86_64 -cpu qemu64,+bmi1
> ./a.out $a $b $c >/tmp/res.qemu; ./pc $a $b $c > /tmp/res.pc; if !
> diff /tmp/res.pc /tmp/res.qemu; then echo $a $b $c; fi ; done ; done
> ; done
>
> with this build options
> gcc kk.c -mbmi -O3 -Wall
> gcc kk.c -march=native -O3 -Wall -o pc
>
> and this code:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <x86intrin.h>
>
>
> long test_blsr(long val){
>
> return (val & (val - 1));
> }
>
> long test_blsi(long val){
>
> return (val & (-val));
> }
>
> long test_blmsk(long val){
>
> return (val ^ (val -1));
> }
>
> long test_andn(long v1, long v2){
>
> return (~v1 & v2);
> }
>
> long test_tzcnt(long val) {
> #ifdef PC
> return val ? __builtin_ctz(val) : 32;
> #else
> return __tzcnt_u32(val);
> #endif
> }
>
> long test_bextr(long src, unsigned char start, unsigned char len) {
> #ifdef PC
> return (src >> start) & ((1 << len)-1);
> #else
> return __bextr_u32(src, start | len <<8);
> #endif
> }
>
> int main(int argc, char *argv[]) {
> long op1, op2, op3;
> long ret;
>
> if (argc < 4) {
> fprintf(stderr, "use %s op1 op2 op3\n", argv[0]);
> return -1;
> }
> op1 = strtoul(argv[1], NULL, 0);
> op2 = strtoul(argv[2], NULL, 0);
> op3 = strtoul(argv[3], NULL, 0);
>
> fprintf(stdout, "op 1 %ld (0x%lx)\n", op1, op1);
> fprintf(stdout, "op 2 %ld (0x%lx)\n", op2, op2);
>
> ret = test_blsr(op1);
> fprintf(stdout, "blsr %ld (0x%lx)\n",ret, ret);
>
> ret = test_blsi(op1);
> fprintf(stdout, "blsi %ld (0x%lx)\n",ret, ret);
>
> ret = test_blmsk(op1);
> fprintf(stdout, "blmsk %ld (0x%lx)\n",ret, ret);
>
> ret = test_andn(op1,op2);
> fprintf(stdout, "andn %ld (0x%lx)\n",ret, ret);
>
> ret = test_tzcnt(op1);
> fprintf(stdout, "tzcnt %ld (0x%lx)\n",ret, ret);
>
> ret = test_bextr(op1, op2, op3);
> fprintf(stdout, "bextr %ld (0x%lx)\n",ret, ret);
>
> return 0;
> }
>
> On Thu, Jul 13, 2017 at 10:42 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>> Hi Richard
>>
>> The simple example works as expected, but my big application
>> (gobject-introspection) still crashes with sigsegv :(.
>>
>> it seems to be something related to the bmi and tbm instructions. If I
>> disable them in gcc ( -mno-bmi -mno-tbm), the application
>> runs ok.
>>
>> A look at qemu's code does not show anything obvious, but I am not
>> that familiar with qemu source yet to find something like this through
>> static analysis.
>>
>> My plan (as soon as I have some time) is to create a small set of apps
>> to validate bmi/tbm/ (Are you aware of something already existing for
>> this?)
>> My stupid guess is that maybe the ops are switched, or the flags are
>> not properly modified.
>>
>> If you want, I can share the application that crashes with you, just
>> be aware that the number of dependencies is considerable.
>>
>> BTW I can only run the gdb stub on version 2.8.0. On git HEAD I am getting only:
>>
>> Quit
>> (gdb) c
>> Continuing.
>> warning: Remote failure reply: E22
>>
>> Program stopped.
>> 0x00000040017bac07 in ?? ()
>> (gdb) c
>> Continuing.
>>
>>
>>
>> Thanks for your help, it is greatly appreciated!
>>
>> On Wed, Jul 12, 2017 at 9:29 PM, Richard Henderson <rth@twiddle.net> wrote:
>>> The implementation of these two instructions was swapped.
>>> At the same time, unify the setup of eflags for the insn group.
>>>
>>> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>>> ---
>>> target/i386/translate.c | 26 +++++++++-----------------
>>> 1 file changed, 9 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>>> index 9d5f1c3..69d3787 100644
>>> --- a/target/i386/translate.c
>>> +++ b/target/i386/translate.c
>>> @@ -4031,34 +4031,26 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>>> ot = mo_64_32(s->dflag);
>>> gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
>>>
>>> + tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>>> switch (reg & 7) {
>>> case 1: /* blsr By,Ey */
>>> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
>>> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
>>> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>>> - gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
>>> - gen_op_update2_cc();
>>> - set_cc_op(s, CC_OP_BMILGB + ot);
>>> break;
>>> -
>>> case 2: /* blsmsk By,Ey */
>>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>>> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
>>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>>> - set_cc_op(s, CC_OP_BMILGB + ot);
>>> + tcg_gen_subi_tl(cpu_T1, cpu_T0, 1);
>>> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
>>> break;
>>> -
>>> case 3: /* blsi By, Ey */
>>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>>> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
>>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>>> - set_cc_op(s, CC_OP_BMILGB + ot);
>>> + tcg_gen_neg_tl(cpu_T1, cpu_T0);
>>> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>>> break;
>>> -
>>> default:
>>> goto unknown_op;
>>> }
>>> + tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>>> + gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
>>> + set_cc_op(s, CC_OP_BMILGB + ot);
>>> break;
>>>
>>> default:
>>> --
>>> 2.9.4
>>>
>>
>>
>>
>> --
>> Ricardo Ribalda
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2017-07-12 18:45 ` [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI Richard Henderson
2017-07-12 18:58 ` Ricardo Ribalda Delgado
@ 2018-06-06 9:13 ` Ricardo Ribalda Delgado
2018-06-06 12:32 ` Paolo Bonzini
1 sibling, 1 reply; 16+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-06-06 9:13 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, ehabkost, pbonzini
Hi Richard,
I cannot find this patch on qemu master branch. Do you need any help
to get this done?
Thanks!
On Wed, Jul 12, 2017 at 8:45 PM Richard Henderson <rth@twiddle.net> wrote:
>
> The implementation of these two instructions was swapped.
> At the same time, unify the setup of eflags for the insn group.
>
> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/i386/translate.c | 25 ++++++++-----------------
> 1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 8365a6d..087a2e6 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -4029,36 +4029,27 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
> goto illegal_op;
> }
> ot = mo_64_32(s->dflag);
> - gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
> + gen_ldst_modrm(env, s, modrm, ot, OR_TMP1, 0);
>
> switch (reg & 7) {
> case 1: /* blsr By,Ey */
> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
> - gen_op_update2_cc();
> - set_cc_op(s, CC_OP_BMILGB + ot);
> break;
> -
> case 2: /* blsmsk By,Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> case 3: /* blsi By, Ey */
> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
> - set_cc_op(s, CC_OP_BMILGB + ot);
> + tcg_gen_neg_tl(cpu_T0, cpu_T1);
> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
> break;
> -
> default:
> goto unknown_op;
> }
> + gen_op_update2_cc();
> + set_cc_op(s, CC_OP_BMILGB + ot);
> break;
>
> default:
> --
> 2.9.4
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI
2018-06-06 9:13 ` Ricardo Ribalda Delgado
@ 2018-06-06 12:32 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-06-06 12:32 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Richard Henderson; +Cc: qemu-devel, ehabkost
On 06/06/2018 11:13, Ricardo Ribalda Delgado wrote:
> Hi Richard,
>
> I cannot find this patch on qemu master branch. Do you need any help
> to get this done?
I queued it now, thanks for the reminder!
Paolo
> Thanks!
> On Wed, Jul 12, 2017 at 8:45 PM Richard Henderson <rth@twiddle.net> wrote:
>>
>> The implementation of these two instructions was swapped.
>> At the same time, unify the setup of eflags for the insn group.
>>
>> Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>> target/i386/translate.c | 25 ++++++++-----------------
>> 1 file changed, 8 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 8365a6d..087a2e6 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -4029,36 +4029,27 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>> goto illegal_op;
>> }
>> ot = mo_64_32(s->dflag);
>> - gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
>> + gen_ldst_modrm(env, s, modrm, ot, OR_TMP1, 0);
>>
>> switch (reg & 7) {
>> case 1: /* blsr By,Ey */
>> - tcg_gen_neg_tl(cpu_T1, cpu_T0);
>> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
>> tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>> gen_op_mov_reg_v(ot, s->vex_v, cpu_T0);
>> - gen_op_update2_cc();
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> break;
>> -
>> case 2: /* blsmsk By,Ey */
>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>> - tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_cc_src);
>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> + tcg_gen_subi_tl(cpu_T0, cpu_T1, 1);
>> + tcg_gen_xor_tl(cpu_T0, cpu_T0, cpu_T1);
>> break;
>> -
>> case 3: /* blsi By, Ey */
>> - tcg_gen_mov_tl(cpu_cc_src, cpu_T0);
>> - tcg_gen_subi_tl(cpu_T0, cpu_T0, 1);
>> - tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_cc_src);
>> - tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
>> - set_cc_op(s, CC_OP_BMILGB + ot);
>> + tcg_gen_neg_tl(cpu_T0, cpu_T1);
>> + tcg_gen_and_tl(cpu_T0, cpu_T0, cpu_T1);
>> break;
>> -
>> default:
>> goto unknown_op;
>> }
>> + gen_op_update2_cc();
>> + set_cc_op(s, CC_OP_BMILGB + ot);
>> break;
>>
>> default:
>> --
>> 2.9.4
>>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-06-06 12:33 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-11 21:21 [Qemu-devel] [PATCH 0/2] target/i386: Implement all TBM instructions Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 1/2] target/i386: Decode AMD XOP prefix Richard Henderson
2017-07-11 21:21 ` [Qemu-devel] [PATCH 2/2] target/i386: Implement all TBM instructions Richard Henderson
2017-07-12 4:04 ` [Qemu-devel] [PATCH 0/2] " Richard Henderson
2017-07-12 13:28 ` Ricardo Ribalda Delgado
2017-07-12 18:43 ` Richard Henderson
2017-07-12 18:45 ` [Qemu-devel] [PATCH] target/i386: Fix BLSR and BLSI Richard Henderson
2017-07-12 18:58 ` Ricardo Ribalda Delgado
2017-07-12 19:12 ` Richard Henderson
2017-07-12 19:25 ` Ricardo Ribalda Delgado
2018-06-06 9:13 ` Ricardo Ribalda Delgado
2018-06-06 12:32 ` Paolo Bonzini
2017-07-12 19:29 ` [Qemu-devel] [PATCH v2] " Richard Henderson
2017-07-13 20:42 ` Ricardo Ribalda Delgado
2017-07-13 21:55 ` Ricardo Ribalda Delgado
2017-07-13 22:49 ` Ricardo Ribalda Delgado
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.