[X86AsmParser] Refactoring, (almost) NFC.

[coby]

Summary

Some refactoring to X86AsmParser, mostly regarding the way rewrites are conducted.
Mainly, we try to concentrate all the rewrite effort under one hood, so it'll hopefully be less of a mess and easier to maintain and understand.
naturally, some frontend tests were affected: D36794

[avt77]

IntelExpr::isValid looks rather strange because it validates possible multiplication only. But if we have Scale == 1 and don't have either BaseReg or Disp?

[rnk]

I didn't get to this this week, but this code could definitely use some refactoring. :)

[coby]

In D36793#845245, @avt77 wrote:

IntelExpr::isValid looks rather strange because it validates possible multiplication only. But if we have Scale == 1 and don't have either BaseReg or Disp?

first of all, thanks for your comment.
AOK_IntelExpr is a rewrite mechanism, and as such its mandate is to be a dumb (but convenient) conveyor. 'isValid()', under those guidelines, checks some most trivial parts which can be inferred with ease. others parts are assumed to be validated on the path that led to its emission (either pre or post).
Specific to the scenario you've described - if we 'have' Scale == 1 (which is the default case) any other part is 'considered' legal (to the purpose of this rewrite, again - this is simply a conveyor - we expect validation effort to be conducted by the parser). Index/Base/ImmDisp will be emitted if present.

[coby]

Removed AOK_Delete as it has no use anymore

[rnk]

lib/Target/X86/AsmParser/X86AsmParser.cpp

339–378

Can you clang-format this?

604

formatting

606

I think HasSymbol would be a better name for this, and Sym != nullptr is a more readable way to force a boolean conversion.

1708

I think ParseIntelMemoryOperandSize would be a better name for this.

1790

Use != instead of <>, it's not commonly used.

[coby]

Addressed @rnk 's comments (thanks!)

[rnk]

lgtm

lib/Target/X86/AsmParser/X86AsmParser.cpp

1306

Woohoo, excited to see this go. :)

[coby]

lib/Target/X86/AsmParser/X86AsmParser.cpp

1306

Aye, won't yearn for it either !

include/llvm/MC/MCParser/MCTargetAsmParser.h

 template <typename T> class SmallVectorImpl;
 
 using OperandVector = SmallVectorImpl<std::unique_ptr<MCParsedAsmOperand>>;
 
 enum AsmRewriteKind {
   AOK_Delete = 0,     // Rewrite should be ignored.
   AOK_Align,          // Rewrite align as .align.
   AOK_EVEN,           // Rewrite even as .even.
-  AOK_DotOperator,    // Rewrite a dot operator expression as an immediate.
-                      // E.g., [eax].foo.bar -> [eax].8
   AOK_Emit,           // Rewrite _emit as .byte.
-  AOK_Imm,            // Rewrite as $$N.
-  AOK_ImmPrefix,      // Add $$ before a parsed Imm.
   AOK_Input,          // Rewrite in terms of $N.
   AOK_Output,         // Rewrite in terms of $N.
   AOK_SizeDirective,  // Add a sizing directive (e.g., dword ptr).
   AOK_Label,          // Rewrite local labels.
   AOK_EndOfStatement, // Add EndOfStatement (e.g., "\n\t").
-  AOK_Skip            // Skip emission (e.g., offset/type operators).
+  AOK_Skip,           // Skip emission (e.g., offset/type operators).
+  AOK_IntelExpr       // SizeDirective SymDisp [BaseReg + IndexReg * Scale + ImmDisp]
 };
 
 const char AsmRewritePrecedence [] = {
   0, // AOK_Delete
   2, // AOK_Align
   2, // AOK_EVEN
-  2, // AOK_DotOperator
   2, // AOK_Emit
-  4, // AOK_Imm
-  4, // AOK_ImmPrefix
   3, // AOK_Input
   3, // AOK_Output
   5, // AOK_SizeDirective
   1, // AOK_Label
   5, // AOK_EndOfStatement
-  2  // AOK_Skip
+  2, // AOK_Skip
+  2  // AOK_IntelExpr
+};
+
+// Represnt the various parts which makes up an intel expression,
+// used for emitting compound intel expressions
+struct IntelExpr {
+  bool NeedBracs;
+  int64_t Imm;
+  StringRef BaseReg;
+  StringRef IndexReg;
+  unsigned Scale;
+
+  IntelExpr(bool needBracs = false) : NeedBracs(needBracs), Imm(0),
+    BaseReg(StringRef()), IndexReg(StringRef()),
+    Scale(1) {}
+  // Compund immediate expression
+  explicit IntelExpr(int64_t imm, bool needBracs) : IntelExpr(needBracs) {
+    Imm = imm;
+  }
+  // [Reg + ImmediateExpression]
+  // We don't bother to emit an immediate expression evaluated to zero
+  IntelExpr(StringRef reg, int64_t imm = 0, unsigned scale = 0,
+    bool needBracs = true) :
+    IntelExpr(imm, needBracs) {
+    IndexReg = reg;
+    if (scale)
+      Scale = scale;
+  }
+  // [BaseReg + IndexReg * ScaleExpression + ImmediateExpression]
+  IntelExpr(StringRef baseReg, StringRef indexReg, unsigned scale = 0,
+    int64_t imm = 0, bool needBracs = true) :
+    IntelExpr(indexReg, imm, scale, needBracs) {
+    BaseReg = baseReg;
+  }
+  bool hasBaseReg() const {
+    return BaseReg.size();
+  }
+  bool hasIndexReg() const {
+    return IndexReg.size();
+  }
+  bool hasRegs() const {
+    return hasBaseReg() || hasIndexReg();
+  }
+  bool isValid() const {
+    return (Scale == 1) ||
+           (hasIndexReg() && (Scale == 2 || Scale == 4 || Scale == 8));
+  }
 };
 
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   int64_t Val;
   StringRef Label;
+  IntelExpr IntelExp;
 
 public:
   AsmRewrite(AsmRewriteKind kind, SMLoc loc, unsigned len = 0, int64_t val = 0)
     : Kind(kind), Loc(loc), Len(len), Val(val) {}
   AsmRewrite(AsmRewriteKind kind, SMLoc loc, unsigned len, StringRef label)
     : Kind(kind), Loc(loc), Len(len), Val(0), Label(label) {}
+  AsmRewrite(SMLoc loc, unsigned len, IntelExpr exp)
+    : Kind(AOK_IntelExpr), Loc(loc), Len(len), IntelExp(exp) {}
 };
 
 struct ParseInstructionInfo {
   SmallVectorImpl<AsmRewrite> *AsmRewrites = nullptr;
 
   ParseInstructionInfo() = default;
   ParseInstructionInfo(SmallVectorImpl<AsmRewrite> *rewrites)
     : AsmRewrites(rewrites) {}

lib/MC/MCParser/AsmParser.cpp

if (Kind == AOK_Skip) {
       continue;
     }
 
     unsigned AdditionalSkip = 0;
     // Rewrite expressions in $N notation.
     switch (Kind) {
     default:
       break;
-    case AOK_Imm:
-      OS << "$$" << AR.Val;
-      break;
-    case AOK_ImmPrefix:
-      OS << "$$";
+    case AOK_IntelExpr:
+      assert(AR.IntelExp.isValid() && "cannot write invalid intel expression");
+      if (AR.IntelExp.NeedBracs)
+        OS << "[";
+      if (AR.IntelExp.hasBaseReg())
+        OS << AR.IntelExp.BaseReg;
+      if (AR.IntelExp.hasIndexReg())
+        OS << (AR.IntelExp.hasBaseReg() ? " + " : "")
+           << AR.IntelExp.IndexReg;
+      if (AR.IntelExp.Scale > 1)
+          OS << " * $$" << AR.IntelExp.Scale;
+      if (AR.IntelExp.Imm || !AR.IntelExp.hasRegs())
+        OS << (AR.IntelExp.hasRegs() ? " + $$" : "$$") << AR.IntelExp.Imm;
+      if (AR.IntelExp.NeedBracs)
+        OS << "]";
       break;
     case AOK_Label:
       OS << Ctx.getAsmInfo()->getPrivateLabelPrefix() << AR.Label;
       break;
     case AOK_Input:
       OS << '$' << InputIdx++;
       break;
     case AOK_Output:
case AOK_Align: {
       OS << ' ' << Val;
       assert(Val < 10 && "Expected alignment less then 2^10.");
       AdditionalSkip = (Val < 4) ? 2 : Val < 7 ? 3 : 4;
       break;
     }
     case AOK_EVEN:
       OS << ".even";
       break;
-    case AOK_DotOperator:
-      // Insert the dot if the user omitted it.
-      OS.flush();
-      if (AsmStringIR.back() != '.')
-        OS << '.';
-      OS << AR.Val;
-      break;
     case AOK_EndOfStatement:
       OS << "\n\t";
       break;
     }
 
     // Skip the original expression.
     AsmStart = Loc + AR.Len + AdditionalSkip;
   }

