Skip to content

Commit 3337f50

Browse files
hvdijkhvdijk
committed
[X86] Fix handling of maskmovdqu in x32 differently
This reverts the functional changes of D103427 but keeps its tests, and and reimplements the functionality by reusing the existing 32-bit MASKMOVDQU and VMASKMOVDQU instructions as suggested by skan in review. These instructions were previously predicated on Not64BitMode. This reimplementation restores the disassembly of a class of instructions, which will see a test added in followup patch D122449. These instructions are in 64-bit mode special cased in X86MCInstLower::Lower, because we use flags with one meaning for subtly different things: we have an AdSize32 class which indicates both that the instruction needs a 0x67 prefix and that the text form of the instruction implies a 0x67 prefix. These instructions are special in needing a 0x67 prefix but having a text form that does *not* imply a 0x67 prefix, so we encode this in MCInst as an instruction that has an explicit address size override. Note that originally VMASKMOVDQU64 was special cased to be excluded from disassembly, as we cannot distinguish between VMASKMOVDQU and VMASKMOVDQU64 and rely on the fact that these are indistinguishable, or close enough to it, at the MCInst level that it does not matter which we use. Because VMASKMOVDQU now receives special casing, even though it does not make a difference in the current implementation, as a precaution VMASKMOVDQU is excluded from disassembly rather than VMASKMOVDQU64. Reviewed By: RKSimon, skan Differential Revision: https://reviews.llvm.org/D122540
1 parent 20aedb1 commit 3337f50

File tree

6 files changed

+24
-51
lines changed

6 files changed

+24
-51
lines changed

‎llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,6 @@ enum attributeBits {
120120
ENUM_ENTRY(IC_VEX_XS, 2, "requires VEX and the XS prefix") \
121121
ENUM_ENTRY(IC_VEX_XD, 2, "requires VEX and the XD prefix") \
122122
ENUM_ENTRY(IC_VEX_OPSIZE, 2, "requires VEX and the OpSize prefix") \
123-
ENUM_ENTRY(IC_64BIT_VEX_OPSIZE, 4, "requires 64-bit mode and VEX") \
124-
ENUM_ENTRY(IC_64BIT_VEX_OPSIZE_ADSIZE, 5, "requires 64-bit mode, VEX, and AdSize")\
125123
ENUM_ENTRY(IC_VEX_W, 3, "requires VEX and the W prefix") \
126124
ENUM_ENTRY(IC_VEX_W_XS, 4, "requires VEX, W, and XS prefix") \
127125
ENUM_ENTRY(IC_VEX_W_XD, 4, "requires VEX, W, and XD prefix") \

‎llvm/lib/Target/X86/X86InstrSSE.td

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3997,7 +3997,10 @@ def PMOVMSKBrr : PDI<0xD7, MRMSrcReg, (outs GR32orGR64:$dst), (ins VR128:$src),
39973997
//===---------------------------------------------------------------------===//
39983998

39993999
let ExeDomain = SSEPackedInt, SchedRW = [SchedWriteVecMoveLS.XMM.MR] in {
4000-
let Uses = [EDI], Predicates = [HasAVX,Not64BitMode] in
4000+
// As VEX does not have separate instruction contexts for address size
4001+
// overrides, VMASKMOVDQU and VMASKMOVDQU64 would have a decode conflict.
4002+
// Prefer VMASKMODDQU64.
4003+
let Uses = [EDI], Predicates = [HasAVX], isAsmParserOnly = 1 in
40014004
def VMASKMOVDQU : VPDI<0xF7, MRMSrcReg, (outs),
40024005
(ins VR128:$src, VR128:$mask),
40034006
"maskmovdqu\t{$mask, $src|$src, $mask}",
@@ -4008,32 +4011,16 @@ def VMASKMOVDQU64 : VPDI<0xF7, MRMSrcReg, (outs),
40084011
(ins VR128:$src, VR128:$mask),
40094012
"maskmovdqu\t{$mask, $src|$src, $mask}",
40104013
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, RDI)]>,
4011-
VEX, VEX_WIG, AdSize64;
4012-
let Uses = [EDI], Predicates = [HasAVX,In64BitMode] in
4013-
def VMASKMOVDQUX32 : VPDI<0xF7, MRMSrcReg, (outs),
4014-
(ins VR128:$src, VR128:$mask), "",
4015-
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, EDI)]>,
4016-
VEX, VEX_WIG, AdSize32 {
4017-
let AsmString = "addr32 vmaskmovdqu\t{$mask, $src|$src, $mask}";
4018-
let AsmVariantName = "NonParsable";
4019-
}
4014+
VEX, VEX_WIG;
40204015

