[x86/MIR] Implement support for pre- and post-instruction symbols, as well as MIR parsing support for `MCSymbol` `MachineOperand`s.

[chandlerc]

Summary

The only real way to test pre- and post-instruction symbol support is to
use them in operands, so I ended up implementing that within the patch
as well. I can split out the operand support if folks really want but it
doesn't really seem worth it.

The functional implementation of pre- and post-instruction symbols is
now *completely trivial*. Two tiny bits of code in the (misnamed)
AsmPrinter. It should be completely target independent as well. We emit
these exactly the same way as we emit basic block labels. Most of the
code here is to give full dumping, MIR printing, and MIR parsing support
so that we can write useful tests.

The MIR parsing of MC symbol operands still isn't 100%, as it forces the
symbols to be non-temporary and non-local symbols with names. However,
those names often can encode most (if not all) of the special semantics
desired, and unnamed symbols seem especially annoying to serialize and
de-serialize. While this isn't perfect or full support, it seems plenty
to write tests that exercise usage of these kinds of operands.

The MIR support for pre-and post-instruction symbols was quite
straightforward. I chose to print them out in an as-if-operand syntax
similar to debug locations as this seemed the cleanest way and let me
use nice introducer tokens rather than inventing more magic punctuation
like we use for memoperands.

However, supporting MIR-based parsing of these symbols caused me to
change the design of the symbol support to allow setting arbitrary
symbols. Without this, I don't see any reasonable way to test things
with MIR.

Depends on revision: D50701

[chandlerc]

Update this to be the complete implementation of pre- and post-instruction
symbol emission.

It turned out that implementing the actual emission was 4 lines and is fully
target independent! Win!

I've also included a test that specifically emits this stuff and checks that it
works on x86. Happy for other target maintainers to get tests for their target,
but given the complexity of crafting a MIR test, I'd like to defer that to
folks familiar w/ those backends and motivated to check support here.
Especially given how trivial the actual implementation is here.

I've also improved the MIR parsing test case to double check that we parse back
the *operands* as well as the pre- and post- symbols. I had verified it myself,
but seems important to actually check.

[rnk]

llvm/include/llvm/CodeGen/MachineInstr.h

1501–1502 ↗

