search

DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.64k stars 560 forks source link

vcvtps2ph, vcvtpd2dq and other's register mnemonic doesn't match gas' and Intel's. #3639

Open hgreving2304 opened 5 years ago

hgreving2304 commented 5 years ago

The AVX and AVX-512 form of vcvtps2ph that takes ymm and zmm sources is specified by Intel like this:

VEX.128.66.0F3A.W0 1D /r ib
VCVTPS2PH xmm1/m64, xmm2,
imm8
VEX.256.66.0F3A.W0 1D /r ib
VCVTPS2PH xmm1/m128, ymm2,
imm8
EVEX.128.66.0F3A.W0 1D /r ib
VCVTPS2PH xmm1/m64 {k1}{z},
xmm2, imm8
EVEX.256.66.0F3A.W0 1D /r ib
VCVTPS2PH xmm1/m128 {k1}{z},
ymm2, imm8
EVEX.512.66.0F3A.W0 1D /r ib
VCVTPS2PH ymm1/m256 {k1}{z},
zmm2{sae}, imm8

Note that the destination for ymm source is specified as xmm. However, the instruction does write zero [MAXVL-1:128]. There are plenty of examples in AVX and AVX-512 of instructions doing the same and their destination mnemonic always matches the source mnemonic. There doesn't seem to be anything special about vcvtps2ph in this regard.

DR does currently not make a special case for this instruction. It looks like that gas does.

  asm volatile("vcvtps2ph $1, %%xmm0, %%xmm1\n\t" : : :);
  /* Note that gas wants the destination's register mnemonic to be xmm */
  asm volatile("vcvtps2ph $1, %%ymm0, %%xmm1\n\t" : : :);                                                                                                                                                                                                                                
  /* Note that gas wants the destination's register mnemonic to be ymm */
  asm volatile("vcvtps2ph $1, %%zmm0, %%ymm1\n\t" : : :);

Unless it somehow becomes a problem, I would like it as-is, but I am filing this issue so this is known, in case this will come up again later.

hgreving2304 commented 5 years ago

xref #1312

hgreving2304 commented 5 years ago

This might affect the encoding. The instruction might expect the L/L' bits set accordingly, in which case this is a more serious bug (not just a cosmetic one) and needs to get fixed. Update this bug does not affect the encoding. It only affects the mnemonic that is printed, and the instruction's source operand size.

derekbruening commented 5 years ago

Is this just the zeroing of the upper ymm bits that's done with all non-legacy instructions? I.e., should DR be treating the destination as xmm instead of ymm, just like it does for all the non-legacy-encoded instructions.

hgreving2304 commented 5 years ago

Is this just the zeroing of the upper ymm bits that's done with all non-legacy instructions? I.e., should DR be treating the destination as xmm instead of ymm, just like it does for all the non-legacy-encoded instructions.

Yes, pretty much. For example, for vcvtps2pd, AVX-512 allows for xmm -> xmm, xmm -> ymm and ymm -> zmm. In order to constrain from e.g. zmm -> ymm, we probably need separate entries for the 3 forms (e.g. Vqq, We would not be enough). This affects old VEX encodings as well, so my plan was to fix this after pull/3641 altogether.