lib/Target/X86/AsmParser/X86AsmParser.cpp

 //===-- X86AsmParser.cpp - Parse X86 assembly to MCInst instructions ------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/X86BaseInfo.h"
 #include "X86AsmInstrumentation.h"
 #include "X86AsmParserCommon.h"
 #include "X86Operand.h"
+#include "InstPrinter/X86IntelInstPrinter.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
int64_t execute() {
         }
       }
       assert (OperandStack.size() == 1 && "Expected a single result.");
       return OperandStack.pop_back_val().second;
     }
   };
 
   enum IntelExprState {
+    IES_INIT,
     IES_OR,
     IES_XOR,
     IES_AND,
     IES_LSHIFT,
     IES_RSHIFT,
     IES_PLUS,
     IES_MINUS,
     IES_NOT,
private:
   };
 
   class IntelExprStateMachine {
     IntelExprState State, PrevState;
     unsigned BaseReg, IndexReg, TmpReg, Scale;
     int64_t Imm;
     const MCExpr *Sym;
     StringRef SymName;
-    bool StopOnLBrac, AddImmPrefix;
     InfixCalculator IC;
     InlineAsmIdentifierInfo Info;
+    short BracCount;
+    bool MemExpr;
 
   public:
-    IntelExprStateMachine(int64_t imm, bool stoponlbrac, bool addimmprefix) :
-      State(IES_PLUS), PrevState(IES_ERROR), BaseReg(0), IndexReg(0), TmpReg(0),
-      Scale(1), Imm(imm), Sym(nullptr), StopOnLBrac(stoponlbrac),
-      AddImmPrefix(addimmprefix) { Info.clear(); }
-
-    unsigned getBaseReg() { return BaseReg; }
-    unsigned getIndexReg() { return IndexReg; }
-    unsigned getScale() { return Scale; }
-    const MCExpr *getSym() { return Sym; }
-    StringRef getSymName() { return SymName; }
-    int64_t getImm() { return Imm + IC.execute(); }
+    IntelExprStateMachine() :
+      State(IES_INIT), PrevState(IES_ERROR), BaseReg(0), IndexReg(0),
+      TmpReg(0), Scale(1), Imm(0), Sym(nullptr), BracCount(0), MemExpr(false) {
+      Info.clear();
+    }
+
+    void addImm(int64_t imm) {
+      Imm += imm;
+    }
+    short getBracCount() {
+      return BracCount;
+    }
+    bool isMemExpr() {
+      return MemExpr;
+    }
+    unsigned getBaseReg() {
+      return BaseReg;
+    }
+    unsigned getIndexReg() {
+      return IndexReg;
+    }
+    unsigned getScale() {
+      return Scale;
+    }
+    const MCExpr *getSym() {
+      return Sym;
+    }
+    StringRef getSymName() {
+      return SymName;
+    }
+    int64_t getImm() {
+      return Imm + IC.execute();
+    }
     bool isValidEndState() {
       return State == IES_RBRAC || State == IES_INTEGER;
     }
-    bool getStopOnLBrac() { return StopOnLBrac; }
-    bool getAddImmPrefix() { return AddImmPrefix; }
-    bool hadError() { return State == IES_ERROR; }
-
+    bool hadError() {
+      return State == IES_ERROR;
+    }
     InlineAsmIdentifierInfo &getIdentifierInfo() {
       return Info;
     }
 
     void onOr() {
       IntelExprState CurrState = State;
       switch (State) {
       default:
         State = IES_ERROR;
bool onMinus(StringRef &ErrMsg) {
       case IES_DIVIDE:
       case IES_MOD:
       case IES_LPAREN:
       case IES_RPAREN:
       case IES_LBRAC:
       case IES_RBRAC:
       case IES_INTEGER:
       case IES_REGISTER:
+      case IES_INIT:
         State = IES_MINUS;
         // push minus operator if it is not a negate operator
         if (CurrState == IES_REGISTER || CurrState == IES_RPAREN ||
             CurrState == IES_INTEGER  || CurrState == IES_RBRAC)
           IC.pushOperator(IC_MINUS);
         else if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
           // We have negate operator for Scale: it's illegal
           ErrMsg = "Scale can't be negative";
void onNot() {
       case IES_PLUS:
       case IES_MINUS:
       case IES_NOT:
       case IES_MULTIPLY:
       case IES_DIVIDE:
       case IES_MOD:
       case IES_LPAREN:
       case IES_LBRAC:
+      case IES_INIT:
         State = IES_NOT;
         IC.pushOperator(IC_NOT);
         break;
       }
       PrevState = CurrState;
     }
 
     bool onRegister(unsigned Reg, StringRef &ErrMsg) {
       IntelExprState CurrState = State;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_PLUS:
       case IES_LPAREN:
+      case IES_LBRAC:
         State = IES_REGISTER;
         TmpReg = Reg;
         IC.pushOperand(IC_REGISTER);
         break;
       case IES_MULTIPLY:
         // Index Register - Scale * Register
         if (PrevState == IES_INTEGER) {
           if (IndexReg) {
bool onRegister(unsigned Reg, StringRef &ErrMsg) {
         } else {
           State = IES_ERROR;
         }
         break;
       }
       PrevState = CurrState;
       return false;
     }
-    void onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName) {
+    bool onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName,
+      StringRef &ErrMsg) {
       PrevState = State;
+      bool HasError = (bool)Sym;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_PLUS:
       case IES_MINUS:
       case IES_NOT:
+      case IES_INIT:
+      case IES_LBRAC:
+        MemExpr = !(SymRef->getKind() == MCExpr::Constant);
         State = IES_INTEGER;
         Sym = SymRef;
         SymName = SymRefName;
         IC.pushOperand(IC_IMM);
         break;
       }
+      if (HasError)
+        ErrMsg = "cannot use more than one symbol in memory operand";
+      return HasError;
     }
     bool onInteger(int64_t TmpInt, StringRef &ErrMsg) {
       IntelExprState CurrState = State;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_PLUS:
       case IES_MINUS:
       case IES_NOT:
       case IES_OR:
       case IES_XOR:
       case IES_AND:
       case IES_LSHIFT:
       case IES_RSHIFT:
       case IES_DIVIDE:
       case IES_MOD:
       case IES_MULTIPLY:
       case IES_LPAREN:
+      case IES_INIT:
+      case IES_LBRAC:
         State = IES_INTEGER;
         if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
           // Index Register - Register * Scale
           if (IndexReg) {
             ErrMsg = "BaseReg/IndexReg already set!";
             return true;
           }
           IndexReg = TmpReg;
void onMod() {
         break;
       case IES_INTEGER:
       case IES_RPAREN:
         State = IES_MOD;
         IC.pushOperator(IC_MOD);
         break;
       }
     }
-    void onLBrac() {
+    bool onLBrac() {
+      if (BracCount)
+        return true;
       PrevState = State;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_RBRAC:
+      case IES_INTEGER:
+      case IES_RPAREN:
         State = IES_PLUS;
         IC.pushOperator(IC_PLUS);
         break;
+      case IES_INIT:
+        assert(!BracCount && "BracCount should be zero on parsing's start");
+        State = IES_LBRAC;
+        break;
       }
+      MemExpr = true;
+      BracCount++;
+      return false;
     }
-    void onRBrac() {
+    bool onRBrac() {
       IntelExprState CurrState = State;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_INTEGER:
       case IES_REGISTER:
       case IES_RPAREN:
+        if (BracCount-- != 1)
+          return true;
         State = IES_RBRAC;
         if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
           // If we already have a BaseReg, then assume this is the IndexReg with
           // a scale of 1.
           if (!BaseReg) {
             BaseReg = TmpReg;
           } else {
             assert (!IndexReg && "BaseReg/IndexReg already set!");
             IndexReg = TmpReg;
             Scale = 1;
           }
         }
         break;
       }
       PrevState = CurrState;
+      return false;
     }
     void onLParen() {
       IntelExprState CurrState = State;
       switch (State) {
       default:
         State = IES_ERROR;
         break;
       case IES_PLUS:
       case IES_MINUS:
       case IES_NOT:
       case IES_OR:
       case IES_XOR:
       case IES_AND:
       case IES_LSHIFT:
       case IES_RSHIFT:
       case IES_MULTIPLY:
       case IES_DIVIDE:
       case IES_MOD:
       case IES_LPAREN:
+      case IES_INIT:
+      case IES_LBRAC:
         State = IES_LPAREN;
         IC.pushOperator(IC_LPAREN);
         break;
       }
       PrevState = CurrState;
     }
     void onRParen() {
       PrevState = State;
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);
-  unsigned IdentifyIntelOperator(StringRef Name);
-  unsigned ParseIntelOperator(unsigned OpKind);
-  std::unique_ptr<X86Operand>
-  ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
+  bool ParseIntelDotOperator(IntelExprStateMachine &SM, SMLoc &End);
+  unsigned IdentifyIntelInlineAsmOperator(StringRef Name);
+  unsigned ParseIntelInlineAsmOperator(unsigned OpKind);
   std::unique_ptr<X86Operand> ParseRoundingModeOp(SMLoc Start, SMLoc End);
   bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
+  void RewriteIntelExpression(IntelExprStateMachine &SM, SMLoc Start,
+                              SMLoc End);
   bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
-  std::unique_ptr<X86Operand>
-  ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
-                           bool isSymbol, unsigned Size);
-  bool ParseIntelIdentifier(const MCExpr *&Val, StringRef &Identifier,
+  bool ParseIntelInlineAsmIdentifier(const MCExpr *&Val, StringRef &Identifier,
                                      InlineAsmIdentifierInfo &Info,
                                      bool IsUnevaluatedOperand, SMLoc &End);
 
   std::unique_ptr<X86Operand> ParseMemOperand(unsigned SegReg, SMLoc StartLoc);
 
+  bool ParseIntelSizeDirective(unsigned &Size);
   std::unique_ptr<X86Operand>
   CreateMemForInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
                         unsigned IndexReg, unsigned Scale, SMLoc Start,
                         SMLoc End, unsigned Size, StringRef Identifier,
-                        InlineAsmIdentifierInfo &Info,
-                        bool AllowBetterSizeMatch = false);
+                        InlineAsmIdentifierInfo &Info);
 
   bool parseDirectiveEven(SMLoc L);
   bool ParseDirectiveWord(unsigned Size, SMLoc L);
   bool ParseDirectiveCode(StringRef IDVal, SMLoc L);
 
   bool processInstruction(MCInst &Inst, const OperandVector &Ops);
 
   /// Wrapper around MCStreamer::EmitInstruction(). Possibly adds

 }
 
 std::unique_ptr<X86Operand> X86AsmParser::ParseOperand() {
   if (isParsingIntelSyntax())
     return ParseIntelOperand();
   return ParseATTOperand();
 }
 
