[x86][inline-asm]Extend support for memory reference expression

[coby]

Summary

Extend support for expressions which represent a variable access in ms-style inline-asm, to allow the incorporation of both registers and variables.
Currently, expression such as 'asm mov eax, [var + eax]' would have been reduced to the (equivalent of) 'asm mov eax, [var]'
This patch amends it
clang counterpart: D35775

[rnk]

I would really prefer to see fewer rewrites and more pass through of inline assembly to MC. The way we parse inline asm in two phases is a confusing source of bugs. It also makes it harder to test the Intel asm parser, since the more rewrites we do the more cross-repository tests we have to add.

include/llvm/MC/MCParser/MCTargetAsmParser.h

77

format

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

21–24

I see, these are in fact valid x86. :) Can you move them to a valid test case so we don't lose coverage, like test/MC/X86/intel-syntax.s?

[coby]

I would really prefer to see fewer rewrites and more pass through of inline assembly to MC
As do I. What are you suggesting? Keep rewrites to the absolute necessary minimum?
Pepole do some funky stuff on MS-Inline-Asm, a large portion of it should be 'legalized' in order for the Assembler to be able to digest.
The other way around is to allow this nonsense in the Assembler as well.
Both options are bad, but I believe the latter is a bit more.

If it's of any comfort - have this 'new' rewrite is to be allowed - AOK_Imm/AOK_ImmPrefix/AOK_SizeDirective can be implemented as a special case of it (AOK_DotOperator should be cancelled).
Net effect will remain the same - but it'll be (possibly) less of a mess of code.

[coby]

Addressed @rnk 's (most of) comments

[rnk]

In D35774#819706, @coby wrote:

I would really prefer to see fewer rewrites and more pass through of inline assembly to MC
As do I. What are you suggesting? Keep rewrites to the absolute necessary minimum?
Pepole do some funky stuff on MS-Inline-Asm, a large portion of it should be 'legalized' in order for the Assembler to be able to digest.
The other way around is to allow this nonsense in the Assembler as well.
Both options are bad, but I believe the latter is a bit more.

If it's of any comfort - have this 'new' rewrite is to be allowed - AOK_Imm/AOK_ImmPrefix/AOK_SizeDirective can be implemented as a special case of it (AOK_DotOperator should be cancelled).
Net effect will remain the same - but it'll be (possibly) less of a mess of code.

Hm, rewriting everything in terms of a general AOK_IntelExpr does sound better, even if it is kind of the opposite direction from what I was thinking. :) What I'm imagining is that any time there is a symbolic reference to C/C++ enums, globals, or variables, we basically fully parse that operand and decompose it into the usual parts of an X86 memory operand: segment, base, index, scale, displacement, etc, check that those parts make sense, and emit a completely rewritten operand. Right now the parsing code in MC has a hard time deciding what's legal because it doesn't know whether certain C/C++ variable references refer to stack locals or global variables. A local variable will require using a base register, but a global will not, but on x86_64, a global will require %rip-relative addressing, which can't use base+index registers.

Doing this exposes a lot of x86 details that aren't really supposed to be in the supposedly target neutral asmparser interface. That battle may already be lost, though.

[coby]

In D35774#822227, @rnk wrote:

In D35774#819706, @coby wrote:

I would really prefer to see fewer rewrites and more pass through of inline assembly to MC
As do I. What are you suggesting? Keep rewrites to the absolute necessary minimum?
Pepole do some funky stuff on MS-Inline-Asm, a large portion of it should be 'legalized' in order for the Assembler to be able to digest.
The other way around is to allow this nonsense in the Assembler as well.
Both options are bad, but I believe the latter is a bit more.

If it's of any comfort - have this 'new' rewrite is to be allowed - AOK_Imm/AOK_ImmPrefix/AOK_SizeDirective can be implemented as a special case of it (AOK_DotOperator should be cancelled).
Net effect will remain the same - but it'll be (possibly) less of a mess of code.