4021-
let Uses = [EDI], Predicates = [UseSSE2,Not64BitMode] in
4016+
let Uses = [EDI], Predicates = [UseSSE2] in
40224017
def MASKMOVDQU : PDI<0xF7, MRMSrcReg, (outs), (ins VR128:$src, VR128:$mask),
40234018
"maskmovdqu\t{$mask, $src|$src, $mask}",
40244019
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, EDI)]>;
40254020
let Uses = [RDI], Predicates = [UseSSE2,In64BitMode] in
40264021
def MASKMOVDQU64 : PDI<0xF7, MRMSrcReg, (outs), (ins VR128:$src, VR128:$mask),
40274022
"maskmovdqu\t{$mask, $src|$src, $mask}",
4028-
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, RDI)]>,
4029-
AdSize64;
4030-
let Uses = [EDI], Predicates = [UseSSE2,In64BitMode] in
4031-
def MASKMOVDQUX32 : PDI<0xF7, MRMSrcReg, (outs), (ins VR128:$src, VR128:$mask),
4032-
"addr32 maskmovdqu\t{$mask, $src|$src, $mask}",
4033-
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, EDI)]>,
4034-
AdSize32 {
4035-
let AsmVariantName = "NonParsable";
4036-
}
4023+
[(int_x86_sse2_maskmov_dqu VR128:$src, VR128:$mask, RDI)]>;
40374024

40384025
} // ExeDomain = SSEPackedInt
40394026

