Re: [PATCH 10/11] x86emul: handle AVX512-FP16 conversion to/from (packed) int{32,64} insns

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.08.2022 21:09, Andrew Cooper wrote:
> On 15/06/2022 11:31, Jan Beulich wrote:
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/x86_emulate/decode.c
>> +++ b/xen/arch/x86/x86_emulate/decode.c
>> @@ -1489,12 +1489,25 @@ int x86emul_decode(struct x86_emulate_st
>>                      s->simd_size = simd_scalar_vexw;
>>                  break;
>>  
>> +            case 0x2a: /* vcvtsi2sh */
>> +                break;
>> +
>> +            case 0x2c: case 0x2d: /* vcvt{,t}sh2si */
>> +                if ( s->evex.pfx == vex_f3 )
>> +                    s->fp16 = true;
>> +                break;
>> +
>>              case 0x2e: case 0x2f: /* v{,u}comish */
>>                  if ( !s->evex.pfx )
>>                      s->fp16 = true;
>>                  s->simd_size = simd_none;
>>                  break;
>>  
>> +            case 0x5b: /* vcvt{d,q}q2ph, vcvt{,t}ph2dq */
>> +                if ( s->evex.pfx && s->evex.pfx != vex_f2 )
>> +                    s->fp16 = true;
>> +                break;
>> +
>>              case 0x6e: /* vmovw r/m16, xmm */
>>                  d = (d & ~SrcMask) | SrcMem16;
>>                  /* fall through */
>> @@ -1504,6 +1517,17 @@ int x86emul_decode(struct x86_emulate_st
>>                  s->simd_size = simd_none;
>>                  break;
>>  
>> +            case 0x78: case 0x79: /* vcvt{,t}ph2u{d,q}q, vcvt{,t}sh2usi */
>> +                if ( s->evex.pfx != vex_f2 )
>> +                    s->fp16 = true;
>> +                break;
>> +
>> +            case 0x7a: /* vcvttph2qq, vcvtu{d,q}q2ph */
>> +            case 0x7b: /* vcvtph2qq, vcvtusi2sh */
>> +                if ( s->evex.pfx == vex_66 )
>> +                    s->fp16 = true;
>> +                break;
>> +
>>              case 0x7c: /* vcvttph2{,u}w */
>>              case 0x7d: /* vcvtph2{,u}w / vcvt{,u}w2ph */
>>                  d = DstReg | SrcMem | TwoOp;
>> @@ -1515,10 +1539,34 @@ int x86emul_decode(struct x86_emulate_st
>>  
>>              switch ( b )
>>              {
>> +            case 0x78:
>> +            case 0x79:
>> +                /* vcvt{,t}ph2u{d,q}q need special casing */
>> +                if ( s->evex.pfx <= vex_66 )
>> +                {
>> +                    if ( !s->evex.brs )
>> +                        disp8scale -= 1 + (s->evex.pfx == vex_66);
>> +                    break;
>> +                }
>> +                /* vcvt{,t}sh2usi needs special casing: fall through */
>> +            case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */
>> +                disp8scale = 1;
>> +                break;
>> +
>>              case 0x5a: /* vcvtph2pd needs special casing */
>>                  if ( !s->evex.pfx && !s->evex.brs )
>>                      disp8scale -= 2;
>>                  break;
>> +
>> +            case 0x5b: /* vcvt{,t}ph2dq need special casing */
>> +                if ( s->evex.pfx && !s->evex.brs )
>> +                    --disp8scale;
>> +                break;
>> +
>> +            case 0x7a: case 0x7b: /* vcvt{,t}ph2qq need special casing */
>> +                if ( s->evex.pfx == vex_66 && !s->evex.brs )
>> +                    disp8scale = s->evex.brs ? 1 : 2 + s->evex.lr;
>> +                break;
>>              }
>>  
>>              break;
> 
> Perhaps here, in terms of overriding...

>From all I can tell, the s->fp16 setting would better remain here, as
it's (embedded) prefix dependent. As are the disp8scale adjustments (if
you go look, you'll find similar pre-existing code a little higher up
in the file.)

Jan