Hm, rewriting everything in terms of a general AOK_IntelExpr does sound better, even if it is kind of the opposite direction from what I was thinking. :) What I'm imagining is that any time there is a symbolic reference to C/C++ enums, globals, or variables, we basically fully parse that operand and decompose it into the usual parts of an X86 memory operand: segment, base, index, scale, displacement, etc, check that those parts make sense, and emit a completely rewritten operand. Right now the parsing code in MC has a hard time deciding what's legal because it doesn't know whether certain C/C++ variable references refer to stack locals or global variables. A local variable will require using a base register, but a global will not, but on x86_64, a global will require %rip-relative addressing, which can't use base+index registers.

Doing this exposes a lot of x86 details that aren't really supposed to be in the supposedly target neutral asmparser interface. That battle may already be lost, though.

This battle can fairly been viewed as lost, until a 'global' refactoring/reconsideration will be announced (if ever) regarding the way targets' specific parsers and AsmParser are to integrate one with each other, in terms of responsibilities. those 'leaks' you've mentioned from X86 to AsmParser are a fine exhibition of it. I have some ideas regarding how we may overcome/'hide' such phenomena in a way which should make rewrites unnecessary, but they are currently seem 'too heavy' (and reaching code parts wihch I should get more familiar with ere tempering). perhaps a llvm-dev discussion should be commenced? to either find a more clean and elegant way, or setting 'guidelines' which will confine this witchcraft to an agreed upon, tolerable minimum.
In the meantime - I deem we should 'allow' ourselves to 'rewrite' only where no scale-able alternative can be conceived, and try to concentrate/uniform all rewrites under one hood (in an utopic future - it should ease rewrites elimination)

[coby]

pi|\|g

[coby]

lads?
anyone?

[rnk]

lib/Target/X86/AsmParser/X86AsmParser.cpp

1281

Please keep the curly braces on the for loop. I know elsewhere they are omitted, but this is just too clever for me.

1283

If we have to keep doing comparing source locations, this code would greatly benefit from some helper functions that do things like the following: isLocBetween, isLocAfter, isLocBefore, so you don't have to deal in pointers all the time.

1308–1319

This is too clever. IMO we should take SymName, make an SMLoc out of it, and use the comparison helpers to figure out if the string is before or after '[', and do the appropriate rewrite.

1525

User-facing diagnostics are supposed to be sentence fragments that start with lower case. We also try to avoid contractions in user-facing diagnostics, so this should be "cannot" instead of "Can't".

[coby]

This patch is largely covered by D36793, and so better be conducted afterwards.
will upload updated version once D36793 is approved and commited. sorry for all the hassle.

[coby]

superseded by D37413

include/llvm/MC/MCParser/MCAsmParser.h

 class MCStreamer;
 class MCTargetAsmParser;
 class SourceMgr;
 
 class InlineAsmIdentifierInfo {
 public:
   void *OpDecl;
   bool IsVarDecl;
+  // Answer whether the identifier in question can be seen as a global lvalue
+  bool IsGlobalLV;
   unsigned Length, Size, Type;
 
   void clear() {
     OpDecl = nullptr;
     IsVarDecl = false;
+    IsGlobalLV = false;
     Length = 1;
     Size = 0;
     Type = 0;
   }
 };
 
 /// \brief Generic Sema callback for assembly parser.
 class MCAsmParserSemaCallback {

include/llvm/MC/MCParser/MCTargetAsmParser.h

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_IntelExpr,      // Rewrite a compound expression to a canonical form
+                      // which is an Intel expression, roughly as:
+                      // [ Base + Index * Scale + ImmDisp ]
   AOK_EndOfStatement, // Add EndOfStatement (e.g., "\n\t").
   AOK_Skip            // Skip emission (e.g., offset/type operators).
 };
 
 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
+  1, // AOK_IntelExpr
   5, // AOK_EndOfStatement
   2  // AOK_Skip
 };
 
+struct IntelExprRewrite {
+  StringRef Base;
+  StringRef Index;
+  unsigned Scale;
+  unsigned ImmDisp;
+
+  IntelExprRewrite() : Scale(1), ImmDisp(0) {}
+  IntelExprRewrite(StringRef base, StringRef index, unsigned scale,
+    unsigned immDisp) : Base(base), Index(index), Scale(scale),
+                        ImmDisp(immDisp) {};
+};
+
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   unsigned Val;
   StringRef Label;
