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

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

[Simon Scherer]

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 <scherer.simon89@gmail.com>
---
 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

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

[Paolo Bonzini]

On 8/5/26 17:18, Simon Scherer wrote:
> 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.

I think this is the same as using d64 instead of v?

     case X86_SIZE_v:  /* 16/32/64-bit, based on operand size */
         *ot = s->dflag;
         return true;
...
     case X86_SIZE_d64:  /* Default to 64-bit in 64-bit mode */
         *ot = CODE64(s) && s->dflag == MO_32 ? MO_64 : s->dflag;
         return true;

That is, something like:

     /* For BSF, pass 2op as the third operand so that we can use zextT0.
      * Use d64 because ctz already zero-extends the full 64-bit result,
      * and v would zero-extend the output register if the input is zero.
      */
     static const X86OpEntry opcodes_0FBC[4] = {
         X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0),
         X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0), /* 0x66 */
         X86_OP_ENTRYwr(TZCNT, G,v,   E,v,            zextT0), /* 0xf3 */
         X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0), /* 0xf2 */
     };

Thanks,

Paolo

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

[Simon Scherer]

You are right, but E needs to stay v, not d64. Otherwise the source
read also widens to 64 bits. Counterexample: bsr edx, ecx with
rcx=0x180000000 gives 31 with E,v (correct) but 32 with E,d64 (wrong),
since it scans all of rcx instead of just ecx. Otherwise, it looks
good. Will send a v2 with just the decode-table change.

On Fri, Aug 7, 2026 at 3:17 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 8/5/26 17:18, Simon Scherer wrote:
> > 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.
>
> I think this is the same as using d64 instead of v?
>
>      case X86_SIZE_v:  /* 16/32/64-bit, based on operand size */
>          *ot = s->dflag;
>          return true;
> ...
>      case X86_SIZE_d64:  /* Default to 64-bit in 64-bit mode */
>          *ot = CODE64(s) && s->dflag == MO_32 ? MO_64 : s->dflag;
>          return true;
>
> That is, something like:
>
>      /* For BSF, pass 2op as the third operand so that we can use zextT0.
>       * Use d64 because ctz already zero-extends the full 64-bit result,
>       * and v would zero-extend the output register if the input is zero.
>       */
>      static const X86OpEntry opcodes_0FBC[4] = {
>          X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0),
>          X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0), /* 0x66 */
>          X86_OP_ENTRYwr(TZCNT, G,v,   E,v,            zextT0), /* 0xf3 */
>          X86_OP_ENTRY3(BSF,    G,d64, E,d64, 2op,d64, zextT0), /* 0xf2 */
>      };
>
> Thanks,
>
> Paolo
>