Re: [Qemu-devel] [PATCH 16/25] tcg/i386: Handle ctz and clz opcodes

[Richard Henderson <rth@twiddle.net>]

On 11/18/2016 01:48 PM, Bastian Koppelmann wrote:
> On 11/18/2016 12:03 AM, Richard Henderson wrote:
>> On 11/17/2016 11:09 PM, Bastian Koppelmann wrote:
>>> On 11/17/2016 08:59 PM, Richard Henderson wrote:
>>>> On 11/17/2016 08:53 PM, Richard Henderson wrote:
>>>>> On 11/17/2016 05:50 PM, Bastian Koppelmann wrote:
>>>>>> On 11/16/2016 08:25 PM, Richard Henderson wrote:
>>>>>>> +
>>>>>>> +    OP_32_64(clz):
>>>>>>> +        if (const_args[2]) {
>>>>>>> +            tcg_debug_assert(have_bmi1);
>>>>>>> +            tcg_debug_assert(args[2] == (rexw ? 64 : 32));
>>>>>>> +            tcg_out_modrm(s, OPC_LZCNT + rexw, args[0], args[1]);
>>>>>>> +        } else {
>>>>>>> +            /* ??? See above.  */
>>>>>>> +            tcg_out_modrm(s, OPC_BSR + rexw, args[0], args[1]);
>>>>>>
>>>>>> The Intel ISA manual states that it find the bit index of the most
>>>>>> significant bit, where the least significant bit is index 0. So for
>>>>>> the
>>>>>> input 0x2 this should return 1. However this is not the number of
>>>>>> leading zeros.
>>>>>
>>>>> Oh, of course you're right.  I thought I was testing this, but while
>>>>> alpha does
>>>>> have this operation, it turns out it isn't used much.
>>>>
>>>> Alternately, what I tested was on a haswell machine, which takes the
>>>> LZCNT path, which *does* produce the intended results.  Just the BSR
>>>> path doesn't.
>>>
>>> Luckily my old laptop is a Core 2 Duo without LZCNT :)
>>
>> Heh.  Well, I've given it another few tests with LZCNT hacked off, and
>> with i686 32-bit.  Here's an incremental update.  Wherein I also note
>> that lzcnt isn't in the same cpuid flag as tzcnt.  Double whoops.
>
> My processor[1] seems to lie about the LZCNT cpuid flag. It says it has
> LZCNT but executes it as BSR. According to [2] ABM flag is used to
> indicate LZCNT support.

Yes, the gcc cpuid.h comment for the lzcnt bit, i.e. to which leaf it should 
apply, is wrong.  I'll get that fixed in the next revision.


r~