+  IntelExprRewrite IntelExpr;
 
 public:
   AsmRewrite(AsmRewriteKind kind, SMLoc loc, unsigned len = 0, unsigned 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) {}
+  // IntelExpr
+  AsmRewrite(SMLoc loc, unsigned len, IntelExprRewrite intelExpr)
+    : Kind(AOK_IntelExpr), Loc(loc), Len(len), IntelExpr(intelExpr) {}
 };
 
 struct ParseInstructionInfo {
   SmallVectorImpl<AsmRewrite> *AsmRewrites = nullptr;
 
   ParseInstructionInfo() = default;
   ParseInstructionInfo(SmallVectorImpl<AsmRewrite> *rewrites)
     : AsmRewrites(rewrites) {}

lib/MC/MCParser/AsmParser.cpp

case AOK_Imm:
       OS << "$$" << AR.Val;
       break;
     case AOK_ImmPrefix:
       OS << "$$";
       break;
     case AOK_Label:
       OS << Ctx.getAsmInfo()->getPrivateLabelPrefix() << AR.Label;
       break;
+    case AOK_IntelExpr:
+      OS << "[";
+      if (!AR.IntelExpr.Base.empty())
+        OS << AR.IntelExpr.Base;
+      if (!AR.IntelExpr.Index.empty()) {
+        if (!AR.IntelExpr.Base.empty())
+          OS << " + ";
+        OS << AR.IntelExpr.Index;
+        if (AR.IntelExpr.Scale > 1)
+          OS << " * $$" << AR.IntelExpr.Scale;
+      }
+      if (AR.IntelExpr.ImmDisp)
+        OS << " + $$" << AR.IntelExpr.ImmDisp;
+      OS << "]";
+      break;
     case AOK_Input:
       OS << '$' << InputIdx++;
       break;
     case AOK_Output:
       OS << '$' << OutputIdx++;
       break;
     case AOK_SizeDirective:
       switch (AR.Val) {

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"
private:
   unsigned ParseIntelOperator(unsigned OpKind);
   std::unique_ptr<X86Operand>
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr<X86Operand> ParseRoundingModeOp(SMLoc Start, SMLoc End);
   bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
   bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
   std::unique_ptr<X86Operand>
   ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
-                           bool isSymbol, unsigned Size);
+                           unsigned Size,
+                           IntelExprStateMachine *SymbolSM = nullptr);
   bool ParseIntelIdentifier(const MCExpr *&Val, StringRef &Identifier,
                             InlineAsmIdentifierInfo &Info,
                             bool IsUnevaluatedOperand, SMLoc &End);
 
   std::unique_ptr<X86Operand> ParseMemOperand(unsigned SegReg, SMLoc StartLoc);
 
   std::unique_ptr<X86Operand>
   CreateMemForInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
std::unique_ptr<X86Operand> X86AsmParser::CreateMemForInlineAsm(
   // calculation purposes.
   unsigned FrontendSize = 0;
   const MCBinaryExpr *BinOp = dyn_cast<MCBinaryExpr>(Disp);
   bool IsSymRef =
       isa<MCSymbolRefExpr>(BinOp ? BinOp->getLHS() : Disp);
   if (IsSymRef && !Size && Info.Type)
     FrontendSize = Info.Type * 8; // Size is in terms of bits in this context.
 
-  // When parsing inline assembly we set the base register to a non-zero value
-  // if we don't know the actual value at this time.  This is necessary to
+  // It is widely common for a MS-style memory reference to use global
+  // variable and one or two registers. In which case - we can't address the
+  // variable via RIP/EIP.
+  // Otherwise, we set the base register to a non-zero value,
+  // if we don't know the actual value at this time. This is necessary to
   // get the matching correct in some cases.
+  if ((BaseReg || IndexReg) && Info.IsGlobalLV) {
+    return X86Operand::CreateMem(getPointerWidth(), Disp, Start, End);
+  } else {
     BaseReg = BaseReg ? BaseReg : 1;
     return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp, BaseReg,
                                  IndexReg, Scale, Start, End, Size, Identifier,
                                  Info.OpDecl, FrontendSize);
   }