-/// getIntelMemOperandSize - Return intel memory operand size.
-static unsigned getIntelMemOperandSize(StringRef OpStr) {
-  unsigned Size = StringSwitch<unsigned>(OpStr)
-    .Cases("BYTE", "byte", 8)
-    .Cases("WORD", "word", 16)
-    .Cases("DWORD", "dword", 32)
-    .Cases("FWORD", "fword", 48)
-    .Cases("QWORD", "qword", 64)
-    .Cases("MMWORD","mmword", 64)
-    .Cases("XWORD", "xword", 80)
-    .Cases("TBYTE", "tbyte", 80)
-    .Cases("XMMWORD", "xmmword", 128)
-    .Cases("YMMWORD", "ymmword", 256)
-    .Cases("ZMMWORD", "zmmword", 512)
-    .Cases("OPAQUE", "opaque", -1U) // needs to be non-zero, but doesn't matter
-    .Default(0);
-  return Size;
-}
-
 std::unique_ptr<X86Operand> X86AsmParser::CreateMemForInlineAsm(
     unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
     unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
-    InlineAsmIdentifierInfo &Info, bool AllowBetterSizeMatch) {
+    InlineAsmIdentifierInfo &Info) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
   // some other label reference.
   if (isa<MCSymbolRefExpr>(Disp) && Info.OpDecl && !Info.IsVarDecl) {
     // Insert an explicit size if the user didn't have one.
     if (!Size) {
       Size = getPointerWidth();
       InstInfo->AsmRewrites->emplace_back(AOK_SizeDirective, Start,
                                           /*Len=*/0, Size);
std::unique_ptr<X86Operand> X86AsmParser::CreateMemForInlineAsm(
   // if we don't know the actual value at this time.  This is necessary to
   // get the matching correct in some cases.
   BaseReg = BaseReg ? BaseReg : 1;
   return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp, BaseReg,
                                IndexReg, Scale, Start, End, Size, Identifier,
                                Info.OpDecl, FrontendSize);
 }
 
-static void
-RewriteIntelBracExpression(SmallVectorImpl<AsmRewrite> &AsmRewrites,
-                           StringRef SymName, int64_t ImmDisp,
-                           int64_t FinalImmDisp, SMLoc &BracLoc,
-                           SMLoc &StartInBrac, SMLoc &End) {
-  // Remove the '[' and ']' from the IR string.
-  AsmRewrites.emplace_back(AOK_Skip, BracLoc, 1);
-  AsmRewrites.emplace_back(AOK_Skip, End, 1);
-
-  // If ImmDisp is non-zero, then we parsed a displacement before the
-  // bracketed expression (i.e., ImmDisp [ BaseReg + Scale*IndexReg + Disp])
-  // If ImmDisp doesn't match the displacement computed by the state machine
-  // then we have an additional displacement in the bracketed expression.
-  if (ImmDisp != FinalImmDisp) {
-    if (ImmDisp) {
-      // We have an immediate displacement before the bracketed expression.
-      // Adjust this to match the final immediate displacement.
-      bool Found = false;
-      for (AsmRewrite &AR : AsmRewrites) {
-        if (AR.Loc.getPointer() > BracLoc.getPointer())
-          continue;
-        if (AR.Kind == AOK_ImmPrefix || AR.Kind == AOK_Imm) {
-          assert (!Found && "ImmDisp already rewritten.");
-          AR.Kind = AOK_Imm;
-          AR.Len = BracLoc.getPointer() - AR.Loc.getPointer();
-          AR.Val = FinalImmDisp;
-          Found = true;
-          break;
-        }
-      }
-      assert (Found && "Unable to rewrite ImmDisp.");
-      (void)Found;
-    } else {
-      // We have a symbolic and an immediate displacement, but no displacement
-      // before the bracketed expression.  Put the immediate displacement
-      // before the bracketed expression.
-      AsmRewrites.emplace_back(AOK_Imm, BracLoc, 0, FinalImmDisp);
-    }
-  }
-  // Remove all the ImmPrefix rewrites within the brackets.
-  // We may have some Imm rewrties as a result of an operator applying,
-  // remove them as well
-  for (AsmRewrite &AR : AsmRewrites) {
-    if (AR.Loc.getPointer() < StartInBrac.getPointer())
-      continue;
-    if (AR.Kind == AOK_ImmPrefix || AR.Kind == AOK_Imm)
-      AR.Kind = AOK_Delete;
-  }
-  const char *SymLocPtr = SymName.data();
-  // Skip everything before the symbol.
-  if (unsigned Len = SymLocPtr - StartInBrac.getPointer()) {
-    assert(Len > 0 && "Expected a non-negative length.");
-    AsmRewrites.emplace_back(AOK_Skip, StartInBrac, Len);
-  }
-  // Skip everything after the symbol.
-  if (unsigned Len = End.getPointer() - (SymLocPtr + SymName.size())) {
-    SMLoc Loc = SMLoc::getFromPointer(SymLocPtr + SymName.size());
-    assert(Len > 0 && "Expected a non-negative length.");
-    AsmRewrites.emplace_back(AOK_Skip, Loc, Len);
-  }
-}
-
 // Some binary bitwise operators have a named synonymous
 // Query a candidate string for being such a named operator
 // and if so - invoke the appropriate handler
 bool X86AsmParser::ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM) {
   // A named operator should be either lower or upper case, but not a mix
   if (Name.compare(Name.lower()) && Name.compare(Name.upper()))
     return false;
   if (Name.equals_lower("not"))
bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   StringRef ErrMsg;
 
   AsmToken::TokenKind PrevTK = AsmToken::Error;
   bool Done = false;
   while (!Done) {
     bool UpdateLocLex = true;
-
     AsmToken::TokenKind TK = getLexer().getKind();
-    // The period in the dot operator (e.g., [ebx].foo.bar) is parsed as an
-    // identifier.  Don't try an parse it as a register.
-    if (PrevTK != AsmToken::Error && Tok.getString().startswith(".") &&
-        TK != AsmToken::Identifier)
-      break;
-
-    // If we're parsing an immediate expression, we don't expect a '['.
-    if (SM.getStopOnLBrac() && getLexer().getKind() == AsmToken::LBrac)
-      break;
 
     switch (TK) {
-    default: {
-      if (SM.isValidEndState()) {
-        Done = true;
+    default:
+      if ((Done = SM.isValidEndState()))
         break;
-      }
       return Error(Tok.getLoc(), "unknown token in expression");
-    }
-    case AsmToken::EndOfStatement: {
+    case AsmToken::EndOfStatement:
       Done = true;
       break;
-    }
+    case AsmToken::Real:
+      // DotOperator: [ebx].0
+      UpdateLocLex = false;
+      if (ParseIntelDotOperator(SM, End))
+        return true;
+      break;
     case AsmToken::String:
     case AsmToken::Identifier: {
       // This could be a register or a symbolic displacement.
       unsigned TmpReg;
       const MCExpr *Val;
       SMLoc IdentLoc = Tok.getLoc();
       StringRef Identifier = Tok.getString();
       UpdateLocLex = false;
       if (TK != AsmToken::String && !ParseRegister(TmpReg, IdentLoc, End)) {
         if (SM.onRegister(TmpReg, ErrMsg))
           return Error(Tok.getLoc(), ErrMsg);
       } else if (ParseIntelNamedOperator(Identifier, SM)) {
         UpdateLocLex = true;
       } else if (!isParsingInlineAsm()) {
         if (getParser().parsePrimaryExpr(Val, End))
           return Error(Tok.getLoc(), "Unexpected identifier!");
-        SM.onIdentifierExpr(Val, Identifier);
-      } else if (unsigned OpKind = IdentifyIntelOperator(Identifier)) {
-        if (OpKind == IOK_OFFSET) 
+        if (auto *CE = dyn_cast<MCConstantExpr>(Val)) {
+          if (SM.onInteger(CE->getValue(), ErrMsg))
+            return Error(IdentLoc, ErrMsg);
+        } else if (SM.onIdentifierExpr(Val, Identifier, ErrMsg))
+          return Error(IdentLoc, ErrMsg);
+      } else if (unsigned OpKind = IdentifyIntelInlineAsmOperator(Identifier)) {
+        if (OpKind == IOK_OFFSET)
           return Error(IdentLoc, "Dealing OFFSET operator as part of"
             "a compound immediate expression is yet to be supported");
-        int64_t Val = ParseIntelOperator(OpKind);
+        int64_t Val = ParseIntelInlineAsmOperator(OpKind);
         if (!Val)
           return true;
         if (SM.onInteger(Val, ErrMsg))
           return Error(IdentLoc, ErrMsg);
-      } else if (Identifier.find('.') != StringRef::npos &&
-            PrevTK == AsmToken::RBrac) {
-          return false;
-      } else {
-        InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
-        if (ParseIntelIdentifier(Val, Identifier, Info,
-                                 /*Unevaluated=*/false, End))
+      } else if (Identifier.count('.') && PrevTK == AsmToken::RBrac) {
+          if (ParseIntelDotOperator(SM, End))
             return true;
-        SM.onIdentifierExpr(Val, Identifier);
+      } else if (ParseIntelInlineAsmIdentifier(Val, Identifier,
+                                               SM.getIdentifierInfo(),
+                                               /*Unevaluated=*/false, End)) {
+        return true;
+      } else if (SM.onIdentifierExpr(Val, Identifier, ErrMsg)) {
+        return Error(IdentLoc, ErrMsg);
       }
       break;
     }
     case AsmToken::Integer: {
-      if (isParsingInlineAsm() && SM.getAddImmPrefix())
-        InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Tok.getLoc());
       // Look for 'b' or 'f' following an Integer as a directional label
       SMLoc Loc = getTok().getLoc();
       int64_t IntVal = getTok().getIntVal();
       End = consumeToken();
       UpdateLocLex = false;
       if (getLexer().getKind() == AsmToken::Identifier) {
         StringRef IDVal = getTok().getString();
         if (IDVal == "f" || IDVal == "b") {
           MCSymbol *Sym =
               getContext().getDirectionalLocalSymbol(IntVal, IDVal == "b");
           MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
           const MCExpr *Val =
               MCSymbolRefExpr::create(Sym, Variant, getContext());
           if (IDVal == "b" && Sym->isUndefined())
             return Error(Loc, "invalid reference to undefined symbol");
           StringRef Identifier = Sym->getName();
-          SM.onIdentifierExpr(Val, Identifier);
+          if (SM.onIdentifierExpr(Val, Identifier, ErrMsg))
+            return Error(Loc, ErrMsg);
           End = consumeToken();
         } else {
           if (SM.onInteger(IntVal, ErrMsg))
             return Error(Loc, ErrMsg);
         }
       } else {
         if (SM.onInteger(IntVal, ErrMsg))
           return Error(Loc, ErrMsg);
while (!Done) {
     case AsmToken::Slash:   SM.onDivide(); break;
     case AsmToken::Pipe:    SM.onOr(); break;
     case AsmToken::Caret:   SM.onXor(); break;
     case AsmToken::Amp:     SM.onAnd(); break;
     case AsmToken::LessLess:
                             SM.onLShift(); break;
     case AsmToken::GreaterGreater:
                             SM.onRShift(); break;
-    case AsmToken::LBrac:   SM.onLBrac(); break;
-    case AsmToken::RBrac:   SM.onRBrac(); break;
+    case AsmToken::LBrac:
+      if (SM.onLBrac())
+        return Error(Tok.getLoc(), "unexpected bracket encountered");
+      break;
+    case AsmToken::RBrac:
+      if (SM.onRBrac())
+        return Error(Tok.getLoc(), "unexpected bracket encountered");
+      break;
     case AsmToken::LParen:  SM.onLParen(); break;
     case AsmToken::RParen:  SM.onRParen(); break;
     }
     if (SM.hadError())
       return Error(Tok.getLoc(), "unknown token in expression");
 
     if (!Done && UpdateLocLex)
       End = consumeToken();
 
     PrevTK = TK;
   }
   return false;
 }
 
-std::unique_ptr<X86Operand>
-X86AsmParser::ParseIntelBracExpression(unsigned SegReg, SMLoc Start,
-                                       int64_t ImmDisp, bool isSymbol,
-                                       unsigned Size) {
-  MCAsmParser &Parser = getParser();
-  const AsmToken &Tok = Parser.getTok();
-  SMLoc BracLoc = Tok.getLoc(), End = Tok.getEndLoc();
-  if (getLexer().isNot(AsmToken::LBrac))
-    return ErrorOperand(BracLoc, "Expected '[' token!");
-  Parser.Lex(); // Eat '['
-
-  SMLoc StartInBrac = Parser.getTok().getLoc();
-  // Parse [ Symbol + ImmDisp ] and [ BaseReg + Scale*IndexReg + ImmDisp ].  We
-  // may have already parsed an immediate displacement before the bracketed
-  // expression.
-  IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
-  if (ParseIntelExpression(SM, End))
-    return nullptr;
-
-  const MCExpr *Disp = nullptr;
-  if (const MCExpr *Sym = SM.getSym()) {
-    // A symbolic displacement.
-    Disp = Sym;
-    if (isParsingInlineAsm())
-      RewriteIntelBracExpression(*InstInfo->AsmRewrites, SM.getSymName(),
-                                 ImmDisp, SM.getImm(), BracLoc, StartInBrac,
-                                 End);
-  }
-
-  if (SM.getImm() || !Disp) {
-    const MCExpr *Imm = MCConstantExpr::create(SM.getImm(), getContext());
-    if (Disp)
-      Disp = MCBinaryExpr::createAdd(Disp, Imm, getContext());
-    else
-      Disp = Imm;  // An immediate displacement only.
-  }
-
-  // Parse struct field access.  Intel requires a dot, but MSVC doesn't.  MSVC
-  // will in fact do global lookup the field name inside all global typedefs,
-  // but we don't emulate that.
-  if ((Parser.getTok().getKind() == AsmToken::Identifier ||
-       Parser.getTok().getKind() == AsmToken::Dot ||
-       Parser.getTok().getKind() == AsmToken::Real) &&
-      Parser.getTok().getString().find('.') != StringRef::npos) {
-    const MCExpr *NewDisp;
-    if (ParseIntelDotOperator(Disp, NewDisp))
-      return nullptr;
-
-    End = Tok.getEndLoc();
-    Parser.Lex();  // Eat the field.
-    Disp = NewDisp;
-  }
-
-  if (isSymbol) {
-    if (SM.getSym()) {
-      Error(Start, "cannot use more than one symbol in memory operand");
-      return nullptr;
-    }
-    if (SM.getBaseReg()) {
-      Error(Start, "cannot use base register with variable reference");
-      return nullptr;
-    }
-    if (SM.getIndexReg()) {
-      Error(Start, "cannot use index register with variable reference");
-      return nullptr;
-    }
-  }
-
-  int BaseReg = SM.getBaseReg();
-  int IndexReg = SM.getIndexReg();
-  int Scale = SM.getScale();
-  if (!isParsingInlineAsm()) {
-    // handle [-42]
-    if (!BaseReg && !IndexReg) {
-      if (!SegReg)
-        return X86Operand::CreateMem(getPointerWidth(), Disp, Start, End, Size);
-      return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp, 0, 0, 1,
-                                   Start, End, Size);
-    }
-    StringRef ErrMsg;
-    if (CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, ErrMsg)) {
-      Error(StartInBrac, ErrMsg);
-      return nullptr;
+void X86AsmParser::RewriteIntelExpression(IntelExprStateMachine &SM,
+                                          SMLoc Start, SMLoc End) {
+  SMLoc Loc = Start;
+  unsigned ExprLen = End.getPointer() - Start.getPointer();
+  // Skip everything before a symbol displacement (if we have one)
+    StringRef SymName = SM.getSymName();
+    if (unsigned Len =  SymName.data() - Start.getPointer())
+      InstInfo->AsmRewrites->emplace_back(AOK_Skip, Start, Len);
+    Loc = SMLoc::getFromPointer(SymName.data() + SymName.size());
+    ExprLen = End.getPointer() - (SymName.data() + SymName.size());
+    // If we have only a symbol than there's no need for complex rewrite,
+    // simply skip everything after it
+    if (!(SM.getBaseReg() || SM.getIndexReg() || SM.getImm())) {
+      if (ExprLen)
+        InstInfo->AsmRewrites->emplace_back(AOK_Skip, Loc, ExprLen);
+      return;
     }
-    return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp, BaseReg,
-                                 IndexReg, Scale, Start, End, Size);
   }
-
-  InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
-  return CreateMemForInlineAsm(SegReg, Disp, BaseReg, IndexReg, Scale, Start,
-                               End, Size, SM.getSymName(), Info,
-                               isParsingInlineAsm());
+  // Build an Intel Expression rewrite
+  StringRef BaseRegStr;
+  StringRef IndexRegStr;
+  if (SM.getBaseReg())
+    BaseRegStr = X86IntelInstPrinter::getRegisterName(SM.getBaseReg());
+  if (SM.getIndexReg())
+    IndexRegStr = X86IntelInstPrinter::getRegisterName(SM.getIndexReg());
+  // Emit it
+  IntelExpr Expr(BaseRegStr, IndexRegStr, SM.getScale(), SM.getImm(), SM.isMemExpr());
+  InstInfo->AsmRewrites->emplace_back(Loc, ExprLen, Expr);
 }
 
 // Inline assembly may use variable names with namespace alias qualifiers.
-bool X86AsmParser::ParseIntelIdentifier(const MCExpr *&Val,
+bool X86AsmParser::ParseIntelInlineAsmIdentifier(const MCExpr *&Val,
                                                  StringRef &Identifier,
                                                  InlineAsmIdentifierInfo &Info,
-                                        bool IsUnevaluatedOperand, SMLoc &End) {
+                                                 bool IsUnevaluatedOperand,
+                                                 SMLoc &End) {
   MCAsmParser &Parser = getParser();
   assert(isParsingInlineAsm() && "Expected to be parsing inline assembly.");
   Val = nullptr;
 
   StringRef LineBuf(Identifier.data());
   void *Result =
     SemaCallback->LookupInlineAsmIdentifier(LineBuf, Info, IsUnevaluatedOperand);
 
bool X86AsmParser::ParseIntelInlineAsmIdentifier(const MCExpr *&Val,
 
   // Create the symbol reference.
   MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
   MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
   Val = MCSymbolRefExpr::create(Sym, Variant, getParser().getContext());
   return false;
 }
 
-/// \brief Parse intel style segment override.
-std::unique_ptr<X86Operand>
-X86AsmParser::ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start,
-                                        unsigned Size) {
-  MCAsmParser &Parser = getParser();
-  assert(SegReg != 0 && "Tried to parse a segment override without a segment!");
-  const AsmToken &Tok = Parser.getTok(); // Eat colon.
-  if (Tok.isNot(AsmToken::Colon))
-    return ErrorOperand(Tok.getLoc(), "Expected ':' token!");
-  Parser.Lex(); // Eat ':'
-
-  int64_t ImmDisp = 0;
-  if (getLexer().is(AsmToken::Integer)) {
-    ImmDisp = Tok.getIntVal();
-    AsmToken ImmDispToken = Parser.Lex(); // Eat the integer.
-
-    if (isParsingInlineAsm())
-      InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, ImmDispToken.getLoc());
-
-    if (getLexer().isNot(AsmToken::LBrac)) {
-      // An immediate following a 'segment register', 'colon' token sequence can
-      // be followed by a bracketed expression.  If it isn't we know we have our
-      // final segment override.
-      const MCExpr *Disp = MCConstantExpr::create(ImmDisp, getContext());
-      return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp,
-                                   /*BaseReg=*/0, /*IndexReg=*/0, /*Scale=*/1,
-                                   Start, ImmDispToken.getEndLoc(), Size);
-    }
-  }
-
-  if (getLexer().is(AsmToken::LBrac))
-    return ParseIntelBracExpression(SegReg, Start, ImmDisp, false, Size);
-
-  const MCExpr *Val;
-  SMLoc End;
-  if (!isParsingInlineAsm()) {
-    if (getParser().parsePrimaryExpr(Val, End))
-      return ErrorOperand(Tok.getLoc(), "unknown token in expression");
-
-    return X86Operand::CreateMem(getPointerWidth(), Val, Start, End, Size);
-  }
-
-  InlineAsmIdentifierInfo Info;
-  StringRef Identifier = Tok.getString();
-  if (ParseIntelIdentifier(Val, Identifier, Info,
-                           /*Unevaluated=*/false, End))
-    return nullptr;
-  return CreateMemForInlineAsm(/*SegReg=*/0, Val, /*BaseReg=*/0,/*IndexReg=*/0,
-                               /*Scale=*/1, Start, End, Size, Identifier, Info);
-}
-
 //ParseRoundingModeOp - Parse AVX-512 rounding mode operand
 std::unique_ptr<X86Operand>
 X86AsmParser::ParseRoundingModeOp(SMLoc Start, SMLoc End) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   // Eat "{" and mark the current place.
   const SMLoc consumedToken = consumeToken();
   if (Tok.getIdentifier().startswith("r")){
if (!getLexer().is(AsmToken::RCurly))
       return ErrorOperand(Tok.getLoc(), "Expected } at this point");
     Parser.Lex();  // Eat "}"
     return X86Operand::CreateToken("{sae}", consumedToken);
   }
   return ErrorOperand(Tok.getLoc(), "unknown token in expression");
 }
 
 /// Parse the '.' operator.
