[PATCH] D103427: [X86] Fix handling of maskmovdqu in X32

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:4032-4039
+           AdSize64;
+let Uses = [EDI], Predicates = [UseSSE2,In64BitMode] in
+def MASKMOVDQUX32 : PDI<0xF7, MRMSrcReg, (outs), (ins VR128:$src, VR128:$mask),
+           "addr32 maskmovdqu\t{$mask, $src|$src, $mask}",
+           [(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, EDI)]>,
+           AdSize32 {
+  let AsmVariantName = "NonParsable";
----------------
craig.topper wrote:
> skan wrote:
> > BTW, why do we need add an instruction like `MASKMOVDQUX32` when we already have  `MASKMOVDQU`? They have the same encoding/decoding except a address-size prefix. We can definitly encode  a `0x67` during encoding and and print a "addr32" during decoding according to the mode w/o adding any new intrustion.  If removing `Not64BitMode` of `MASKMOVDQU` may cause a ISEL issue, we should fix it in ISEL.
> The address register is implicit in the instruction. Our addr32 emission is normally based on the register class of the address registers. But we don't have that here. Are you suggesting to hardcode a special case for MASKMOVDQU in the encoder?
Harcode is one of the solution. Another solution is that we can add `X86::IP_HAS_AD_SIZE` to `Flags` of `MCInst` when translating a `MachineInstr` to a `MCInst`, so that a 0x67 will be emitted. `MASKMOVDQU` accesses memory and has implicit use `EDI`,  and we can get such information from a `MachineInstr`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103427/new/

https://reviews.llvm.org/D103427