+}
 
-static void
+static bool
 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);
-    }
+                           StringRef SymName, int64_t ImmDisp, unsigned BaseReg,
+                           unsigned IndexReg, unsigned Scale, SMLoc Start,
+                           SMLoc &BracLoc, SMLoc &End, bool IsGlobalLV) {
+  if (BaseReg && IndexReg && !IsGlobalLV)
+    return true;
+
+  // Remove any associated immediate rewrites entered by the State Machine,
+  // as we'll emit the accomulated immediate anyway.
+  bool IsRegExpr = BaseReg || IndexReg;
+  for (AsmRewrite &AR : AsmRewrites)
+    if (AR.Kind == AOK_Imm || AR.Kind == AOK_ImmPrefix) {
+      if ((AR.Loc.getPointer() < BracLoc.getPointer()) &&
+          (Start.getPointer() <= AR.Loc.getPointer())) {
+        assert(!Found && "ImmDisp already rewritten");
+        // Either skip or modify an AOK_Imm rewrite which is positioned
+        // before a bracketed expression, depending whether it is a compound
+        // expression (registers + variable), or a simple one (variable)
+        if (IsRegExpr || !ImmDisp)
+          AR.Kind = AOK_Skip;
+        else if (ImmDisp) {
+          AR.Kind = AOK_Imm;
+          AR.Val = ImmDisp;
         }
-  // 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)
+      // Delete all AOK_ImmPrefix rewrites from within a bracketed expression
+      } else if (AR.Loc.getPointer() > BracLoc.getPointer())
         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);
+  // [var + ImmDisp]
+  if (!Found && !IsRegExpr && ImmDisp) {
+    SMLoc Loc = SymName.data() < BracLoc.getPointer() ?
+      SMLoc::getFromPointer(SymName.data()) : BracLoc;
+    AsmRewrites.emplace_back(AOK_Imm, Loc, 0, ImmDisp);
   }
-  // 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.");
+
+  const char *SymLocPtr = SymName.data();
+  int Len = SymLocPtr - BracLoc.getPointer();
+
+  if (Len > 0)
+    // Symbol is within the brackets, skip anything between it and the left brac
+    AsmRewrites.emplace_back(AOK_Skip, BracLoc, Len);
+
+  // Either skip everything after the symbol,
+  // or rewrite it to an AOK_IntelExpr
+  Len = End.getPointer() - (SymLocPtr + SymName.size()) + 1;
+  if (!IsRegExpr) {
     AsmRewrites.emplace_back(AOK_Skip, Loc, Len);
+  } else {
+    StringRef Base = BaseReg ?
+      X86IntelInstPrinter::getRegisterName(BaseReg) : "";
+    StringRef Index = IndexReg ?
+      X86IntelInstPrinter::getRegisterName(IndexReg) : "";
+    IntelExprRewrite IntelExpr(Base, Index, Scale, ImmDisp);
+    AsmRewrites.emplace_back(Loc, Len, IntelExpr);
   }
+  return false;
 }
 
 // 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()))