-bool X86AsmParser::ParseIntelDotOperator(const MCExpr *Disp,
-                                                const MCExpr *&NewDisp) {
-  MCAsmParser &Parser = getParser();
-  const AsmToken &Tok = Parser.getTok();
-  int64_t OrigDispVal, DotDispVal;
-
-  // FIXME: Handle non-constant expressions.
-  if (const MCConstantExpr *OrigDisp = dyn_cast<MCConstantExpr>(Disp))
-    OrigDispVal = OrigDisp->getValue();
-  else
-    return Error(Tok.getLoc(), "Non-constant offsets are not supported!");
+bool X86AsmParser::ParseIntelDotOperator(IntelExprStateMachine &SM, SMLoc &End) {
+  const AsmToken &Tok = getTok();
+  unsigned Offset;
 
   // Drop the optional '.'.
   StringRef DotDispStr = Tok.getString();
   if (DotDispStr.startswith("."))
     DotDispStr = DotDispStr.drop_front(1);
 
   // .Imm gets lexed as a real.
   if (Tok.is(AsmToken::Real)) {
     APInt DotDisp;
     DotDispStr.getAsInteger(10, DotDisp);
-    DotDispVal = DotDisp.getZExtValue();
+    Offset = DotDisp.getZExtValue();
   } else if (isParsingInlineAsm() && Tok.is(AsmToken::Identifier)) {
-    unsigned DotDisp;
     std::pair<StringRef, StringRef> BaseMember = DotDispStr.split('.');
     if (SemaCallback->LookupInlineAsmField(BaseMember.first, BaseMember.second,
-                                           DotDisp))
+                                           Offset))
       return Error(Tok.getLoc(), "Unable to lookup field reference!");
-    DotDispVal = DotDisp;
   } else
     return Error(Tok.getLoc(), "Unexpected token type!");
 