‎llvm/lib/Target/X86/X86MCInstLower.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,12 @@ void X86MCInstLower::Lower(const MachineInstr *MI, MCInst &OutMI) const {
962962
// These are not truly commutable so hide them from the default case.
963963
break;
964964

965+
case X86::MASKMOVDQU:
966+
case X86::VMASKMOVDQU:
967+
if (AsmPrinter.getSubtarget().is64Bit())
968+
OutMI.setFlags(X86::IP_HAS_AD_SIZE);
969+
break;
970+
965971
default: {
966972
// If the instruction is a commutable arithmetic instruction we might be
967973
// able to commute the operands to get a 2 byte VEX prefix.

‎llvm/lib/Target/X86/X86ScheduleBtVer2.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -840,8 +840,8 @@ def JWriteMASKMOVDQU: SchedWriteRes<[JFPU0, JFPA, JFPU1, JSTC, JLAGU, JSAGU, JAL
840840
let ResourceCycles = [1, 1, 2, 2, 2, 16, 42];
841841
let NumMicroOps = 63;
842842
}
843-
def : InstRW<[JWriteMASKMOVDQU], (instrs MASKMOVDQU, MASKMOVDQU64, MASKMOVDQUX32,
844-
VMASKMOVDQU, VMASKMOVDQU64, VMASKMOVDQUX32)>;
843+
def : InstRW<[JWriteMASKMOVDQU], (instrs MASKMOVDQU, MASKMOVDQU64,
844+
VMASKMOVDQU, VMASKMOVDQU64)>;
845845

846846
///////////////////////////////////////////////////////////////////////////////
847847
// SchedWriteVariant definitions.

‎llvm/utils/TableGen/X86DisassemblerTables.cpp

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,7 @@ static inline bool inheritsFrom(InstructionContext child,
105105
case IC_64BIT_ADSIZE:
106106
return (noPrefix && inheritsFrom(child, IC_64BIT_OPSIZE_ADSIZE, noPrefix));
107107
case IC_64BIT_OPSIZE_ADSIZE:
108-
return (noPrefix &&
109-
inheritsFrom(child, IC_64BIT_VEX_OPSIZE_ADSIZE, noPrefix));
108+
return false;
110109
case IC_XD:
111110
return inheritsFrom(child, IC_64BIT_XD);
112111
case IC_XS:
@@ -127,11 +126,10 @@ static inline bool inheritsFrom(InstructionContext child,
127126
case IC_64BIT_OPSIZE:
128127
return inheritsFrom(child, IC_64BIT_REXW_OPSIZE) ||
129128
(!AdSize64 && inheritsFrom(child, IC_64BIT_OPSIZE_ADSIZE)) ||
130-
(!AdSize64 && inheritsFrom(child, IC_64BIT_REXW_ADSIZE)) ||
131-
(!AdSize64 && inheritsFrom(child, IC_64BIT_VEX_OPSIZE_ADSIZE));
129+
(!AdSize64 && inheritsFrom(child, IC_64BIT_REXW_ADSIZE));
132130
case IC_64BIT_XD:
133-
return (inheritsFrom(child, IC_64BIT_REXW_XD) ||
134-
(!AdSize64 && inheritsFrom(child, IC_64BIT_XD_ADSIZE)));
131+
return(inheritsFrom(child, IC_64BIT_REXW_XD) ||
132+
(!AdSize64 && inheritsFrom(child, IC_64BIT_XD_ADSIZE)));
135133
case IC_64BIT_XS:
136134
return(inheritsFrom(child, IC_64BIT_REXW_XS) ||
137135
(!AdSize64 && inheritsFrom(child, IC_64BIT_XS_ADSIZE)));
@@ -161,12 +159,7 @@ static inline bool inheritsFrom(InstructionContext child,
161159
case IC_VEX_OPSIZE:
162160
return (VEX_LIG && VEX_WIG && inheritsFrom(child, IC_VEX_L_W_OPSIZE)) ||
163161
(VEX_WIG && inheritsFrom(child, IC_VEX_W_OPSIZE)) ||
164-
(VEX_LIG && inheritsFrom(child, IC_VEX_L_OPSIZE)) ||
165-
inheritsFrom(child, IC_64BIT_VEX_OPSIZE);
166-
case IC_64BIT_VEX_OPSIZE:
167-
return inheritsFrom(child, IC_64BIT_VEX_OPSIZE_ADSIZE);
168-
case IC_64BIT_VEX_OPSIZE_ADSIZE:
169-
return false;
162+
(VEX_LIG && inheritsFrom(child, IC_VEX_L_OPSIZE));
170163
case IC_VEX_W:
171164
return VEX_LIG && inheritsFrom(child, IC_VEX_L_W);
172165
case IC_VEX_W_XS:
@@ -888,9 +881,6 @@ void DisassemblerTables::emitContextTable(raw_ostream &o, unsigned &i) const {
888881
if ((index & ATTR_EVEX) || (index & ATTR_VEX) || (index & ATTR_VEXL)) {
889882
if (index & ATTR_EVEX)
890883
o << "IC_EVEX";
891-
else if ((index & (ATTR_64BIT | ATTR_VEXL | ATTR_REXW | ATTR_OPSIZE)) ==
892-
(ATTR_64BIT | ATTR_OPSIZE))
893-
o << "IC_64BIT_VEX";
894884
else
895885
o << "IC_VEX";
896886

@@ -902,13 +892,9 @@ void DisassemblerTables::emitContextTable(raw_ostream &o, unsigned &i) const {
902892
if (index & ATTR_REXW)
903893
o << "_W";
904894

905-
if (index & ATTR_OPSIZE) {
895+
if (index & ATTR_OPSIZE)
906896
o << "_OPSIZE";
907-
if ((index & (ATTR_64BIT | ATTR_EVEX | ATTR_VEX | ATTR_VEXL |
908-
ATTR_REXW | ATTR_ADSIZE)) ==
909-
(ATTR_64BIT | ATTR_VEX | ATTR_ADSIZE))
910-
o << "_ADSIZE";
911-
} else if (index & ATTR_XD)
897+
else if (index & ATTR_XD)
912898
o << "_XD";
913899
else if (index & ATTR_XS)
914900
o << "_XS";
@@ -922,7 +908,8 @@ void DisassemblerTables::emitContextTable(raw_ostream &o, unsigned &i) const {
922908
if (index & ATTR_EVEXB)
923909
o << "_B";
924910
}
925-
} else if ((index & ATTR_64BIT) && (index & ATTR_REXW) && (index & ATTR_XS))
911+
}
912+
else if ((index & ATTR_64BIT) && (index & ATTR_REXW) && (index & ATTR_XS))
926913
o << "IC_64BIT_REXW_XS";
927914
else if ((index & ATTR_64BIT) && (index & ATTR_REXW) && (index & ATTR_XD))
928915
o << "IC_64BIT_REXW_XD";

‎llvm/utils/TableGen/X86RecognizableInstr.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,6 @@ InstructionContext RecognizableInstr::insnContext() const {
310310
insnContext = IC_VEX_L_OPSIZE;
311311
else if (OpPrefix == X86Local::PD && HasVEX_W)
312312
insnContext = IC_VEX_W_OPSIZE;
313-
else if (OpPrefix == X86Local::PD && Is64Bit &&
314-
AdSize == X86Local::AdSize32)
315-
insnContext = IC_64BIT_VEX_OPSIZE_ADSIZE;
316-
else if (OpPrefix == X86Local::PD && Is64Bit)
317-
insnContext = IC_64BIT_VEX_OPSIZE;
318313
else if (OpPrefix == X86Local::PD)
319314
insnContext = IC_VEX_OPSIZE;
320315
else if (HasVEX_L && OpPrefix == X86Local::XS)

0 commit comments

Comments
 (0)