while (!Done) {
 
     PrevTK = TK;
   }
   return false;
 }
 
 std::unique_ptr<X86Operand>
 X86AsmParser::ParseIntelBracExpression(unsigned SegReg, SMLoc Start,
-                                       int64_t ImmDisp, bool isSymbol,
-                                       unsigned Size) {
+                                       int64_t ImmDisp, unsigned Size,
+                                       IntelExprStateMachine *SymbolSM) {
   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
+  // 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);
+  bool IsSymbol = SM.getSym() && SM.getSym()->getKind() != MCExpr::Constant;
+  if (SymbolSM && IsSymbol) {
+    Error(Start, "cannot use more than one symbol in memory operand");
+    return nullptr;
   }
 
-  if (SM.getImm() || !Disp) {
-    const MCExpr *Imm = MCConstantExpr::create(SM.getImm(), getContext());
+  int64_t Imm = 0;
+  if (SM.getSym() && !IsSymbol)
+    SM.getSym()->evaluateAsAbsolute(Imm);
+  Imm += SM.getImm();
+
+  IntelExprStateMachine *SymSM = IsSymbol ? &SM : SymbolSM;
+  const MCExpr *Disp = SymSM ? SymSM->getSym() : nullptr;
+  if (Disp && isParsingInlineAsm())
+    if (RewriteIntelBracExpression(*InstInfo->AsmRewrites, SymSM->getSymName(),
+                                   Imm, SM.getBaseReg(), SM.getIndexReg(),
+                                   SM.getScale(), Start, BracLoc, End,
+                                   SymSM->getIdentifierInfo().IsGlobalLV))
+      return ErrorOperand(BracLoc, "Can't use a local variable with both "
+                         "base and index registers");
+
+  if (Imm || !Disp) {
+    const MCExpr *ImmDisp = MCConstantExpr::create(Imm, getContext());
     if (Disp)
-      Disp = MCBinaryExpr::createAdd(Disp, Imm, getContext());
+      Disp = MCBinaryExpr::createAdd(Disp, ImmDisp, getContext());
     else
-      Disp = Imm;  // An immediate displacement only.
+      Disp = ImmDisp;  // 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 (CheckBaseRegAndIndexReg(BaseReg, IndexReg, ErrMsg)) {
       Error(StartInBrac, ErrMsg);
       return nullptr;
     }
     return X86Operand::CreateMem(getPointerWidth(), SegReg, Disp, BaseReg,
                                  IndexReg, Scale, Start, End, Size);
   }
 
-  InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+  if (SymSM)
     return CreateMemForInlineAsm(SegReg, Disp, BaseReg, IndexReg, Scale, Start,
-                               End, Size, SM.getSymName(), Info,
-                               isParsingInlineAsm());
+                                 End, Size, SymSM->getSymName(),
+                                 SymSM->getIdentifierInfo());
+
+  InlineAsmIdentifierInfo Info;
+  Info.clear();
+  return CreateMemForInlineAsm(SegReg, Disp, BaseReg, IndexReg, Scale, Start,
+                               End, Size, StringRef(), Info);
 }
 
 // Inline assembly may use variable names with namespace alias qualifiers.
 bool X86AsmParser::ParseIntelIdentifier(const MCExpr *&Val,
                                         StringRef &Identifier,
                                         InlineAsmIdentifierInfo &Info,
                                         bool IsUnevaluatedOperand, SMLoc &End) {
   MCAsmParser &Parser = getParser();
if (getLexer().isNot(AsmToken::LBrac)) {
       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);
+    return ParseIntelBracExpression(SegReg, Start, ImmDisp, 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);
if (getLexer().is(AsmToken::Identifier) &&
     }
     return ParseIntelSegmentOverride(/*SegReg=*/RegNo, Start, Size);
   }
 
   // Immediates and Memory
 
   // Parse [ BaseReg + Scale*IndexReg + Disp ].
   if (getLexer().is(AsmToken::LBrac))
-    return ParseIntelBracExpression(/*SegReg=*/0, Start, /*ImmDisp=*/0, false,
-                                    Size);
+    return ParseIntelBracExpression(/*SegReg=*/0, Start, /*ImmDisp=*/0, Size);
 
   AsmToken StartTok = Tok;
   IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
                            /*AddImmPrefix=*/false);
   if (ParseIntelExpression(SM, End))
     return nullptr;
 
   bool isSymbol = SM.getSym() && SM.getSym()->getKind() != MCExpr::Constant;
if (isSymbol) {
                                    Size);
     }
 
     const MCExpr *ImmExpr = MCConstantExpr::create(Imm, getContext());
     return X86Operand::CreateImm(ImmExpr, Start, End);
   }
 
   // Only positive immediates are valid.
-  if (Imm < 0)
+  if (Imm < 0 )
     return ErrorOperand(Start, "expected a positive immediate displacement "
                                "before bracketed expr.");
 
-  return ParseIntelBracExpression(/*SegReg=*/0, Start, Imm, isSymbol, Size);
+  IntelExprStateMachine *SymbolSM = isSymbol ? &SM : nullptr;
+  return ParseIntelBracExpression(/*SegReg=*/0, Start, Imm, Size, SymbolSM);
 }
 
 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-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]