Re: [Xen-devel] [PATCH 2/2] x86emul: slightly alter AVX512 exception checking conditionals

2018-12-10 Thread Jan Beulich
>>> On 10.12.18 at 15:00,  wrote:
> On 07/12/2018 11:18, Jan Beulich wrote:
>> While actually benign (operands are either register or memory ones
>> anyway), I think it is better to use != instead of == for such checks.
>>
>> Signed-off-by: Jan Beulich 
> 
> I don't see the point of making this change.  Code is easier to follow
> when there are fewer negations.

Is != a negation? I ask because in general I agree with that position
of yours. The idea here is that besides OP_REG and OP_MEM we
also have OP_IMM and OP_NONE. By carefully using != and == I'm
trying to make sure (and not the least document) that (in the cases
here) we don't let things go through in case the value (mistakenly)
is one of the "impossible" (in these contexts) two.

> But whatever - Acked-by: Andrew Cooper 

Thanks.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/2] x86emul: slightly alter AVX512 exception checking conditionals

2018-12-10 Thread Andrew Cooper
On 07/12/2018 11:18, Jan Beulich wrote:
> While actually benign (operands are either register or memory ones
> anyway), I think it is better to use != instead of == for such checks.
>
> Signed-off-by: Jan Beulich 

I don't see the point of making this change.  Code is easier to follow
when there are fewer negations.

But whatever - Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/2] x86emul: slightly alter AVX512 exception checking conditionals

2018-12-07 Thread Jan Beulich
While actually benign (operands are either register or memory ones
anyway), I think it is better to use != instead of == for such checks.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5918,11 +5918,11 @@ x86_emulate(
 CASE_SIMD_ALL_FP(_EVEX, 0x0f, 0x5e):/* vdiv{p,s}{s,d} 
[xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
 CASE_SIMD_ALL_FP(_EVEX, 0x0f, 0x5f):/* vmax{p,s}{s,d} 
[xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
 generate_exception_if((evex.w != (evex.pfx & VEX_PREFIX_DOUBLE_MASK) ||
-   (ea.type == OP_MEM && evex.br &&
+   (ea.type != OP_REG && evex.br &&
 (evex.pfx & VEX_PREFIX_SCALAR_MASK))),
   EXC_UD);
 host_and_vcpu_must_have(avx512f);
-if ( ea.type == OP_MEM || !evex.br )
+if ( ea.type != OP_REG || !evex.br )
 avx512_vlen_check(evex.pfx & VEX_PREFIX_SCALAR_MASK);
 simd_zmm:
 get_fpu(X86EMUL_FPU_zmm);
@@ -7614,12 +7614,12 @@ x86_emulate(
 
 CASE_SIMD_ALL_FP(_EVEX, 0x0f, 0xc2): /* vcmp{p,s}{s,d} 
$imm8,[xyz]mm/mem,[xyz]mm,k{k} */
 generate_exception_if((evex.w != (evex.pfx & VEX_PREFIX_DOUBLE_MASK) ||
-   (ea.type == OP_MEM && evex.br &&
+   (ea.type != OP_REG && evex.br &&
 (evex.pfx & VEX_PREFIX_SCALAR_MASK)) ||
!evex.r || !evex.R || evex.z),
   EXC_UD);
 host_and_vcpu_must_have(avx512f);
-if ( ea.type == OP_MEM || !evex.br )
+if ( ea.type != OP_REG || !evex.br )
 avx512_vlen_check(evex.pfx & VEX_PREFIX_SCALAR_MASK);
 simd_imm8_zmm:
 if ( (d & SrcMask) == SrcImmByte )
@@ -8509,7 +8509,7 @@ x86_emulate(
 case X86EMUL_OPC_EVEX_66(0x0f38, 0xbc): /* vfnmadd231p{s,d} 
[xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
 case X86EMUL_OPC_EVEX_66(0x0f38, 0xbe): /* vfnmsub231p{s,d} 
[xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
 host_and_vcpu_must_have(avx512f);
-if ( ea.type == OP_MEM || !evex.br )
+if ( ea.type != OP_REG || !evex.br )
 avx512_vlen_check(false);
 goto simd_zmm;
 





___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel