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

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 <[email protected]> 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
>