[llvm] r225036 - [X86] Fix disassembly of absolute moves to work correctly in 16 and 32-bit modes with all 4 combinations of OpSize and AdSize prefixes being present or not.

Craig Topper craig.topper at gmail.com
Tue Dec 30 23:07:31 PST 2014


Author: ctopper
Date: Wed Dec 31 01:07:31 2014
New Revision: 225036

URL: http://llvm.org/viewvc/llvm-project?rev=225036&view=rev
Log:
[X86] Fix disassembly of absolute moves to work correctly in 16 and 32-bit modes with all 4 combinations of OpSize and AdSize prefixes being present or not.

Modified:
    llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
    llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h
    llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp
    llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp

Modified: llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp?rev=225036&r1=225035&r2=225036&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp (original)
+++ llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp Wed Dec 31 01:07:31 2014
@@ -1019,6 +1019,32 @@ static int getID(struct InternalInstruct
     }
   }
 
+  /*
+   * Absolute moves need special handling.
+   * -For 16-bit mode because the meaning of the AdSize and OpSize prefixes are
+   *  inverted w.r.t.
+   * -For 32-bit mode we need to ensure the ADSIZE prefix is observed in
+   *  any position.
+   */
+  if (insn->opcodeType == ONEBYTE && ((insn->opcode & 0xFC) == 0xA0)) {
+    /* Make sure we observed the prefixes in any position. */
+    if (insn->prefixPresent[0x67])
+      attrMask |= ATTR_ADSIZE;
+    if (insn->prefixPresent[0x66])
+      attrMask |= ATTR_OPSIZE;
+
+    /* In 16-bit, invert the attributes. */
+    if (insn->mode == MODE_16BIT)
+      attrMask ^= ATTR_ADSIZE | ATTR_OPSIZE;
+
+    if (getIDWithAttrMask(&instructionID, insn, attrMask))
+      return -1;
+
+    insn->instructionID = instructionID;
+    insn->spec = specifierForUID(instructionID);
+    return 0;
+  }
+
   if ((insn->mode == MODE_16BIT || insn->prefixPresent[0x66]) &&
       !(attrMask & ATTR_OPSIZE)) {
     /*

Modified: llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h?rev=225036&r1=225035&r2=225036&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h (original)
+++ llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h Wed Dec 31 01:07:31 2014
@@ -82,6 +82,7 @@ enum attributeBits {
                                         "operands change width")               \
   ENUM_ENTRY(IC_ADSIZE,             3,  "requires an ADSIZE prefix, so "       \
                                         "operands change width")               \
+  ENUM_ENTRY(IC_OPSIZE_ADSIZE,      4,  "requires ADSIZE and OPSIZE prefixes") \
   ENUM_ENTRY(IC_XD,                 2,  "may say something about the opcode "  \
                                         "but not the operands")                \
   ENUM_ENTRY(IC_XS,                 2,  "may say something about the opcode "  \
@@ -90,20 +91,22 @@ enum attributeBits {
                                         "operands change width")               \
   ENUM_ENTRY(IC_XS_OPSIZE,          3,  "requires an OPSIZE prefix, so "       \
                                         "operands change width")               \
-  ENUM_ENTRY(IC_64BIT_REXW,         4,  "requires a REX.W prefix, so operands "\
+  ENUM_ENTRY(IC_64BIT_REXW,         5,  "requires a REX.W prefix, so operands "\
                                         "change width; overrides IC_OPSIZE")   \
   ENUM_ENTRY(IC_64BIT_OPSIZE,       3,  "Just as meaningful as IC_OPSIZE")     \
   ENUM_ENTRY(IC_64BIT_ADSIZE,       3,  "Just as meaningful as IC_ADSIZE")     \
-  ENUM_ENTRY(IC_64BIT_XD,           5,  "XD instructions are SSE; REX.W is "   \
+  ENUM_ENTRY(IC_64BIT_OPSIZE_ADSIZE, 4, "Just as meaningful as IC_OPSIZE/"     \
+                                        "IC_ADSIZE")                           \
+  ENUM_ENTRY(IC_64BIT_XD,           6,  "XD instructions are SSE; REX.W is "   \
                                         "secondary")                           \
-  ENUM_ENTRY(IC_64BIT_XS,           5,  "Just as meaningful as IC_64BIT_XD")   \
+  ENUM_ENTRY(IC_64BIT_XS,           6,  "Just as meaningful as IC_64BIT_XD")   \
   ENUM_ENTRY(IC_64BIT_XD_OPSIZE,    3,  "Just as meaningful as IC_XD_OPSIZE")  \
   ENUM_ENTRY(IC_64BIT_XS_OPSIZE,    3,  "Just as meaningful as IC_XS_OPSIZE")  \
-  ENUM_ENTRY(IC_64BIT_REXW_XS,      6,  "OPSIZE could mean a different "       \
+  ENUM_ENTRY(IC_64BIT_REXW_XS,      7,  "OPSIZE could mean a different "       \
                                         "opcode")                              \
-  ENUM_ENTRY(IC_64BIT_REXW_XD,      6,  "Just as meaningful as "               \
+  ENUM_ENTRY(IC_64BIT_REXW_XD,      7,  "Just as meaningful as "               \
                                         "IC_64BIT_REXW_XS")                    \
-  ENUM_ENTRY(IC_64BIT_REXW_OPSIZE,  7,  "The Dynamic Duo!  Prefer over all "   \
+  ENUM_ENTRY(IC_64BIT_REXW_OPSIZE,  8,  "The Dynamic Duo!  Prefer over all "   \
                                         "else because this changes most "      \
                                         "operands' meaning")                   \
   ENUM_ENTRY(IC_VEX,                1,  "requires a VEX prefix")               \

Modified: llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp?rev=225036&r1=225035&r2=225036&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp (original)
+++ llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp Wed Dec 31 01:07:31 2014
@@ -93,9 +93,15 @@ static inline bool inheritsFrom(Instruct
            inheritsFrom(child, IC_64BIT_XD)     ||
            inheritsFrom(child, IC_64BIT_XS));
   case IC_OPSIZE:
-    return inheritsFrom(child, IC_64BIT_OPSIZE);
+    return inheritsFrom(child, IC_64BIT_OPSIZE) ||
+           inheritsFrom(child, IC_OPSIZE_ADSIZE);
   case IC_ADSIZE:
+    return inheritsFrom(child, IC_OPSIZE_ADSIZE);
+  case IC_OPSIZE_ADSIZE:
+    return false;
   case IC_64BIT_ADSIZE:
+    return inheritsFrom(child, IC_64BIT_OPSIZE_ADSIZE);
+  case IC_64BIT_OPSIZE_ADSIZE:
     return false;
   case IC_XD:
     return inheritsFrom(child, IC_64BIT_XD);
@@ -110,7 +116,8 @@ static inline bool inheritsFrom(Instruct
            inheritsFrom(child, IC_64BIT_REXW_XD) ||
            inheritsFrom(child, IC_64BIT_REXW_OPSIZE));
   case IC_64BIT_OPSIZE:
-    return(inheritsFrom(child, IC_64BIT_REXW_OPSIZE));
+    return inheritsFrom(child, IC_64BIT_REXW_OPSIZE) ||
+           inheritsFrom(child, IC_64BIT_OPSIZE_ADSIZE);
   case IC_64BIT_XD:
     return(inheritsFrom(child, IC_64BIT_REXW_XD));
   case IC_64BIT_XS:
@@ -721,6 +728,9 @@ void DisassemblerTables::emitContextTabl
       o << "IC_64BIT_XS";
     else if ((index & ATTR_64BIT) && (index & ATTR_XD))
       o << "IC_64BIT_XD";
+    else if ((index & ATTR_64BIT) && (index & ATTR_OPSIZE) &&
+             (index & ATTR_ADSIZE))
+      o << "IC_64BIT_OPSIZE_ADSIZE";
     else if ((index & ATTR_64BIT) && (index & ATTR_OPSIZE))
       o << "IC_64BIT_OPSIZE";
     else if ((index & ATTR_64BIT) && (index & ATTR_ADSIZE))
@@ -737,6 +747,8 @@ void DisassemblerTables::emitContextTabl
       o << "IC_XS";
     else if (index & ATTR_XD)
       o << "IC_XD";
+    else if ((index & ATTR_OPSIZE) && (index & ATTR_ADSIZE))
+      o << "IC_OPSIZE_ADSIZE";
     else if (index & ATTR_OPSIZE)
       o << "IC_OPSIZE";
     else if (index & ATTR_ADSIZE)
@@ -822,18 +834,6 @@ void DisassemblerTables::setTableFields(
         InstructionSpecifier &previousInfo =
           InstructionSpecifiers[decision.instructionIDs[index]];
 
-        // FIXME this doesn't actually work. The MCInsts the disassembler
-        // create don't encode re-encode correctly. They just manage to mostly
-        // print correctly.
-        // Instructions such as MOV8ao8 and MOV8ao8_16 differ only in the
-        // presence of the AdSize prefix. However, the disassembler doesn't
-        // care about that difference in the instruction definition; it
-        // handles 16-bit vs. 32-bit addressing for itself based purely
-        // on the 0x67 prefix and the CPU mode. So there's no need to
-        // disambiguate between them; just let them conflict/coexist.
-        if (previousInfo.name + "_16" == newInfo.name)
-          continue;
-
         if(previousInfo.name == "NOOP" && (newInfo.name == "XCHG16ar" ||
                                            newInfo.name == "XCHG32ar" ||
                                            newInfo.name == "XCHG32ar64" ||

Modified: llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp?rev=225036&r1=225035&r2=225036&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp (original)
+++ llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp Wed Dec 31 01:07:31 2014
@@ -412,6 +412,8 @@ InstructionContext RecognizableInstr::in
       insnContext = IC_64BIT_XD_OPSIZE;
     else if (OpSize == X86Local::OpSize16 && OpPrefix == X86Local::XS)
       insnContext = IC_64BIT_XS_OPSIZE;
+    else if (OpSize == X86Local::OpSize16 && AdSize == X86Local::AdSize32)
+      insnContext = IC_64BIT_OPSIZE_ADSIZE;
     else if (OpSize == X86Local::OpSize16 || OpPrefix == X86Local::PD)
       insnContext = IC_64BIT_OPSIZE;
     else if (AdSize == X86Local::AdSize32)
@@ -433,6 +435,8 @@ InstructionContext RecognizableInstr::in
       insnContext = IC_XD_OPSIZE;
     else if (OpSize == X86Local::OpSize16 && OpPrefix == X86Local::XS)
       insnContext = IC_XS_OPSIZE;
+    else if (OpSize == X86Local::OpSize16 && AdSize == X86Local::AdSize16)
+      insnContext = IC_OPSIZE_ADSIZE;
     else if (OpSize == X86Local::OpSize16 || OpPrefix == X86Local::PD)
       insnContext = IC_OPSIZE;
     else if (AdSize == X86Local::AdSize16)





More information about the llvm-commits mailing list