[PATCH] target/i386: do not zero-extend BSR/BSF dest when source is zero

For the bsr and bsf instructions per the Intel SDM: "If the content
of the source operand is 0, the content of the destination operand
is undefined." The AMD64 Architecture Programmer's Manual is more
specific: it states the destination operand remains unchanged when
the source is zero. Testing on real hardware (multiple Intel and
AMD systems) confirms that when the source operand is zero, the CPU
leaves the entire 64-bit destination register untouched, including
the upper 32 bits, even when executing the 32-bit form of the
instruction (e.g. "bsr edx, ecx") in 64-bit mode.

gen_BSF()/gen_BSR() already encode this intent (see the existing
comment) by arranging for T0 to hold the correct full-width
passthrough value when the source is zero. However, that correct
value was then handed to the generic register writeback path
(gen_writeback), which for a 32-bit destination unconditionally
applies tcg_gen_ext32u_tl() and clears the upper 32 bits regardless
of what gen_BSF()/gen_BSR() had just computed.

This patch bypasses the generic writeback for this specific case
(64-bit mode, 32-bit operand size) and writes the already-correct
value directly to the register instead.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4132
Signed-off-by: Simon Scherer <[email protected]>
---
 target/i386/tcg/emit.c.inc | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 473f415766..c555ef49aa 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1409,6 +1409,17 @@ static void gen_BSF(DisasContext *s, X86DecodedInsn *decode)
      * by passing the output as the value to return upon zero.
      */
     tcg_gen_ctz_tl(s->T0, s->T0, s->T1);
+
+    /*
+     * Bypass the gen_writeback: for a 32-bit destination in 64-bit
+     * mode it would zero-extend the upper 32 bits, but T0 already holds
+     * the exact final register value for both the zero- and non-zero-
+     * source cases (see comment above).
+     */
+    if (CODE64(s) && ot == MO_32) {
+        tcg_gen_mov_tl(cpu_regs[decode->op[0].n], s->T0);
+        decode->op[0].unit = X86_OP_SKIP;
+    }
 }
 
 /* Non-standard convention - on entry T0 is zero-extended input, T1 is the output.  */
@@ -1431,6 +1442,12 @@ static void gen_BSR(DisasContext *s, X86DecodedInsn *decode)
     tcg_gen_xori_tl(s->T1, s->T1, TARGET_LONG_BITS - 1);
     tcg_gen_clz_tl(s->T0, s->T0, s->T1);
     tcg_gen_xori_tl(s->T0, s->T0, TARGET_LONG_BITS - 1);
+
+    /* See gen_BSF() above. */
+    if (CODE64(s) && ot == MO_32) {
+        tcg_gen_mov_tl(cpu_regs[decode->op[0].n], s->T0);
+        decode->op[0].unit = X86_OP_SKIP;
+    }
 }
 
 static void gen_BSWAP(DisasContext *s, X86DecodedInsn *decode)
-- 
2.53.0