(On Diff #160981)

I still think getOrCreate is the right API for CodeGen. Should this be made private between MachineInstr and MIParser? The only way I can imagine to do that is to introduce some public MIParserHelper class which we forward declare and then define only in the MIParser.cpp file.

llvm/lib/CodeGen/MIRParser/MIParser.cpp

763 ↗

(On Diff #160981)

I think it would be more stylistically consistent for this to be a method of MIParser. Something just like parseMachineOperandAndTiedFlags above. I'd also use out parameters to match the error handling style.

llvm/lib/CodeGen/MachineInstr.cpp

507 ↗

(On Diff #160981)

That should be fairly easy if every MachineInstr owns its own ExtraInfo. Also, I don't think we're saving much memory from using TrailingObjects for the optional pre and post labels. We might as well make those regular nullable pointer members of ExtraInfo for simplicity. Then, mutating them is pretty straightforward.

[chandlerc]

Switch to a more conventional code pattern for the parsing based on code review.

llvm/include/llvm/CodeGen/MachineInstr.h

1501–1502 ↗

(On Diff #160981)

I think it is useful for the caller to control the way the symbol is created.

For example, it might be specifically desirable for these to be external symbols for some passes.

The caller will still have to find all references to the current symbol and update them if it wants to change the symbol here, but it doesn't seem like the API should block that from happening just because it is hard?

Alternatively, I can make these be a *list* of symbols and emit all of them. That would make it even more usable for supporting different use cases for the symbols. Thoughts?

llvm/lib/CodeGen/MIRParser/MIParser.cpp

763 ↗

(On Diff #160981)

Gah. Fine.

I really, really dislike this style which is why I didn't try to emulate it, but I also think you're right.

llvm/lib/CodeGen/MachineInstr.cpp

507 ↗

(On Diff #160981)

Folks seemed very concerned about the memory usage of ExtraInfo so I'm happy to keep it optimized for now... I can make them mutable though (or we can allow appending if that helps based on my other comment above). Probably a follow-up patch though?

[rnk]

lgtm

llvm/include/llvm/CodeGen/MachineInstr.h

1501–1502 ↗

(On Diff #160981)

I think the external symbol use case is kind of interesting. Fair enough.

llvm/lib/CodeGen/MIRParser/MIParser.cpp

763 ↗

(On Diff #160981)

Thanks!

llvm/lib/CodeGen/MachineInstr.cpp

507 ↗

(On Diff #160981)

Yeah, a follow-up is fine.

[chandlerc]

Thanks for the review!

Landing now, but also happy to keep poking this in follow-ups until everyone is happy.

[thegameg]

I fixed a couple of issues in https://reviews.llvm.org/rL340034. Let me know if that doesn't work for you.

llvm/trunk/docs/MIRLangRef.rst

 can serialize:
 
 - The target-specific state in the target-specific ``MachineFunctionInfo``
   subclasses isn't serialized at the moment.
 
 - The target-specific ``MachineConstantPoolValue`` subclasses (in the ARM and
   SystemZ backends) aren't serialized at the moment.
 
-- The ``MCSymbol`` machine operands are only printed, they can't be parsed.
+- The ``MCSymbol`` machine operands don't support temporary or local symbols.
 
 - A lot of the state in ``MachineModuleInfo`` isn't serialized - only the CFI
   instructions and the variable debug information from MMI is serialized right
   now.
 
 These limitations impose restrictions on what you can test with the MIR format.
 For now, tests that would like to test some behaviour that depends on the state
-of certain ``MCSymbol``  operands or the exception handling state in MMI, can't
-use the MIR format. As well as that, tests that test some behaviour that
-depends on the state of the target specific ``MachineFunctionInfo`` or
+of temporary or local ``MCSymbol``  operands or the exception handling state in
+MMI, can't use the MIR format. As well as that, tests that test some behaviour
+that depends on the state of the target specific ``MachineFunctionInfo`` or
 ``MachineConstantPoolValue`` subclasses can't use the MIR format at the moment.
 
 High Level Structure
 ====================
 
 .. _embedded-module:
 
 Embedded Module

 represented using a '``$noreg``' named register, although the former syntax
 is preferred.
 
 .. _machine-operands:
 
 Machine Operands
 ----------------
 
-There are seventeen different kinds of machine operands, and all of them, except
-the ``MCSymbol`` operand, can be serialized. The ``MCSymbol`` operands are
-just printed out - they can't be parsed back yet.
+There are seventeen different kinds of machine operands, and all of them can be
+serialized.
 
 Immediate Operands
 ^^^^^^^^^^^^^^^^^^
 
 The immediate machine operands are untyped, 64-bit signed integers. The
 example below shows an instance of the X86 ``MOV32ri`` instruction that has an
 immediate machine operand ``-42``:
 

llvm/trunk/include/llvm/CodeGen/MachineInstr.h

   ///
   /// Note that `*this` may be one of the incoming MIs!
   ///
   /// Prefer this API whenever possible as it can avoid allocations in common
   /// cases.
   void cloneMergedMemRefs(MachineFunction &MF,
                           ArrayRef<const MachineInstr *> MIs);
 
-  /// Get or create a temporary symbol that will be emitted just prior to the
-  /// instruction itself.
+  /// Set a symbol that will be emitted just prior to the instruction itself.
+  ///
+  /// Setting this to a null pointer will remove any such symbol.
   ///
   /// FIXME: This is not fully implemented yet.
-  MCSymbol *getOrCreatePreInstrTempSymbol(MCContext &MCCtx);
+  void setPreInstrSymbol(MachineFunction &MF, MCSymbol *Symbol);
 
-  /// Get or create a temporary symbol that will be emitted just after the
-  /// instruction itself.
+  /// Set a symbol that will be emitted just after the instruction itself.
+  ///
+  /// Setting this to a null pointer will remove any such symbol.
   ///
   /// FIXME: This is not fully implemented yet.
-  MCSymbol *getOrCreatePostInstrTempSymbol(MCContext &MCCtx);
+  void setPostInstrSymbol(MachineFunction &MF, MCSymbol *Symbol);
 
   /// Return the MIFlags which represent both MachineInstrs. This
   /// should be used when merging two MachineInstrs into one. This routine does
   /// not modify the MIFlags of this MachineInstr.
   uint16_t mergeFlagsWith(const MachineInstr& Other) const;
 
   /// Break any tie involving OpIdx.
   void untieRegOperand(unsigned OpIdx) {

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (auto &MBB : *MF) {
     for (auto &MI : MBB) {
       // Print the assembly for the instruction.
       if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() &&
           !MI.isDebugInstr()) {
         HasAnyRealCode = true;
         ++NumInstsInFunction;
       }
 
+      // If there is a pre-instruction symbol, emit a label for it here.
+      if (MCSymbol *S = MI.getPreInstrSymbol())
+        OutStreamer->EmitLabel(S);
+
       if (ShouldPrintDebugScopes) {
         for (const HandlerInfo &HI : Handlers) {
           NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
                              HI.TimerGroupName, HI.TimerGroupDescription,
                              TimePassesIsEnabled);
           HI.Handler->beginInstruction(&MI);
         }
       }
for (auto &MI : MBB) {
       case TargetOpcode::KILL:
         if (isVerbose()) emitKill(&MI, *this);
         break;
       default:
         EmitInstruction(&MI);
         break;
       }
 
+      // If there is a post-instruction symbol, emit a label for it here.
+      if (MCSymbol *S = MI.getPostInstrSymbol())
+        OutStreamer->EmitLabel(S);
+
       if (ShouldPrintDebugScopes) {
         for (const HandlerInfo &HI : Handlers) {
           NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
                              HI.TimerGroupName, HI.TimerGroupDescription,
                              TimePassesIsEnabled);
           HI.Handler->endInstruction();
         }
       }

llvm/trunk/lib/CodeGen/MIRParser/MILexer.h

enum TokenKind {
     kw_call_entry,
     kw_liveout,
     kw_address_taken,
     kw_landing_pad,
     kw_liveins,
     kw_successors,
     kw_floatpred,
     kw_intpred,
+    kw_pre_instr_symbol,
+    kw_post_instr_symbol,
 
     // Named metadata keywords
     md_tbaa,
     md_alias_scope,
     md_noalias,
     md_range,
     md_diexpr,
 
     // Identifier tokens
     Identifier,
     NamedRegister,
     NamedVirtualRegister,
     MachineBasicBlockLabel,
     MachineBasicBlock,
     StackObject,
     FixedStackObject,
     NamedGlobalValue,
     GlobalValue,
     ExternalSymbol,
+    MCSymbol,
 
     // Other tokens
     IntegerLiteral,
     FloatingPointLiteral,
     HexLiteral,
     VirtualRegister,
     ConstantPoolItem,
     JumpTableIndex,

llvm/trunk/lib/CodeGen/MIRParser/MILexer.cpp

return StringSwitch<MIToken::TokenKind>(Identifier)
       .Case("call-entry", MIToken::kw_call_entry)
       .Case("liveout", MIToken::kw_liveout)
       .Case("address-taken", MIToken::kw_address_taken)
       .Case("landing-pad", MIToken::kw_landing_pad)
       .Case("liveins", MIToken::kw_liveins)
       .Case("successors", MIToken::kw_successors)
       .Case("floatpred", MIToken::kw_floatpred)
       .Case("intpred", MIToken::kw_intpred)
+      .Case("pre-instr-symbol", MIToken::kw_pre_instr_symbol)
+      .Case("post-instr-symbol", MIToken::kw_post_instr_symbol)
       .Default(MIToken::Identifier);
 }
 
 static Cursor maybeLexIdentifier(Cursor C, MIToken &Token) {
   if (!isalpha(C.peek()) && C.peek() != '_')
     return None;
   auto Range = C;
   while (isIdentifierChar(C.peek()))

 static Cursor maybeLexExternalSymbol(Cursor C, MIToken &Token,
                                      ErrorCallbackType ErrorCallback) {
   if (C.peek() != '&')
     return None;
   return lexName(C, Token, MIToken::ExternalSymbol, /*PrefixLength=*/1,
                  ErrorCallback);
 }
 
+static Cursor maybeLexMCSymbol(Cursor C, MIToken &Token,
+                               ErrorCallbackType ErrorCallback) {
+  const StringRef Rule = "<mcsymbol ";
+  if (!C.remaining().startswith(Rule))
+    return None;
+  auto Start = C;
+  C.advance(Rule.size());
+
+  // Try a simple unquoted name.
+  if (C.peek() != '"') {
+    while (isIdentifierChar(C.peek()))
+      C.advance();
+    StringRef String = Start.upto(C).drop_front(Rule.size());
+    if (C.peek() != '>') {
+      ErrorCallback(C.location(),
+                    "expected the '<mcsymbol ...' to be closed by a '>'");
+      Token.reset(MIToken::Error, Start.remaining());
+      return Start;
+    }
+    C.advance();
+
+    Token.reset(MIToken::MCSymbol, Start.upto(C)).setStringValue(String);
+    return C;
+  }
+
+  // Otherwise lex out a quoted name.
+  Cursor R = lexStringConstant(C, ErrorCallback);
+  if (!R) {
+    ErrorCallback(C.location(),
+                  "unable to parse quoted string from opening quote");
+    Token.reset(MIToken::Error, Start.remaining());
+    return Start;
+  }
+  StringRef String = Start.upto(R).drop_front(Rule.size());
+  if (R.peek() != '>') {
+    ErrorCallback(R.location(),
+                  "expected the '<mcsymbol ...' to be closed by a '>'");
+    Token.reset(MIToken::Error, Start.remaining());
+    return Start;
+  }
+  R.advance();
+
+  Token.reset(MIToken::MCSymbol, Start.upto(R))
+      .setOwnedStringValue(unescapeQuotedString(String));
+  return R;
+}
+
 static bool isValidHexFloatingPointPrefix(char C) {
   return C == 'H' || C == 'K' || C == 'L' || C == 'M';
 }
 
 static Cursor lexFloatingPointLiteral(Cursor Range, Cursor C, MIToken &Token) {
   C.advance();
   // Skip over [0-9]*([eE][-+]?[0-9]+)?
   while (isdigit(C.peek()))
StringRef llvm::lexMIToken(StringRef Source, MIToken &Token,
   if (Cursor R = maybeLexIRValue(C, Token, ErrorCallback))
     return R.remaining();
   if (Cursor R = maybeLexRegister(C, Token, ErrorCallback))
     return R.remaining();
   if (Cursor R = maybeLexGlobalValue(C, Token, ErrorCallback))
     return R.remaining();
   if (Cursor R = maybeLexExternalSymbol(C, Token, ErrorCallback))
     return R.remaining();
+  if (Cursor R = maybeLexMCSymbol(C, Token, ErrorCallback))
+    return R.remaining();
   if (Cursor R = maybeLexHexadecimalLiteral(C, Token))
     return R.remaining();
   if (Cursor R = maybeLexNumericalLiteral(C, Token))
     return R.remaining();
   if (Cursor R = maybeLexExlaim(C, Token, ErrorCallback))
     return R.remaining();
   if (Cursor R = maybeLexSymbol(C, Token))
     return R.remaining();

llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp

 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSlotTracker.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/MC/LaneBitmask.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/LowLevelTypeImpl.h"
public:
   bool parseFixedStackFrameIndex(int &FI);
   bool parseFixedStackObjectOperand(MachineOperand &Dest);
   bool parseGlobalValue(GlobalValue *&GV);
   bool parseGlobalAddressOperand(MachineOperand &Dest);
   bool parseConstantPoolIndexOperand(MachineOperand &Dest);
   bool parseSubRegisterIndexOperand(MachineOperand &Dest);
   bool parseJumpTableIndexOperand(MachineOperand &Dest);
   bool parseExternalSymbolOperand(MachineOperand &Dest);
+  bool parseMCSymbolOperand(MachineOperand &Dest);
   bool parseMDNode(MDNode *&Node);
   bool parseDIExpression(MDNode *&Expr);
   bool parseMetadataOperand(MachineOperand &Dest);
   bool parseCFIOffset(int &Offset);
   bool parseCFIRegister(unsigned &Reg);
   bool parseCFIEscapeValues(std::string& Values);
   bool parseCFIOperand(MachineOperand &Dest);
   bool parseIRBlock(BasicBlock *&BB, const Function &F);
public:
   bool parseOperandsOffset(MachineOperand &Op);
   bool parseIRValue(const Value *&V);
   bool parseMemoryOperandFlag(MachineMemOperand::Flags &Flags);
   bool parseMemoryPseudoSourceValue(const PseudoSourceValue *&PSV);
   bool parseMachinePointerInfo(MachinePointerInfo &Dest);
   bool parseOptionalScope(LLVMContext &Context, SyncScope::ID &SSID);
   bool parseOptionalAtomicOrdering(AtomicOrdering &Order);
   bool parseMachineMemoryOperand(MachineMemOperand *&Dest);
+  bool parsePreOrPostInstrSymbol(MCSymbol *&Symbol);
 
 private:
   /// Convert the integer literal in the current token into an unsigned integer.
   ///
   /// Return true if an error occurred.
   bool getUnsigned(unsigned &Result);
 
   /// Convert the integer literal in the current token into an uint64.
private:
   void initNames2MMOTargetFlags();
 
   /// Try to convert a name of a MachineMemOperand target flag to the
   /// corresponding target flag.
   ///
   /// Return true if the name isn't a name of a target MMO flag.
   bool getMMOTargetFlag(StringRef Name, MachineMemOperand::Flags &Flag);
 
+  /// Get or create an MCSymbol for a given name.
+  MCSymbol *getOrCreateMCSymbol(StringRef Name);
+
   /// parseStringConstant
   ///   ::= StringConstant
   bool parseStringConstant(std::string &Result);
 };
 
 } // end anonymous namespace
 
 MIParser::MIParser(PerFunctionMIParsingState &PFS, SMDiagnostic &Error,
bool MIParser::parse(MachineInstr *&MI) {
   if (!Operands.empty() && expectAndConsume(MIToken::equal))
     return true;
 
   unsigned OpCode, Flags = 0;
   if (Token.isError() || parseInstruction(OpCode, Flags))
     return true;
 
   // Parse the remaining machine operands.
-  while (!Token.isNewlineOrEOF() && Token.isNot(MIToken::kw_debug_location) &&
+  while (!Token.isNewlineOrEOF() && Token.isNot(MIToken::kw_pre_instr_symbol) &&
+         Token.isNot(MIToken::kw_post_instr_symbol) &&
+         Token.isNot(MIToken::kw_debug_location) &&
          Token.isNot(MIToken::coloncolon) && Token.isNot(MIToken::lbrace)) {
     auto Loc = Token.location();
     Optional<unsigned> TiedDefIdx;
     if (parseMachineOperandAndTargetFlags(MO, TiedDefIdx))
       return true;
     Operands.push_back(
         ParsedMachineOperand(MO, Loc, Token.location(), TiedDefIdx));
     if (Token.isNewlineOrEOF() || Token.is(MIToken::coloncolon) ||
         Token.is(MIToken::lbrace))
       break;
     if (Token.isNot(MIToken::comma))
       return error("expected ',' before the next machine operand");
     lex();
   }
 
+  MCSymbol *PreInstrSymbol = nullptr;
+  if (Token.is(MIToken::kw_pre_instr_symbol))
+    if (parsePreOrPostInstrSymbol(PreInstrSymbol))
+      return true;
+  MCSymbol *PostInstrSymbol = nullptr;
+  if (Token.is(MIToken::kw_post_instr_symbol))
+    if (parsePreOrPostInstrSymbol(PostInstrSymbol))
+      return true;
+
   DebugLoc DebugLocation;
   if (Token.is(MIToken::kw_debug_location)) {
     lex();
     if (Token.isNot(MIToken::exclaim))
       return error("expected a metadata node after 'debug-location'");
     MDNode *Node = nullptr;
     if (parseMDNode(Node))
       return true;
bool MIParser::parse(MachineInstr *&MI) {
 
   // TODO: Check for extraneous machine operands.
   MI = MF.CreateMachineInstr(MCID, DebugLocation, /*NoImplicit=*/true);
   MI->setFlags(Flags);
   for (const auto &Operand : Operands)
     MI->addOperand(MF, Operand.Operand);
   if (assignRegisterTies(*MI, Operands))
     return true;
-  if (MemOperands.empty())
-    return false;
+  if (PreInstrSymbol)
+    MI->setPreInstrSymbol(MF, PreInstrSymbol);
+  if (PostInstrSymbol)
+    MI->setPostInstrSymbol(MF, PostInstrSymbol);
+  if (!MemOperands.empty())
     MI->setMemRefs(MF, MemOperands);
   return false;
 }
 
 bool MIParser::parseStandaloneMBB(MachineBasicBlock *&MBB) {
   lex();
   if (Token.isNot(MIToken::MachineBasicBlock))
     return error("expected a machine basic block reference");
   if (parseMBBReference(MBB))
bool MIParser::parseExternalSymbolOperand(MachineOperand &Dest) {
   const char *Symbol = MF.createExternalSymbolName(Token.stringValue());
   lex();
   Dest = MachineOperand::CreateES(Symbol);
   if (parseOperandsOffset(Dest))
     return true;
   return false;
 }
 
+bool MIParser::parseMCSymbolOperand(MachineOperand &Dest) {
+  assert(Token.is(MIToken::MCSymbol));
+  MCSymbol *Symbol = getOrCreateMCSymbol(Token.stringValue());
+  lex();
+  Dest = MachineOperand::CreateMCSymbol(Symbol);
+  if (parseOperandsOffset(Dest))
+    return true;
+  return false;
+}
+
 bool MIParser::parseSubRegisterIndexOperand(MachineOperand &Dest) {
   assert(Token.is(MIToken::SubRegisterIndex));
   StringRef Name = Token.stringValue();
   unsigned SubRegIndex = getSubRegIndex(Token.stringValue());
   if (SubRegIndex == 0)
     return error(Twine("unknown subregister index '") + Name + "'");
   lex();
   Dest = MachineOperand::CreateImm(SubRegIndex);
bool MIParser::parseMachineOperand(MachineOperand &Dest,
   case MIToken::NamedGlobalValue:
     return parseGlobalAddressOperand(Dest);
   case MIToken::ConstantPoolItem:
     return parseConstantPoolIndexOperand(Dest);
   case MIToken::JumpTableIndex:
     return parseJumpTableIndexOperand(Dest);
   case MIToken::ExternalSymbol:
     return parseExternalSymbolOperand(Dest);
+  case MIToken::MCSymbol:
+    return parseMCSymbolOperand(Dest);
   case MIToken::SubRegisterIndex:
     return parseSubRegisterIndexOperand(Dest);
   case MIToken::md_diexpr:
   case MIToken::exclaim:
     return parseMetadataOperand(Dest);
   case MIToken::kw_cfi_same_value:
   case MIToken::kw_cfi_offset:
   case MIToken::kw_cfi_rel_offset:
bool MIParser::parseMachineMemoryOperand(MachineMemOperand *&Dest) {
   }
   if (expectAndConsume(MIToken::rparen))
     return true;
   Dest = MF.getMachineMemOperand(Ptr, Flags, Size, BaseAlignment, AAInfo, Range,
                                  SSID, Order, FailureOrder);
   return false;
 }
 
+bool MIParser::parsePreOrPostInstrSymbol(MCSymbol *&Symbol) {
+  assert((Token.is(MIToken::kw_pre_instr_symbol) ||
+          Token.is(MIToken::kw_post_instr_symbol)) &&
+         "Invalid token for a pre- post-instruction symbol!");
+  lex();
+  if (Token.isNot(MIToken::MCSymbol))
+    return error("expected a symbol after 'pre-instr-symbol'");
+  Symbol = getOrCreateMCSymbol(Token.stringValue());
+  lex();
+  if (Token.isNewlineOrEOF() || Token.is(MIToken::coloncolon) ||
+      Token.is(MIToken::lbrace))
+    return false;
+  if (Token.isNot(MIToken::comma))
+    return error("expected ',' before the next machine operand");
+  lex();
+  return false;
+}
+
 void MIParser::initNames2InstrOpCodes() {
   if (!Names2InstrOpCodes.empty())
     return;
   const auto *TII = MF.getSubtarget().getInstrInfo();
   assert(TII && "Expected target instruction info");
   for (unsigned I = 0, E = TII->getNumOpcodes(); I < E; ++I)
     Names2InstrOpCodes.insert(std::make_pair(StringRef(TII->getName(I)), I));
 }
bool MIParser::getMMOTargetFlag(StringRef Name,
   initNames2MMOTargetFlags();
   auto FlagInfo = Names2MMOTargetFlags.find(Name);
   if (FlagInfo == Names2MMOTargetFlags.end())
     return true;
   Flag = FlagInfo->second;
   return false;
 }
 
+MCSymbol *MIParser::getOrCreateMCSymbol(StringRef Name) {
+  // FIXME: Currently we can't recognize temporary or local symbols and call all
+  // of the appropriate forms to create them. However, this handles basic cases
+  // well as most of the special aspects are recognized by a prefix on their
+  // name, and the input names should already be unique. For test cases, keeping
+  // the symbol name out of the symbol table isn't terribly important.
+  return MF.getContext().getOrCreateSymbol(Name);
+}
+
 bool MIParser::parseStringConstant(std::string &Result) {
   if (Token.isNot(MIToken::StringConstant))
     return error("expected string constant");
   Result = Token.stringValue();
   lex();
   return false;
 }
 

llvm/trunk/lib/CodeGen/MIRPrinter.cpp

 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSlotTracker.h"
 #include "llvm/IR/Value.h"
 #include "llvm/MC/LaneBitmask.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
void MIPrinter::print(const MachineInstr &MI) {
   for (; I < E; ++I) {
     if (NeedComma)
       OS << ", ";
     print(MI, I, TRI, ShouldPrintRegisterTies,
           MI.getTypeToPrint(I, PrintedTypes, MRI));
     NeedComma = true;
   }
 
+  // Print any optional symbols attached to this instruction as-if they were
+  // operands.
+  if (MCSymbol *PreInstrSymbol = MI.getPreInstrSymbol()) {
+    if (NeedComma)
+      OS << ',';
+    OS << " pre-instr-symbol ";
+    MachineOperand::printSymbol(OS, *PreInstrSymbol);
+    NeedComma = true;
+  }
+  if (MCSymbol *PostInstrSymbol = MI.getPostInstrSymbol()) {
+    if (NeedComma)
+      OS << ',';
+    OS << " post-instr-symbol ";
+    MachineOperand::printSymbol(OS, *PostInstrSymbol);
+    NeedComma = true;
+  }
+
   if (const DebugLoc &DL = MI.getDebugLoc()) {
     if (NeedComma)
       OS << ',';
     OS << " debug-location ";
     DL->printAsOperand(OS, MST);
   }
 
   if (!MI.memoperands_empty()) {

llvm/trunk/lib/CodeGen/MachineInstr.cpp

for (const MachineInstr &MI : make_pointee_range(MIs.slice(1))) {
 
     // Otherwise accumulate these into our temporary buffer of the merged state.
     MergedMMOs.append(MI.memoperands_begin(), MI.memoperands_end());
   }
 
   setMemRefs(MF, MergedMMOs);
 }
 
-MCSymbol *MachineInstr::getOrCreatePreInstrTempSymbol(MCContext &MCCtx) {
-  MCSymbol *S = getPreInstrSymbol();
-  if (S)
-    return S;
+void MachineInstr::setPreInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
+  MCSymbol *OldSymbol = getPreInstrSymbol();
+  if (OldSymbol == Symbol)
+    return;
+  if (OldSymbol && !Symbol) {
+    // We're removing a symbol rather than adding one. Try to clean up any
+    // extra info carried around.
+    if (Info.is<EIIK_PreInstrSymbol>()) {
+      Info.clear();
+      return;
+    }
 
-  // Create a new temp symbol.
-  S = MCCtx.createTempSymbol();
+    if (memoperands_empty()) {
+      assert(getPostInstrSymbol() &&
+             "Should never have only a single symbol allocated out-of-line!");
+      Info.set<EIIK_PostInstrSymbol>(getPostInstrSymbol());
+      return;
+    }
 
-  if (!Info) {
+    // Otherwise fallback on the generic update.
+  } else if (!Info || Info.is<EIIK_PreInstrSymbol>()) {
     // If we don't have any other extra info, we can store this inline.
-    Info.set<EIIK_PreInstrSymbol>(S);
-    return S;
+    Info.set<EIIK_PreInstrSymbol>(Symbol);
+    return;
   }
 
-  // Otherwise, allocate a fully set of extra info.
+  // Otherwise, allocate a full new set of extra info.
+  // FIXME: Maybe we should make the symbols in the extra info mutable?
   Info.set<EIIK_OutOfLine>(
-      getMF()->createMIExtraInfo(memoperands(), S, getPostInstrSymbol()));
-
-  return S;
+      MF.createMIExtraInfo(memoperands(), Symbol, getPostInstrSymbol()));
 }
 
-MCSymbol *MachineInstr::getOrCreatePostInstrTempSymbol(MCContext &MCCtx) {
-  MCSymbol *S = getPostInstrSymbol();
-  if (S)
-    return S;
+void MachineInstr::setPostInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
+  MCSymbol *OldSymbol = getPostInstrSymbol();
+  if (OldSymbol == Symbol)
+    return;
+  if (OldSymbol && !Symbol) {
+    // We're removing a symbol rather than adding one. Try to clean up any
+    // extra info carried around.
+    if (Info.is<EIIK_PostInstrSymbol>()) {
+      Info.clear();
+      return;
+    }
 
-  // Create a new temp symbol.
-  S = MCCtx.createTempSymbol();
+    if (memoperands_empty()) {
+      assert(getPreInstrSymbol() &&
+             "Should never have only a single symbol allocated out-of-line!");
+      Info.set<EIIK_PreInstrSymbol>(getPreInstrSymbol());
+      return;
+    }
 
-  if (!Info) {
+    // Otherwise fallback on the generic update.
+  } else if (!Info || Info.is<EIIK_PostInstrSymbol>()) {
     // If we don't have any other extra info, we can store this inline.
-    Info.set<EIIK_PostInstrSymbol>(S);
-    return S;
+    Info.set<EIIK_PostInstrSymbol>(Symbol);
+    return;
   }
 
-  // Otherwise, allocate a fully set of extra info.
+  // Otherwise, allocate a full new set of extra info.
+  // FIXME: Maybe we should make the symbols in the extra info mutable?
   Info.set<EIIK_OutOfLine>(
-      getMF()->createMIExtraInfo(memoperands(), getPreInstrSymbol(), S));
-  return S;
+      MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol));
 }
 
 uint16_t MachineInstr::mergeFlagsWith(const MachineInstr &Other) const {
   // For now, the just return the union of the flags. If the flags get more
   // complicated over time, we might need more logic here.
   return getFlags() | Other.getFlags();
 }
 
if (isDebugValue() && MO.isMetadata()) {
       if (MO.isImm() && isOperandSubregIdx(i))
         MachineOperand::printSubRegIdx(OS, MO.getImm(), TRI);
       else
         MO.print(OS, MST, TypeToPrint, /*PrintDef=*/true, IsStandalone,
                  ShouldPrintRegisterTies, TiedOperandIdx, TRI, IntrinsicInfo);
     }
   }
 
+  // Print any optional symbols attached to this instruction as-if they were
+  // operands.
+  if (MCSymbol *PreInstrSymbol = getPreInstrSymbol()) {
+    if (!FirstOp) {
+      FirstOp = false;
+      OS << ',';
+    }
+    OS << " pre-instr-symbol ";
+    MachineOperand::printSymbol(OS, *PreInstrSymbol);
+  }
+  if (MCSymbol *PostInstrSymbol = getPostInstrSymbol()) {
+    if (!FirstOp) {
+      FirstOp = false;
+      OS << ',';
+    }
+    OS << " post-instr-symbol ";
+    MachineOperand::printSymbol(OS, *PostInstrSymbol);
+  }
+
   if (!SkipDebugLoc) {
     if (const DebugLoc &DL = getDebugLoc()) {
       if (!FirstOp)
         OS << ',';
       OS << " debug-location ";
       DL->printAsOperand(OS, MST);
     }
   }

llvm/trunk/test/CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir

+# RUN: llc -march=x86-64 -run-pass none -o - %s | FileCheck %s
+# This test ensures that the MIR parser parses pre- and post-instruction symbols
+# and MCSymbol operands correctly.
+
+--- |
+  declare void @f(i32 %x) nounwind
+
+  declare void @g(i32 %x) nounwind
+
+  declare void @h(i32 %x) nounwind
+
+  define i64 @test(i32 %x) nounwind {
+  entry:
+    call void @f(i32 %x)
+    call void @g(i32 %x)
+    call void @h(i32 %x), !dbg !9
+    ret i64 undef
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!6, !7}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
+  !1 = !DIFile(filename: "test.ll", directory: "")
+  !2 = !{}
+  !4 = distinct !DISubprogram(name: "test", scope: !5, file: !5, line: 4, type: !2, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+  !5 = !DIFile(filename: "test.c", directory: "")
+  !6 = !{i32 2, !"Dwarf Version", i32 4}
+  !7 = !{i32 2, !"Debug Info Version", i32 3}
+  !9 = !DILocation(line: 1, scope: !4)
+
+...
+---
+name: test
+# CHECK-LABEL: name: test
+tracksRegLiveness: true
+frameInfo:
+  hasCalls: true
+body: |
+  bb.0.entry:
+    liveins: $edi
+
+    %0:gr32 = COPY $edi
+    %1:gr32 = COPY killed %0
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit $edi, pre-instr-symbol <mcsymbol .Lpre_f>, post-instr-symbol <mcsymbol .Lpost_f>
+  ; CHECK: CALL64pcrel32 @f, {{.*}}, pre-instr-symbol <mcsymbol .Lpre_f>, post-instr-symbol <mcsymbol .Lpost_f>
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @g, csr_64, implicit $rsp, implicit $ssp, implicit $edi, pre-instr-symbol <mcsymbol .Lpre_g>
+  ; CHECK: CALL64pcrel32 @g, {{.*}}, pre-instr-symbol <mcsymbol .Lpre_g>
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @h, csr_64, implicit $rsp, implicit $ssp, implicit $edi, post-instr-symbol <mcsymbol .Lpost_h>, debug-location !9
+  ; CHECK: CALL64pcrel32 @h, {{.*}}, post-instr-symbol <mcsymbol .Lpost_h>, debug-location
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+
+    %2:gr64 = MOV64ri32 <mcsymbol .Lpre_f>
+    %3:gr64 = MOV64ri32 <mcsymbol .Lpost_f>
+    %4:gr64 = MOV64ri32 <mcsymbol .Lpre_g>
+    %5:gr64 = MOV64ri32 <mcsymbol .Lpost_h>
+  ; CHECK: %2:gr64 = MOV64ri32 <mcsymbol .Lpre_f>
+  ; CHECK: %3:gr64 = MOV64ri32 <mcsymbol .Lpost_f>
+  ; CHECK: %4:gr64 = MOV64ri32 <mcsymbol .Lpre_g>
+  ; CHECK: %5:gr64 = MOV64ri32 <mcsymbol .Lpost_h>
+
+    %6:gr64 = ADD64rr killed %2, killed %3, implicit-def $eflags
+    %7:gr64 = ADD64rr killed %4, killed %5, implicit-def $eflags
+    %8:gr64 = ADD64rr killed %6, killed %7, implicit-def $eflags
+    $rax = COPY %8
+    RETQ implicit $rax
+
+...

llvm/trunk/test/CodeGen/X86/instr-symbols.mir

+# RUN: llc -march=x86-64 -o - %s | FileCheck %s
+#
+# Test the emission of pre- and post-instruction labels.
+
+--- |
+  declare void @f(i32 %x) nounwind
+
+  declare void @g(i32 %x) nounwind
+
+  declare void @h(i32 %x) nounwind
+
+  define i64 @test(i32 %x) nounwind {
+  entry:
+    call void @f(i32 %x)
+    call void @g(i32 %x)
+    call void @h(i32 %x)
+    ret i64 undef
+  }
+
+...
+---
+name: test
+# CHECK-LABEL: {{^}}test:
+tracksRegLiveness: true
+frameInfo:
+  hasCalls: true
+body: |
+  bb.0.entry:
+    liveins: $edi
+
+    %0:gr32 = COPY $edi
+    %1:gr32 = COPY killed %0
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit $edi, pre-instr-symbol <mcsymbol .Lpre_f>, post-instr-symbol <mcsymbol .Lpost_f>
+  ; CHECK:      .Lpre_f:
+  ; CHECK-NEXT:   callq f
+  ; CHECK-NEXT: .Lpost_f:
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @g, csr_64, implicit $rsp, implicit $ssp, implicit $edi, pre-instr-symbol <mcsymbol .Lpre_g>
+  ; CHECK:      .Lpre_g:
+  ; CHECK-NEXT:   callq g
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+    $edi = COPY %1
+
+    CALL64pcrel32 @h, csr_64, implicit $rsp, implicit $ssp, implicit $edi, post-instr-symbol <mcsymbol .Lpost_h>
+  ; CHECK:        callq h
+  ; CHECK-NEXT: .Lpost_h:
+
+    ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
+
+    %2:gr64 = MOV64ri32 <mcsymbol .Lpre_f>
+    %3:gr64 = MOV64ri32 <mcsymbol .Lpost_f>
+    %4:gr64 = MOV64ri32 <mcsymbol .Lpre_g>
+    %5:gr64 = MOV64ri32 <mcsymbol .Lpost_h>
+  ; CHECK:        movq $.Lpre_f, %{{.*}}
+  ; CHECK-NEXT:   movq $.Lpost_f, %{{.*}}
+  ; CHECK-NEXT:   movq $.Lpre_g, %{{.*}}
+  ; CHECK-NEXT:   movq $.Lpost_h, %{{.*}}
+
+    %6:gr64 = ADD64rr killed %2, killed %3, implicit-def $eflags
+    %7:gr64 = ADD64rr killed %4, killed %5, implicit-def $eflags
+    %8:gr64 = ADD64rr killed %6, killed %7, implicit-def $eflags
+    $rax = COPY %8
+    RETQ implicit $rax
+
+...