Adding support for missing variations of X86 string related instructions

[myatsina]

Summary

The following are legal according to X86 spec:

ins mem, DX
outs DX, mem
lods mem
stos mem
scas mem
cmps mem, mem
movs mem, mem

[rnk]

Can you add tests to cover the new error paths you are adding?

test/MC/X86/intel-syntax.s

740–741 ↗

(On Diff #40657)

I'm confused. Why are these two assembly instructions equivalent? Ditto for the rest, they don't look equivalent.

[myatsina]

In D14827#295382, @rnk wrote:

Can you add tests to cover the new error paths you are adding?

Do you mean the "mismatching source and destination index registers" error?
If so, then this code is just a refactoring of the old logic that used "doSrcDstMatch" and returned this error if there was no match.
It should be covered by existent tests, but I can add one more just in case.

test/MC/X86/intel-syntax.s

740–741 ↗

(On Diff #40657)

If you look at the X86 spec of these instructions:

INS m8, DX - Input byte from I/O port specified in DX into memory location specified in ES:(E)DI or RDI
which is actually the same as the definition of the "insb" instruction.
The mem operand has no meaning other than distinguishing if it's insb, insw or insl (in this case it's mem8, so it should be insb).

"insb" and "ins m8, DX" are equivalent and should map to the same instruction.

Does this answer your question?

[rnk]

In D14827#295822, @myatsina wrote:

In D14827#295382, @rnk wrote:

Can you add tests to cover the new error paths you are adding?

Do you mean the "mismatching source and destination index registers" error?
If so, then this code is just a refactoring of the old logic that used "doSrcDstMatch" and returned this error if there was no match.
It should be covered by existent tests, but I can add one more just in case.

Ah, you're right. Nevermind.

[rnk]

lib/Target/X86/AsmParser/X86AsmParser.cpp

1057–1058 ↗

(On Diff #40657)

Please use a static_cast here

test/MC/X86/intel-syntax.s

740 ↗

(On Diff #40657)

You should test what happens with different pointer sizes. Since this is a 64-bit test, can you add a test like this:

ins byte ptr [rax], dx

GAS assembles that to this:

insb   (%dx),%es:(%rdi)

And then again in a separate 32-bit test file, you should do something like:

insb byte ptr [ax], dx
insb byte ptr [eax], dx
// CHECK: insb   (%dx),%es:(%di)
// CHECK: insb   (%dx),%es:(%edi)

740–741 ↗

(On Diff #40657)

OK, so the register eax is essentially discarded and is only used to size the implicit memory operand. binutils 'as' issues a warning in this case, but MSVC does not. What does Intel's assembler do? I would like it if we issued a warning that EDI or ESI was expected in place of the user's register.

[myatsina]

Added tests as asked

[myatsina]

test/MC/X86/intel-syntax.s

740 ↗

(On Diff #41409)

Added test cases in index-operations.s
I've added only for insw, I believe these cases with the case here have enough coverage

740–741 ↗

(On Diff #40657)

Intel assembler does not issue a warning, same as MSVC.
I think we shouldn't issue a warning either because according to x86 spec this is a completely legal instruction which has the semantics of looking at ESI/EDI.
The spec does not have any restrictions about the mem operand and does not "expect" ESI/EDI.

[rnk]

Craig, can you give us a second opinion here?

These string instructions need a memory operand size, but the encodings only support [esi]/[edi] as a memory operand. If the users asks for ins byte ptr [eax], dx and we assemble it to ins byte ptr es:[edi], dx, it seems highly likely that the user made a mistake.

I'm trying to insist that we should either error or warn when the instruction that the user asked for is not encodable.

[silvas]

In D14827#298742, @rnk wrote:

Craig, can you give us a second opinion here?

These string instructions need a memory operand size, but the encodings only support [esi]/[edi] as a memory operand. If the users asks for ins byte ptr [eax], dx and we assemble it to ins byte ptr es:[edi], dx, it seems highly likely that the user made a mistake.

I'm trying to insist that we should either error or warn when the instruction that the user asked for is not encodable.

The intel manual even has a paragraph about this being misleading:

"""
At the assembly-code level, two forms of this instruction are allowed: the “explicit-operands” form and the “no- operands” form. The explicit-operands form (specified with the INS mnemonic) allows the source and destination operands to be specified explicitly. Here, the source operand must be “DX,” and the destination operand should be a symbol that indicates the size of the I/O port and the destination address. This explicit-operands form is provided to allow documentation; *however, note that the documentation provided by this form can be misleading*. That is, the destination operand symbol must specify the correct type (size) of the operand (byte, word, or doubleword), but it does not have to specify the correct location. The location is always specified by the ES:(E)DI registers, which must be loaded correctly before the INS instruction is executed.

"""

(emphasis mine)

[myatsina]

Sean, if I understand correctly, you, as well as Reid, support producing a warning, though the spec explicitly mentions that the memory operand is ignored, and though MSVC and ICC do not produce such warnings, right?
If so, how about some kind of warning like "Memory operand ignored, assuming ES:(R|E)SI" or something of that sort?

[myatsina]

Added warning regarding the fact that these instructions use (R|E)SI or (R|E)DI instead of the specified memory operand

[myatsina]

ping

[myatsina]

I've added the warning as you suggested, can you please review the new version?

[rnk]

lgtm

[sberg]

Building LibreOffice (which has some asm blocks) started to fail now with

error: memory operand is only for determining the size, ES:(R|E)SI will be used for the location [-Werror,-Winline-asm]
<inline asm>:2:10: note: instantiated into assembly here

movsd   (%rax), %xmm0

[myatsina]

Committed a fix in revision 258312.
Please let me know if you encounter additional issues.

[sberg]

r258312 does not make that -Werror,-Winline-asm go away for me

[myatsina]

yes you are right, I should not produce the warning in your case, working on a fix.

llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp

private:
 
   std::nullptr_t ErrorOperand(SMLoc Loc, StringRef Msg) {
     Error(Loc, Msg);
     return nullptr;
   }
 
   std::unique_ptr<X86Operand> DefaultMemSIOperand(SMLoc Loc);
   std::unique_ptr<X86Operand> DefaultMemDIOperand(SMLoc Loc);
-  void AddDefaultSrcDestOperands(
-      OperandVector& Operands, std::unique_ptr<llvm::MCParsedAsmOperand> &&Src,
+  bool IsSIReg(unsigned Reg);
+  unsigned GetSIDIForRegClass(unsigned RegClassID, unsigned Reg, bool IsSIReg);
+  void
+  AddDefaultSrcDestOperands(OperandVector &Operands,
+                            std::unique_ptr<llvm::MCParsedAsmOperand> &&Src,
                             std::unique_ptr<llvm::MCParsedAsmOperand> &&Dst);
+  bool VerifyAndAdjustOperands(OperandVector &OrigOperands,
+                               OperandVector &FinalOperands);
   std::unique_ptr<X86Operand> ParseOperand();
   std::unique_ptr<X86Operand> ParseATTOperand();
   std::unique_ptr<X86Operand> ParseIntelOperand();
   std::unique_ptr<X86Operand> ParseIntelOffsetOfOperator();
   bool ParseIntelDotOperator(const MCExpr *Disp, const MCExpr *&NewDisp);
   std::unique_ptr<X86Operand> ParseIntelOperator(unsigned OpKind);
   std::unique_ptr<X86Operand>
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
private:
 
   bool MatchAndEmitIntelInstruction(SMLoc IDLoc, unsigned &Opcode,
                                     OperandVector &Operands, MCStreamer &Out,
                                     uint64_t &ErrorInfo,
                                     bool MatchingInlineAsm);
 
   bool OmitRegisterFromClobberLists(unsigned RegNo) override;
 
-  /// doSrcDstMatch - Returns true if operands are matching in their
-  /// word size (%si and %di, %esi and %edi, etc.). Order depends on
-  /// the parsing mode (Intel vs. AT&T).
-  bool doSrcDstMatch(X86Operand &Op1, X86Operand &Op2);
-
   /// Parses AVX512 specific operand primitives: masked registers ({%k<NUM>}, {z})
   /// and memory broadcasting ({1to<NUM>}) primitives, updating Operands vector if required.
   /// \return \c true if no parsing errors occurred, \c false otherwise.
   bool HandleAVX512Operand(OperandVector &Operands,
                            const MCParsedAsmOperand &Op);
 
   bool is64BitMode() const {
     // FIXME: Can tablegen auto-generate this?
if (X86MCRegisterClasses[X86::GR16RegClassID].contains(BaseReg)) {
         ErrMsg = "invalid 16-bit base/index register combination";
         return true;
       }
     }
   }
   return false;
 }
 
-bool X86AsmParser::doSrcDstMatch(X86Operand &Op1, X86Operand &Op2)
-{
-  // Return true and let a normal complaint about bogus operands happen.
-  if (!Op1.isMem() || !Op2.isMem())
-    return true;
-
-  // Actually these might be the other way round if Intel syntax is
-  // being used. It doesn't matter.
-  unsigned diReg = Op1.Mem.BaseReg;
-  unsigned siReg = Op2.Mem.BaseReg;
-
-  if (X86MCRegisterClasses[X86::GR16RegClassID].contains(siReg))
-    return X86MCRegisterClasses[X86::GR16RegClassID].contains(diReg);
-  if (X86MCRegisterClasses[X86::GR32RegClassID].contains(siReg))
-    return X86MCRegisterClasses[X86::GR32RegClassID].contains(diReg);
-  if (X86MCRegisterClasses[X86::GR64RegClassID].contains(siReg))
-    return X86MCRegisterClasses[X86::GR64RegClassID].contains(diReg);
-  // Again, return true and let another error happen.
-  return true;
-}
-
 bool X86AsmParser::ParseRegister(unsigned &RegNo,
                                  SMLoc &StartLoc, SMLoc &EndLoc) {
   MCAsmParser &Parser = getParser();
   RegNo = 0;
   const AsmToken &PercentTok = Parser.getTok();
   StartLoc = PercentTok.getLoc();
 
   // If we encounter a %, ignore it. This code handles registers with and
std::unique_ptr<X86Operand> X86AsmParser::DefaultMemDIOperand(SMLoc Loc) {
   unsigned basereg =
     is64BitMode() ? X86::RDI : (is32BitMode() ? X86::EDI : X86::DI);
   const MCExpr *Disp = MCConstantExpr::create(0, getContext());
   return X86Operand::CreateMem(getPointerWidth(), /*SegReg=*/0, Disp,
                                /*BaseReg=*/basereg, /*IndexReg=*/0, /*Scale=*/1,
                                Loc, Loc, 0);
 }
 
+bool X86AsmParser::IsSIReg(unsigned Reg) {
+  switch (Reg) {
+  default:
+    assert("Only (R|E)SI and (R|E)DI are expected!");
+    return false;
+  case X86::RSI:
+  case X86::ESI:
+  case X86::SI:
+    return true;
+  case X86::RDI:
+  case X86::EDI:
+  case X86::DI:
+    return false;
+  }
+}
+
+unsigned X86AsmParser::GetSIDIForRegClass(unsigned RegClassID, unsigned Reg,
+                                          bool IsSIReg) {
+  switch (RegClassID) {
+  default:
+    assert("Unexpected register class");
+    return Reg;
+  case X86::GR64RegClassID:
+    return IsSIReg ? X86::RSI : X86::RDI;
+  case X86::GR32RegClassID:
+    return IsSIReg ? X86::ESI : X86::EDI;
+  case X86::GR16RegClassID:
+    return IsSIReg ? X86::SI : X86::DI;
+  }
+}
+
 void X86AsmParser::AddDefaultSrcDestOperands(
     OperandVector& Operands, std::unique_ptr<llvm::MCParsedAsmOperand> &&Src,
     std::unique_ptr<llvm::MCParsedAsmOperand> &&Dst) {
   if (isParsingIntelSyntax()) {
     Operands.push_back(std::move(Dst));
     Operands.push_back(std::move(Src));
   }
   else {
     Operands.push_back(std::move(Src));
     Operands.push_back(std::move(Dst));
   }
 }
 
+bool X86AsmParser::VerifyAndAdjustOperands(OperandVector &OrigOperands,
+                                           OperandVector &FinalOperands) {
+
+  if (OrigOperands.size() > 1) {
+    // Check if sizes match, OrigOpernads also contains the instruction name
+    assert(OrigOperands.size() == FinalOperands.size() + 1 &&
+           "Opernand size mismatch");
+
+    // Verify types match
+    int RegClassID = -1;
+    for (unsigned int i = 0; i < FinalOperands.size(); ++i) {
+      X86Operand &OrigOp = static_cast<X86Operand &>(*OrigOperands[i + 1]);
+      X86Operand &FinalOp = static_cast<X86Operand &>(*FinalOperands[i]);
+
+      if (FinalOp.isReg() &&
+          (!OrigOp.isReg() || FinalOp.getReg() != OrigOp.getReg()))
+        // Return false and let a normal complaint about bogus operands happen
+        return false;
+
+      if (FinalOp.isMem()) {
+
+        if (!OrigOp.isMem())
+          // Return false and let a normal complaint about bogus operands happen
+          return false;
+
+        unsigned OrigReg = OrigOp.Mem.BaseReg;
+        unsigned FinalReg = FinalOp.Mem.BaseReg;
+
+        // If we've already encounterd a register class, make sure all register
+        // bases are of the same register class
+        if (RegClassID != -1 &&
+            !X86MCRegisterClasses[RegClassID].contains(OrigReg)) {
+          return Error(OrigOp.getStartLoc(),
+                       "mismatching source and destination index registers");
+        }
+
+        if (X86MCRegisterClasses[X86::GR64RegClassID].contains(OrigReg))
+          RegClassID = X86::GR64RegClassID;
+        else if (X86MCRegisterClasses[X86::GR32RegClassID].contains(OrigReg))
+          RegClassID = X86::GR32RegClassID;
+        else if (X86MCRegisterClasses[X86::GR16RegClassID].contains(OrigReg))
+          RegClassID = X86::GR16RegClassID;
+
+        bool IsSI = IsSIReg(FinalReg);
+        FinalReg = GetSIDIForRegClass(RegClassID, FinalReg, IsSI);
+
+        if (FinalReg != OrigReg) {
+          std::string RegName = IsSI ? "ES:(R|E)SI" : "ES:(R|E)DI";
+          Warning(OrigOp.getStartLoc(),
+                  "memory operand is only for determining the size, " +
+                      RegName + " will be used for the location");
+        }
+
+        FinalOp.Mem.Size = OrigOp.Mem.Size;
+        FinalOp.Mem.SegReg = OrigOp.Mem.SegReg;
+        FinalOp.Mem.BaseReg = FinalReg;
+      }
+    }
+
+    // Remove old operandss
+    for (unsigned int i = 0; i < FinalOperands.size(); ++i)
+      OrigOperands.pop_back();
+  }
+  // OrigOperands.append(FinalOperands.begin(), FinalOperands.end());
+  for (unsigned int i = 0; i < FinalOperands.size(); ++i)
+    OrigOperands.push_back(std::move(FinalOperands[i]));
+
+  return false;
+}
+
 std::unique_ptr<X86Operand> X86AsmParser::ParseOperand() {
   if (isParsingIntelSyntax())
     return ParseIntelOperand();
   return ParseATTOperand();
 }
 
 /// getIntelMemOperandSize - Return intel memory operand size.
 static unsigned getIntelMemOperandSize(StringRef OpStr) {
if (Op.isMem() && Op.Mem.SegReg == 0 &&
         isa<MCConstantExpr>(Op.Mem.Disp) &&
         cast<MCConstantExpr>(Op.Mem.Disp)->getValue() == 0 &&
         Op.Mem.BaseReg == MatchRegisterName("dx") && Op.Mem.IndexReg == 0) {
       SMLoc Loc = Op.getEndLoc();
       Operands[1] = X86Operand::CreateReg(Op.Mem.BaseReg, Loc, Loc);
     }
   }
 
+  SmallVector<std::unique_ptr<MCParsedAsmOperand>, 2> TmpOperands;
+  bool HadVerifyError = false;
+
   // Append default arguments to "ins[bwld]"
-  if (Name.startswith("ins") && Operands.size() == 1 &&
-      (Name == "insb" || Name == "insw" || Name == "insl" || Name == "insd")) {
-    AddDefaultSrcDestOperands(Operands,
+  if (Name.startswith("ins") && 
+      (Operands.size() == 1 || Operands.size() == 3) &&
+      (Name == "insb" || Name == "insw" || Name == "insl" || Name == "insd" ||
+       Name == "ins")) {
+    
+    AddDefaultSrcDestOperands(TmpOperands,
                               X86Operand::CreateReg(X86::DX, NameLoc, NameLoc),
                               DefaultMemDIOperand(NameLoc));
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
   }
 
   // Append default arguments to "outs[bwld]"
-  if (Name.startswith("outs") && Operands.size() == 1 &&
+  if (Name.startswith("outs") && 
+      (Operands.size() == 1 || Operands.size() == 3) &&
       (Name == "outsb" || Name == "outsw" || Name == "outsl" ||
-       Name == "outsd" )) {
-    AddDefaultSrcDestOperands(Operands,
-                              DefaultMemSIOperand(NameLoc),
+       Name == "outsd" || Name == "outs")) {
+    AddDefaultSrcDestOperands(TmpOperands, DefaultMemSIOperand(NameLoc),
                               X86Operand::CreateReg(X86::DX, NameLoc, NameLoc));
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
   }
 
   // Transform "lods[bwlq]" into "lods[bwlq] ($SIREG)" for appropriate
   // values of $SIREG according to the mode. It would be nice if this
   // could be achieved with InstAlias in the tables.
-  if (Name.startswith("lods") && Operands.size() == 1 &&
+  if (Name.startswith("lods") &&
+      (Operands.size() == 1 || Operands.size() == 2) &&
       (Name == "lods" || Name == "lodsb" || Name == "lodsw" ||
-       Name == "lodsl" || Name == "lodsd" || Name == "lodsq"))
-    Operands.push_back(DefaultMemSIOperand(NameLoc));
+       Name == "lodsl" || Name == "lodsd" || Name == "lodsq")) {
+    TmpOperands.push_back(DefaultMemSIOperand(NameLoc));
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
+  }
 
   // Transform "stos[bwlq]" into "stos[bwlq] ($DIREG)" for appropriate
   // values of $DIREG according to the mode. It would be nice if this
   // could be achieved with InstAlias in the tables.
-  if (Name.startswith("stos") && Operands.size() == 1 &&
+  if (Name.startswith("stos") &&
+      (Operands.size() == 1 || Operands.size() == 2) &&
       (Name == "stos" || Name == "stosb" || Name == "stosw" ||
-       Name == "stosl" || Name == "stosd" || Name == "stosq"))
-    Operands.push_back(DefaultMemDIOperand(NameLoc));
+       Name == "stosl" || Name == "stosd" || Name == "stosq")) {
+    TmpOperands.push_back(DefaultMemDIOperand(NameLoc));
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
+  }
 
   // Transform "scas[bwlq]" into "scas[bwlq] ($DIREG)" for appropriate
   // values of $DIREG according to the mode. It would be nice if this
   // could be achieved with InstAlias in the tables.
-  if (Name.startswith("scas") && Operands.size() == 1 &&
+  if (Name.startswith("scas") &&
+      (Operands.size() == 1 || Operands.size() == 2) &&
       (Name == "scas" || Name == "scasb" || Name == "scasw" ||
-       Name == "scasl" || Name == "scasd" || Name == "scasq"))
-    Operands.push_back(DefaultMemDIOperand(NameLoc));
+       Name == "scasl" || Name == "scasd" || Name == "scasq")) {
+    TmpOperands.push_back(DefaultMemDIOperand(NameLoc));
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
+  }
 
   // Add default SI and DI operands to "cmps[bwlq]".
   if (Name.startswith("cmps") &&
+      (Operands.size() == 1 || Operands.size() == 3) &&
       (Name == "cmps" || Name == "cmpsb" || Name == "cmpsw" ||
        Name == "cmpsl" || Name == "cmpsd" || Name == "cmpsq")) {
-    if (Operands.size() == 1) {
-      AddDefaultSrcDestOperands(Operands,
-                                DefaultMemDIOperand(NameLoc),
-                                DefaultMemSIOperand(NameLoc));
-    } else if (Operands.size() == 3) {
-      X86Operand &Op = (X86Operand &)*Operands[1];
-      X86Operand &Op2 = (X86Operand &)*Operands[2];
-      if (!doSrcDstMatch(Op, Op2))
-        return Error(Op.getStartLoc(),
-                     "mismatching source and destination index registers");
-    }
+    AddDefaultSrcDestOperands(TmpOperands, DefaultMemDIOperand(NameLoc),
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
   }
 
   // Add default SI and DI operands to "movs[bwlq]".
-  if ((Name.startswith("movs") &&
+  if (((Name.startswith("movs") &&
         (Name == "movs" || Name == "movsb" || Name == "movsw" ||
          Name == "movsl" || Name == "movsd" || Name == "movsq")) ||
        (Name.startswith("smov") &&
         (Name == "smov" || Name == "smovb" || Name == "smovw" ||
-       Name == "smovl" || Name == "smovd" || Name == "smovq"))) {
-    if (Operands.size() == 1) {
-      if (Name == "movsd")
-        Operands.back() = X86Operand::CreateToken("movsl", NameLoc);
-      AddDefaultSrcDestOperands(Operands,
-                                DefaultMemSIOperand(NameLoc),
-                                DefaultMemDIOperand(NameLoc));
-    } else if (Operands.size() == 3) {
-      X86Operand &Op = (X86Operand &)*Operands[1];
-      X86Operand &Op2 = (X86Operand &)*Operands[2];
-      if (!doSrcDstMatch(Op, Op2))
-        return Error(Op.getStartLoc(),
-                     "mismatching source and destination index registers");
-    }
+         Name == "smovl" || Name == "smovd" || Name == "smovq"))) &&
+      (Operands.size() == 1 || Operands.size() == 3)) {
+    if (Name == "movsd" && Operands.size() == 1)
+    AddDefaultSrcDestOperands(TmpOperands, DefaultMemSIOperand(NameLoc),
+    HadVerifyError = VerifyAndAdjustOperands(Operands, TmpOperands);
+
+  // Check if we encountered an error for one the string insturctions
+  if (HadVerifyError) {
+    return HadVerifyError;
   }
 
   // FIXME: Hack to handle recognize s{hr,ar,hl} $1, <op>.  Canonicalize to
   // "shift <op>".
   if ((Name.startswith("shr") || Name.startswith("sar") ||
        Name.startswith("shl") || Name.startswith("sal") ||
        Name.startswith("rcl") || Name.startswith("rcr") ||
        Name.startswith("rol") || Name.startswith("ror")) &&

llvm/trunk/lib/Target/X86/X86InstrInfo.td

 def : InstAlias<"lodsb\t$src", (LODSB srcidx8:$src),  0>;
 def : InstAlias<"lodsw\t$src", (LODSW srcidx16:$src), 0>;
 def : InstAlias<"lods{l|d}\t$src", (LODSL srcidx32:$src), 0>;
 def : InstAlias<"lodsq\t$src", (LODSQ srcidx64:$src), 0>, Requires<[In64BitMode]>;
 def : InstAlias<"lods\t{$src, %al|al, $src}", (LODSB srcidx8:$src),  0>;
 def : InstAlias<"lods\t{$src, %ax|ax, $src}", (LODSW srcidx16:$src), 0>;
 def : InstAlias<"lods\t{$src, %eax|eax, $src}", (LODSL srcidx32:$src), 0>;
 def : InstAlias<"lods\t{$src, %rax|rax, $src}", (LODSQ srcidx64:$src), 0>, Requires<[In64BitMode]>;
+def : InstAlias<"lods\t$src", (LODSB srcidx8:$src),  0>;
+def : InstAlias<"lods\t$src", (LODSW srcidx16:$src), 0>;
+def : InstAlias<"lods\t$src", (LODSL srcidx32:$src), 0>;
+def : InstAlias<"lods\t$src", (LODSQ srcidx64:$src), 0>, Requires<[In64BitMode]>;
+
 
 // stos aliases. Accept the source being omitted because it's implicit in
 // the mnemonic, or the mnemonic suffix being omitted because it's implicit
 // in the source.
 def : InstAlias<"stosb\t$dst", (STOSB dstidx8:$dst),  0>;
 def : InstAlias<"stosw\t$dst", (STOSW dstidx16:$dst), 0>;
 def : InstAlias<"stos{l|d}\t$dst", (STOSL dstidx32:$dst), 0>;
 def : InstAlias<"stosq\t$dst", (STOSQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
 def : InstAlias<"stos\t{%al, $dst|$dst, al}", (STOSB dstidx8:$dst),  0>;
 def : InstAlias<"stos\t{%ax, $dst|$dst, ax}", (STOSW dstidx16:$dst), 0>;
 def : InstAlias<"stos\t{%eax, $dst|$dst, eax}", (STOSL dstidx32:$dst), 0>;
 def : InstAlias<"stos\t{%rax, $dst|$dst, rax}", (STOSQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
+def : InstAlias<"stos\t$dst", (STOSB dstidx8:$dst),  0>;
+def : InstAlias<"stos\t$dst", (STOSW dstidx16:$dst), 0>;
+def : InstAlias<"stos\t$dst", (STOSL dstidx32:$dst), 0>;
+def : InstAlias<"stos\t$dst", (STOSQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
+
 
 // scas aliases. Accept the destination being omitted because it's implicit
 // in the mnemonic, or the mnemonic suffix being omitted because it's implicit
 // in the destination.
 def : InstAlias<"scasb\t$dst", (SCASB dstidx8:$dst),  0>;
 def : InstAlias<"scasw\t$dst", (SCASW dstidx16:$dst), 0>;
 def : InstAlias<"scas{l|d}\t$dst", (SCASL dstidx32:$dst), 0>;
 def : InstAlias<"scasq\t$dst", (SCASQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
 def : InstAlias<"scas\t{$dst, %al|al, $dst}", (SCASB dstidx8:$dst),  0>;
 def : InstAlias<"scas\t{$dst, %ax|ax, $dst}", (SCASW dstidx16:$dst), 0>;
 def : InstAlias<"scas\t{$dst, %eax|eax, $dst}", (SCASL dstidx32:$dst), 0>;
 def : InstAlias<"scas\t{$dst, %rax|rax, $dst}", (SCASQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
+def : InstAlias<"scas\t$dst", (SCASB dstidx8:$dst),  0>;
+def : InstAlias<"scas\t$dst", (SCASW dstidx16:$dst), 0>;
+def : InstAlias<"scas\t$dst", (SCASL dstidx32:$dst), 0>;
+def : InstAlias<"scas\t$dst", (SCASQ dstidx64:$dst), 0>, Requires<[In64BitMode]>;
+
+// cmps aliases. Mnemonic suffix being omitted because it's implicit
+// in the destination.
+def : InstAlias<"cmps\t{$dst, $src|$src, $dst}", (CMPSB dstidx8:$dst, srcidx8:$src),  0>;
+def : InstAlias<"cmps\t{$dst, $src|$src, $dst}", (CMPSW dstidx16:$dst, srcidx16:$src), 0>;
+def : InstAlias<"cmps\t{$dst, $src|$src, $dst}", (CMPSL dstidx32:$dst, srcidx32:$src), 0>;
+def : InstAlias<"cmps\t{$dst, $src|$src, $dst}", (CMPSQ dstidx64:$dst, srcidx64:$src), 0>, Requires<[In64BitMode]>;
+
+// movs aliases. Mnemonic suffix being omitted because it's implicit
+// in the destination.
+def : InstAlias<"movs\t{$src, $dst|$dst, $src}", (MOVSB dstidx8:$dst, srcidx8:$src),  0>;
+def : InstAlias<"movs\t{$src, $dst|$dst, $src}", (MOVSW dstidx16:$dst, srcidx16:$src), 0>;
+def : InstAlias<"movs\t{$src, $dst|$dst, $src}", (MOVSL dstidx32:$dst, srcidx32:$src), 0>;
+def : InstAlias<"movs\t{$src, $dst|$dst, $src}", (MOVSQ dstidx64:$dst, srcidx64:$src), 0>, Requires<[In64BitMode]>;
 
 // div and idiv aliases for explicit A register.
 def : InstAlias<"div{b}\t{$src, %al|al, $src}", (DIV8r  GR8 :$src)>;
 def : InstAlias<"div{w}\t{$src, %ax|ax, $src}", (DIV16r GR16:$src)>;
 def : InstAlias<"div{l}\t{$src, %eax|eax, $src}", (DIV32r GR32:$src)>;
 def : InstAlias<"div{q}\t{$src, %rax|rax, $src}", (DIV64r GR64:$src)>;
 def : InstAlias<"div{b}\t{$src, %al|al, $src}", (DIV8m  i8mem :$src)>;
 def : InstAlias<"div{w}\t{$src, %ax|ax, $src}", (DIV16m i16mem:$src)>;

 // "imul <imm>, B" is an alias for "imul <imm>, B, B".
 def : InstAlias<"imul{w}\t{$imm, $r|$r, $imm}", (IMUL16rri  GR16:$r, GR16:$r, i16imm:$imm), 0>;
 def : InstAlias<"imul{w}\t{$imm, $r|$r, $imm}", (IMUL16rri8 GR16:$r, GR16:$r, i16i8imm:$imm), 0>;
 def : InstAlias<"imul{l}\t{$imm, $r|$r, $imm}", (IMUL32rri  GR32:$r, GR32:$r, i32imm:$imm), 0>;
 def : InstAlias<"imul{l}\t{$imm, $r|$r, $imm}", (IMUL32rri8 GR32:$r, GR32:$r, i32i8imm:$imm), 0>;
 def : InstAlias<"imul{q}\t{$imm, $r|$r, $imm}", (IMUL64rri32 GR64:$r, GR64:$r, i64i32imm:$imm), 0>;
 def : InstAlias<"imul{q}\t{$imm, $r|$r, $imm}", (IMUL64rri8 GR64:$r, GR64:$r, i64i8imm:$imm), 0>;
 
+// ins aliases. Accept the mnemonic suffix being omitted because it's implicit
+// in the destination.
+def : InstAlias<"ins\t{%dx, $dst|$dst, dx}", (INSB dstidx8:$dst),  0>;
+def : InstAlias<"ins\t{%dx, $dst|$dst, dx}", (INSW dstidx16:$dst),  0>;
+def : InstAlias<"ins\t{%dx, $dst|$dst, dx}", (INSL dstidx32:$dst),  0>;
+
+// outs aliases. Accept the mnemonic suffix being omitted because it's implicit
+// in the source.
+def : InstAlias<"outs\t{$src, %dx|dx, $src}", (OUTSB srcidx8:$src),  0>;
+def : InstAlias<"outs\t{$src, %dx|dx, $src}", (OUTSW srcidx16:$src),  0>;
+def : InstAlias<"outs\t{$src, %dx|dx, $src}", (OUTSL srcidx32:$src),  0>;
+
 // inb %dx -> inb %al, %dx
 def : InstAlias<"inb\t{%dx|dx}", (IN8rr), 0>;
 def : InstAlias<"inw\t{%dx|dx}", (IN16rr), 0>;
 def : InstAlias<"inl\t{%dx|dx}", (IN32rr), 0>;
 def : InstAlias<"inb\t$port", (IN8ri u8imm:$port), 0>;
 def : InstAlias<"inw\t$port", (IN16ri u8imm:$port), 0>;
 def : InstAlias<"inl\t$port", (IN32ri u8imm:$port), 0>;
 

llvm/trunk/test/MC/X86/index-operations.s

 // 64: outsw %fs:(%esi), %dx # encoding: [0x66,0x64,0x67,0x6f]
 // 32: outsw %fs:(%esi), %dx # encoding: [0x66,0x64,0x6f]
 // 16: outsw %fs:(%esi), %dx # encoding: [0x64,0x67,0x6f]
 
 insw %dx, (%edi)
 // 64: insw %dx, %es:(%edi) # encoding: [0x66,0x67,0x6d]
 // 32: insw %dx, %es:(%edi) # encoding: [0x66,0x6d]
 // 16: insw %dx, %es:(%edi) # encoding: [0x67,0x6d]
+
+insw %dx, (%bx)
+// ERR64: invalid 16-bit base register
+// 32: insw %dx, %es:(%di) # encoding: [0x66,0x67,0x6d]
+// 16: insw %dx, %es:(%di) # encoding: [0x6d]
+
+insw %dx, (%ebx)
+// 64: insw %dx, %es:(%edi) # encoding: [0x66,0x67,0x6d]
+// 32: insw %dx, %es:(%edi) # encoding: [0x66,0x6d]
+// 16: insw %dx, %es:(%edi) # encoding: [0x67,0x6d]
+
+insw %dx, (%rbx)
+// 64: insw %dx, %es:(%rdi) # encoding: [0x66,0x6d]
+// ERR32: 64-bit
+// ERR16: 64-bit
+

llvm/trunk/test/MC/X86/intel-syntax.s

 
 loopz _foo
 loopnz _foo
 // CHECK: loope _foo
 // CHECK: loopne _foo
 
 sidt fword ptr [eax]
 // CHECK: sidtq (%eax)
+
+ins byte ptr [eax], dx
+// CHECK: insb %dx, %es:(%edi)
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+outs dx, word ptr [eax]
+// CHECK: outsw (%esi), %dx
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+lods dword ptr [eax]
+// CHECK: lodsl (%esi), %eax
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+stos qword ptr [eax]
+// CHECK: stosq %rax, %es:(%edi)
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+scas byte ptr [eax]
+// CHECK: scasb %es:(%edi), %al
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+cmps word ptr [eax], word ptr [ebx]
+// CHECK: cmpsw %es:(%edi), (%esi)
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+movs dword ptr [eax], dword ptr [ebx]
+// CHECK: movsl (%esi), %es:(%edi)
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location