Re: [PATCH] target/i386: fix operand order for PDEP and PEXT

[Richard Henderson]

On 11/23/20 5:14 AM, Paolo Bonzini wrote:
> For PDEP and PEXT, the mask is provided in the memory (mod+r/m)
> operand, and therefore is loaded in s->T0 by gen_ldst_modrm.
> The source is provided in the second source operand (VEX.vvvv)
> and therefore is loaded in s->T1.  Fix the order in which
> they are passed to the helpers.
> 
> Reported-by: Lenard Szolnoki <blog@lenardszolnoki.com>
> Analyzed-by: Lenard Szolnoki <blog@lenardszolnoki.com>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1605123
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

The patch itself looks fine.

> +test-i386-bmi2: CFLAGS += -mbmi2
> +run-test-i386-bmi2: QEMU_OPTS += -cpu max
> +run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max

I suspect that we still support host operating systems whose compilers do not
support -mbmi2.  This might require a bit in tests/tcg/configure.sh akin to
CROSS_CC_HAS_ARMV8_3.

> +int main(int argc, char *argv[]) {
> +    char hello[16];
> +    uint64_t ehlo = 0x202020204f4c4845ull;
> +    uint64_t mask = 0xa080800302020001ull;
> +    uint64_t result64;
> +    uint32_t result32;
> +
> +    /* 64 bits */
> +    asm volatile ("pextq   %2, %1, %0" : "=r"(result64) : "r"(ehlo), 
> "m"(mask));
> +    assert(result64 == 133);

The test is written for x86_64 not i386.  How are we preventing the test case
from being run on 32-bit in the makefile?

> +    /* 32 bits */
> +    asm volatile ("pextl   %2, %k1, %k0" : "=r"(result32) : "r"(ehlo), 
> "m"(mask));
> +    assert(result32 == 5);

Surely we should test the full 64-bit register result, and not truncate to
uint32_t in the output variable?


r~