-  if (isParsingInlineAsm() && Tok.is(AsmToken::Identifier)) {
-    SMLoc Loc = SMLoc::getFromPointer(DotDispStr.data());
-    unsigned Len = DotDispStr.size();
-    InstInfo->AsmRewrites->emplace_back(AOK_DotOperator, Loc, Len, DotDispVal);
-  }
-
-  NewDisp = MCConstantExpr::create(OrigDispVal + DotDispVal, getContext());
+  // Eat the DotExpression and update End
+  End = SMLoc::getFromPointer(DotDispStr.data());
+  const char *DotExprEndLoc = DotDispStr.data() + DotDispStr.size();
+  while (Tok.getLoc().getPointer() < DotExprEndLoc)
+    Lex();
+  SM.addImm(Offset);
   return false;
 }
 
 /// Parse the 'offset' operator.  This operator is used to specify the
 /// location rather then the content of a variable.
 std::unique_ptr<X86Operand> X86AsmParser::ParseIntelOffsetOfOperator() {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   SMLoc OffsetOfLoc = Tok.getLoc();
   Parser.Lex(); // Eat offset.
 
   const MCExpr *Val;
   InlineAsmIdentifierInfo Info;
   SMLoc Start = Tok.getLoc(), End;
   StringRef Identifier = Tok.getString();
-  if (ParseIntelIdentifier(Val, Identifier, Info,
+  if (ParseIntelInlineAsmIdentifier(Val, Identifier, Info,
                                     /*Unevaluated=*/false, End))
     return nullptr;
 
   // Don't emit the offset operator.
   InstInfo->AsmRewrites->emplace_back(AOK_Skip, OffsetOfLoc, 7);
 
   // The offset operator will have an 'r' constraint, thus we need to create
   // register operand to ensure proper matching.  Just pick a GPR based on
   // the size of a pointer.
   bool Parse32 = is32BitMode() || Code16GCC;
   unsigned RegNo = is64BitMode() ? X86::RBX : (Parse32 ? X86::EBX : X86::BX);
 
   return X86Operand::CreateReg(RegNo, Start, End, /*GetAddress=*/true,
                                OffsetOfLoc, Identifier, Info.OpDecl);
 }
 
 // Query a candidate string for being an Intel assembly operator
 // Report back its kind, or IOK_INVALID if does not evaluated as a known one
-unsigned X86AsmParser::IdentifyIntelOperator(StringRef Name) {
+unsigned X86AsmParser::IdentifyIntelInlineAsmOperator(StringRef Name) {
   return StringSwitch<unsigned>(Name)
     .Cases("TYPE","type",IOK_TYPE)
     .Cases("SIZE","size",IOK_SIZE)
     .Cases("LENGTH","length",IOK_LENGTH)
     .Cases("OFFSET","offset",IOK_OFFSET)
     .Default(IOK_INVALID);
 }
 
 /// Parse the 'LENGTH', 'TYPE' and 'SIZE' operators.  The LENGTH operator
 /// returns the number of elements in an array.  It returns the value 1 for
 /// non-array variables.  The SIZE operator returns the size of a C or C++
 /// variable.  A variable's size is the product of its LENGTH and TYPE.  The
 /// TYPE operator returns the size of a C or C++ type or variable. If the
 /// variable is an array, TYPE returns the size of a single element.
-unsigned X86AsmParser::ParseIntelOperator(unsigned OpKind) {
+unsigned X86AsmParser::ParseIntelInlineAsmOperator(unsigned OpKind) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
-  SMLoc TypeLoc = Tok.getLoc();
   Parser.Lex(); // Eat operator.
 
   const MCExpr *Val = nullptr;
   InlineAsmIdentifierInfo Info;
   SMLoc Start = Tok.getLoc(), End;
   StringRef Identifier = Tok.getString();
-  if (ParseIntelIdentifier(Val, Identifier, Info,
+  if (ParseIntelInlineAsmIdentifier(Val, Identifier, Info,
                                     /*Unevaluated=*/true, End))
     return 0;
 
   if (!Info.OpDecl) {
     Error(Start, "unable to lookup expression");
     return 0;
   }
-  
   unsigned CVal = 0;
   switch(OpKind) {
   default: llvm_unreachable("Unexpected operand kind!");
   case IOK_LENGTH: CVal = Info.Length; break;
   case IOK_SIZE: CVal = Info.Size; break;
   case IOK_TYPE: CVal = Info.Type; break;
   }
 
-  // Rewrite the type operator and the C or C++ type or variable in terms of an
-  // immediate.  E.g. TYPE foo -> $$4
-  unsigned Len = End.getPointer() - TypeLoc.getPointer();
-  InstInfo->AsmRewrites->emplace_back(AOK_Imm, TypeLoc, Len, CVal);
-
   return CVal;
 }
 
+bool X86AsmParser::ParseIntelSizeDirective(unsigned &Size) {
+  Size = StringSwitch<unsigned>(getTok().getString())
+    .Cases("BYTE", "byte", 8)
+    .Cases("WORD", "word", 16)
+    .Cases("DWORD", "dword", 32)
+    .Cases("FWORD", "fword", 48)
+    .Cases("QWORD", "qword", 64)
+    .Cases("MMWORD","mmword", 64)
+    .Cases("XWORD", "xword", 80)
+    .Cases("TBYTE", "tbyte", 80)
+    .Cases("XMMWORD", "xmmword", 128)
+    .Cases("YMMWORD", "ymmword", 256)
+    .Cases("ZMMWORD", "zmmword", 512)
+    .Cases("OPAQUE", "opaque", -1U) // needs to be non-zero, but doesn't matter
+    .Default(0);
+  if (Size) {
+    const AsmToken &Tok = Lex(); // Eat operand size (e.g., byte, word).
+    if (!(Tok.getString().equals("PTR") || Tok.getString().equals("ptr")))
+      return Error(Tok.getLoc(), "Expected 'PTR' or 'ptr' token!");
+    Lex(); // Eat ptr.
+  }
+  return false;
+}
+
 std::unique_ptr<X86Operand> X86AsmParser::ParseIntelOperand() {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   SMLoc Start, End;
 
   // FIXME: Offset operator
   // Should be handled as part of immediate expression, as other operators
   // Currently, only supported as a stand-alone operand
   if (isParsingInlineAsm())
-    if (IdentifyIntelOperator(Tok.getString()) == IOK_OFFSET)
+    if (IdentifyIntelInlineAsmOperator(Tok.getString()) == IOK_OFFSET)
       return ParseIntelOffsetOfOperator();
 
-  bool PtrInOperand = false;
-  unsigned Size = getIntelMemOperandSize(Tok.getString());
-  if (Size) {
-    Parser.Lex(); // Eat operand size (e.g., byte, word).
-    if (Tok.getString() != "PTR" && Tok.getString() != "ptr")
-      return ErrorOperand(Tok.getLoc(), "Expected 'PTR' or 'ptr' token!");
-    Parser.Lex(); // Eat ptr.
-    PtrInOperand = true;
-  }
+  // Parse optional Size directive.
+  unsigned Size;
+  if (ParseIntelSizeDirective(Size))
+    return nullptr;
+  bool PtrInOperand = bool(Size);
 
   Start = Tok.getLoc();
 
-  // rounding mode token
+  // Rounding mode operand.
   if (getSTI().getFeatureBits()[X86::FeatureAVX512] &&
       getLexer().is(AsmToken::LCurly))
     return ParseRoundingModeOp(Start, End);
 
-  // Register.
+  // Register operand.
   unsigned RegNo = 0;
-  if (getLexer().is(AsmToken::Identifier) &&
-      !ParseRegister(RegNo, Start, End)) {
-    // If this is a segment register followed by a ':', then this is the start
-    // of a segment override, otherwise this is a normal register reference.
-    // In case it is a normal register and there is ptr in the operand this
-    // is an error
+  if (Tok.is(AsmToken::Identifier) && !ParseRegister(RegNo, Start, End)) {
     if (RegNo == X86::RIP)
       return ErrorOperand(Start, "rip can only be used as a base register");
-    if (getLexer().isNot(AsmToken::Colon)) {
-      if (PtrInOperand) {
-        return ErrorOperand(Start, "expected memory operand after "
-                                   "'ptr', found register operand instead");
-      }
-      return X86Operand::CreateReg(RegNo, Start, End);
-    }
-    return ParseIntelSegmentOverride(/*SegReg=*/RegNo, Start, Size);
+    // A Register followed by ':' is considered a segment override
+    if (Tok.isNot(AsmToken::Colon))
+      return !PtrInOperand ? X86Operand::CreateReg(RegNo, Start, End) :
+        ErrorOperand(Start, "expected memory operand after 'ptr', "
+                            "found register operand instead");
+    // An alleged segment override. check if we have a valid segment register
+    if (!X86MCRegisterClasses[X86::SEGMENT_REGRegClassID].contains(RegNo))
+      return ErrorOperand(Start, "invalid segment register");
+    // Eat ':' and update Start location
+    Start = Lex().getLoc();
   }
 
   // Immediates and Memory
-
-  // Parse [ BaseReg + Scale*IndexReg + Disp ].
-  if (getLexer().is(AsmToken::LBrac))
-    return ParseIntelBracExpression(/*SegReg=*/0, Start, /*ImmDisp=*/0, false,
-                                    Size);
-
-  AsmToken StartTok = Tok;
-  IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
-                           /*AddImmPrefix=*/false);
+  IntelExprStateMachine SM;
   if (ParseIntelExpression(SM, End))
     return nullptr;
 
-  bool isSymbol = SM.getSym() && SM.getSym()->getKind() != MCExpr::Constant;
-  int64_t Imm = SM.getImm();
-  if (SM.getSym() && SM.getSym()->getKind() == MCExpr::Constant)
-    SM.getSym()->evaluateAsAbsolute(Imm);
-
-  if (StartTok.isNot(AsmToken::Identifier) &&
-      StartTok.isNot(AsmToken::String) && isParsingInlineAsm()) {
-    unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
-    if (StartTok.getString().size() == Len)
-      // Just add a prefix if this wasn't a complex immediate expression.
-      InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
-    else
-      // Otherwise, rewrite the complex expression as a single immediate.
-      InstInfo->AsmRewrites->emplace_back(AOK_Imm, Start, Len, Imm);
-  }
-
-  if (getLexer().isNot(AsmToken::LBrac)) {
-    // If a directional label (ie. 1f or 2b) was parsed above from
-    // ParseIntelExpression() then SM.getSym() was set to a pointer to
-    // to the MCExpr with the directional local symbol and this is a
-    // memory operand not an immediate operand.
-    if (isSymbol) {
-      if (isParsingInlineAsm())
-        return CreateMemForInlineAsm(/*SegReg=*/0, SM.getSym(), /*BaseReg=*/0,
-                                     /*IndexReg=*/0,
-                                     /*Scale=*/1, Start, End, Size,
-                                     SM.getSymName(), SM.getIdentifierInfo());
-      return X86Operand::CreateMem(getPointerWidth(), SM.getSym(), Start, End,
-                                   Size);
-    }
+    RewriteIntelExpression(SM, Start, Tok.getLoc());
 
-    const MCExpr *ImmExpr = MCConstantExpr::create(Imm, getContext());
-    return X86Operand::CreateImm(ImmExpr, Start, End);
-  }
+  int64_t Imm = SM.getImm();
+  const MCExpr *Disp = SM.getSym();
+  const MCExpr *ImmDisp = MCConstantExpr::create(Imm, getContext());
+  if (Disp && Imm)
+    Disp = MCBinaryExpr::createAdd(Disp, ImmDisp, getContext());
+  if (!Disp)
+    Disp = ImmDisp;
+
+  // RegNo <> 0 specifies a valid segment register,
+  // and we are parsing a segment override
+  if (!SM.isMemExpr() && !RegNo)
+    return X86Operand::CreateImm(Disp, Start, End);
 
-  return ParseIntelBracExpression(/*SegReg=*/0, Start, Imm, isSymbol, Size);
+  StringRef ErrMsg;
+  unsigned BaseReg = SM.getBaseReg();
+  unsigned IndexReg = SM.getIndexReg();
+  unsigned Scale = SM.getScale();
+
+  if ((BaseReg || IndexReg) &&
+      CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, ErrMsg))
+    return ErrorOperand(Start, ErrMsg);
+  if (isParsingInlineAsm())
+    return CreateMemForInlineAsm(RegNo, Disp, BaseReg, IndexReg,
+                                 Scale, Start, End, Size, SM.getSymName(),
+                                 SM.getIdentifierInfo());
+  if (!(BaseReg || IndexReg || RegNo))
+    return X86Operand::CreateMem(getPointerWidth(), Disp, Start, End, Size);
+  return X86Operand::CreateMem(getPointerWidth(), RegNo, Disp,
+                               BaseReg, IndexReg, Scale, Start, End, Size);
 }
 
 std::unique_ptr<X86Operand> X86AsmParser::ParseATTOperand() {
   MCAsmParser &Parser = getParser();
   switch (getLexer().getKind()) {
   default:
     // Parse a memory operand with no segment register.
     return ParseMemOperand(0, Parser.getTok().getLoc());

test/MC/X86/intel-syntax-3.s

-// RUN: not llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s 2> %t.err
-// RUN: FileCheck --check-prefix=CHECK-STDERR < %t.err %s
-
-_test:
-// CHECK-LABEL: _test:
-// CHECK: xorl    %eax, %eax
-
-  xor EAX, EAX
-  ret
-
-.set  number, 8
-.global _foo
-
-.text
-  .global main
-main:
-
-// CHECK-STDERR:  error: unknown token in expression
-  lea RDX, [RAX * number + RBX + _foo]
-
-// CHECK-STDERR:  error: unknown token in expression
-  lea RDX, [_foo + RAX * number + RBX]
-
-// CHECK-STDERR:  error: unknown token in expression
-  lea RDX, [number + RAX * number + RCX]
-
-// CHECK-STDERR:  error: unknown token in expression
-  lea RDX, [_foo + RAX * number]
-
-// CHECK-STDERR:  error: unknown token in expression
-  lea RDX, [_foo + RAX * number + RBX]
-
-// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
-  lea RDX, [number * RAX + RBX + _foo]
-
-// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
-  lea RDX, [_foo + number * RAX + RBX]
-
-// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
-  lea RDX, [8 + number * RAX + RCX]
-
-// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
-lea RDX, [unknown_number * RAX + RBX + _foo]
-
-// CHECK-STDERR: error: BaseReg/IndexReg already set!
-lea RDX, [4 * RAX + 27 * RBX + _pat]

test/MC/X86/intel-syntax-error.s

// CHECK: error: '.att_syntax noprefix' is not supported: registers must have a '%' prefix in .att_syntax
   movl  $257, -4(esp)
 
 
 .intel_syntax noprefix
 
 .global arr
 .global i
 .set FOO, 2
-//CHECK: error: cannot use base register with variable reference
-mov eax, DWORD PTR arr[ebp + 1 + (2 * 5) - 3 + 1<<1]
-//CHECK: error: cannot use index register with variable reference
-mov eax, DWORD PTR arr[esi*4]
+
 //CHECK: error: cannot use more than one symbol in memory operand
 mov eax, DWORD PTR arr[i]
 //CHECK: error: rip can only be used as a base register
 .code64
 mov rax, rip
 //CHECK: error: invalid base+index expression
 mov rbx, [rax+rip]
+//CHECK: error: scale factor in address must be 1, 2, 4 or 8
+lea RDX, [unknown_number * RAX + RBX + _foo]
+//CHECK: error: BaseReg/IndexReg already set!
+lea RDX, [4 * RAX + 27 * RBX + _pat]
+//CHECK: error: unexpected bracket encountered
+lea RDX, [[arr]
+//CHECK: error: unexpected bracket encountered
+lea RDX, [arr[]

test/MC/X86/intel-syntax.s

 .global _foo
 
 .text
   .global main
 main:
 
 // CHECK: leaq    _foo(%rbx,%rax,8), %rdx
   lea RDX, [8 * RAX + RBX      + _foo]
-
 // CHECK: leaq _foo(%rbx,%rax,8), %rdx
   lea RDX, [_foo + 8 * RAX + RBX]
-
 // CHECK: leaq 8(%rcx,%rax,8), %rdx
   lea RDX, [8 + RAX * 8 + RCX]
-
 // CHECK: leaq 8(%rcx,%rax,8), %rdx
   lea RDX, [number + 8 * RAX + RCX]
-
 // CHECK: leaq _foo(,%rax,8), %rdx
   lea RDX, [_foo + RAX * 8]
-
 // CHECK:  leaq _foo(%rbx,%rax,8), %rdx
   lea RDX, [_foo + RAX * 8 + RBX]
-
-// CHECK: leaq 8(%rax), %rdx
+// CHECK: leaq -8(%rax), %rdx
   lea RDX, [RAX - number]
 // CHECK: leaq -8(%rax), %rdx
   lea RDX, [RAX - 8]
-
 // CHECK: leaq    _foo(%rax), %rdx
   lea RDX, [RAX + _foo]
 // CHECK: leaq    8(%rax), %rdx
   lea RDX, [RAX + number]
 // CHECK: leaq    8(%rax), %rdx
   lea RDX, [RAX + 8]
-
+// CHECK: leaq    _foo(%rbx,%rax,8), %rdx
+  lea RDX, [RAX * number + RBX + _foo]
+// CHECK: leaq    _foo(%rbx,%rax,8), %rdx
+  lea RDX, [_foo + RAX * number + RBX]
+// CHECK: leaq    8(%rcx,%rax,8), %rdx
+  lea RDX, [number + RAX * number + RCX]
+// CHECK: leaq    _foo(,%rax,8), %rdx
+  lea RDX, [_foo + RAX * number]
+// CHECK: leaq    _foo(%rbx,%rax,8), %rdx
+  lea RDX, [number * RAX + RBX + _foo]
+// CHECK: leaq    _foo(%rbx,%rax,8), %rdx
+  lea RDX, [_foo + number * RAX + RBX]
+// CHECK: leaq    8(%rcx,%rax,8), %rdx
+  lea RDX, [8 + number * RAX + RCX]
 // CHECK: leaq    _foo(%rax), %rdx
   lea RDX, [_foo + RAX]
 // CHECK: leaq    8(%rax), %rdx
   lea RDX, [number + RAX]
 // CHECK: leaq    8(%rax), %rdx
   lea RDX, [8 + RAX]
 
 // CHECK: lcalll *(%rax)

 fxrstor64 opaque ptr [rax]
 
 .bss
 .globl _g0
 .text
 
 // CHECK: movq _g0, %rbx
 // CHECK: movq _g0+8, %rcx
+// CHECK: movq _g0+18(%rbp), %rax
+// CHECK: movq _g0(,%rsi,4), %rax
 mov rbx, qword ptr [_g0]
 mov rcx, qword ptr [_g0 + 8]
+mov rax, QWORD PTR _g0[rbp + 1 + (2 * 5) - 3 + 1<<1]
+mov rax, QWORD PTR _g0[rsi*4]
 
 "?half@?0??bar@@YAXXZ@4NA":
   .quad   4602678819172646912
 
 fadd   dword ptr "?half@?0??bar@@YAXXZ@4NA"
 fadd   dword ptr "?half@?0??bar@@YAXXZ@4NA"@IMGREL
 // CHECK: fadds   "?half@?0??bar@@YAXXZ@4NA"
 // CHECK: fadds   "?half@?0??bar@@YAXXZ@4NA"@IMGREL