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

Paolo Bonzini <[email protected]>
Newsgroups org.nongnu.qemu-devel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.