Revert "Add an instruction marker field to the ExtraInfo in MachineInstrs."

Reverting commit b85b4e5a6f8579c137fecb59a4d75d7bfb111f79 due to some
buildbot failures/ out of memory errors.

7 files changed, 117 insertions, 114 deletions

diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 513361e1341..73c53d6c4af 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1065,9 +1065,13 @@ void AsmPrinter::EmitFunctionBody() {
         ++NumInstsInFunction;
       }
 
-      // If there is a pre-instruction symbol, emit a label for it here.
+      // If there is a pre-instruction symbol, emit a label for it here. If the
+      // instruction was duplicated and the label has already been emitted,
+      // don't re-emit the same label.
+      // FIXME: Consider strengthening that to an assertion.
       if (MCSymbol *S = MI.getPreInstrSymbol())
-        OutStreamer->EmitLabel(S);
+        if (S->isUndefined())
+          OutStreamer->EmitLabel(S);
 
       if (ShouldPrintDebugScopes) {
         for (const HandlerInfo &HI : Handlers) {
@@ -1120,9 +1124,13 @@ void AsmPrinter::EmitFunctionBody() {
         break;
       }
 
-      // If there is a post-instruction symbol, emit a label for it here.
+      // If there is a post-instruction symbol, emit a label for it here.  If
+      // the instruction was duplicated and the label has already been emitted,
+      // don't re-emit the same label.
+      // FIXME: Consider strengthening that to an assertion.
       if (MCSymbol *S = MI.getPostInstrSymbol())
-        OutStreamer->EmitLabel(S);
+        if (S->isUndefined())
+          OutStreamer->EmitLabel(S);
 
       if (ShouldPrintDebugScopes) {
         for (const HandlerInfo &HI : Handlers) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 62ad356e7f8..c6457f3626d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1100,8 +1100,14 @@ void CodeViewDebug::emitDebugInfoForFunction(const Function *GV,
     }
 
     for (auto HeapAllocSite : FI.HeapAllocSites) {
-      const MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
-      const MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+      MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
+      MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+
+      // The labels might not be defined if the instruction was replaced
+      // somewhere in the codegen pipeline.
+      if (!BeginLabel->isDefined() || !EndLabel->isDefined())
+        continue;
+
       const DIType *DITy = std::get<2>(HeapAllocSite);
       MCSymbol *HeapAllocEnd = beginSymbolRecord(SymbolKind::S_HEAPALLOCSITE);
       OS.AddComment("Call site offset");
@@ -1421,16 +1427,6 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
     DebugLoc FnStartDL = PrologEndLoc.getFnDebugLoc();
     maybeRecordLocation(FnStartDL, MF);
   }
-
-  // Find heap alloc sites and emit labels around them.
-  for (const auto &MBB : *MF) {
-    for (const auto &MI : MBB) {
-      if (MI.getHeapAllocMarker()) {
-        requestLabelBeforeInsn(&MI);
-        requestLabelAfterInsn(&MI);
-      }
-    }
-  }
 }
 
 static bool shouldEmitUdt(const DIType *T) {
@@ -2854,18 +2850,8 @@ void CodeViewDebug::endFunctionImpl(const MachineFunction *MF) {
     return;
   }
 
-  // Find heap alloc sites and add to list.
-  for (const auto &MBB : *MF) {
-    for (const auto &MI : MBB) {
-      if (MDNode *MD = MI.getHeapAllocMarker()) {
-        CurFn->HeapAllocSites.push_back(std::make_tuple(getLabelBeforeInsn(&MI),
-                                                        getLabelAfterInsn(&MI),
-                                                        dyn_cast<DIType>(MD)));
-      }
-    }
-  }
-
   CurFn->Annotations = MF->getCodeViewAnnotations();
+  CurFn->HeapAllocSites = MF->getCodeViewHeapAllocSites();
 
   CurFn->End = Asm->getFunctionEnd();
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index b56b9047e1a..7ffd77926cf 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -148,7 +148,7 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
     SmallVector<LexicalBlock *, 1> ChildBlocks;
 
     std::vector<std::pair<MCSymbol *, MDNode *>> Annotations;
-    std::vector<std::tuple<const MCSymbol *, const MCSymbol *, const DIType *>>
+    std::vector<std::tuple<MCSymbol *, MCSymbol *, const DIType *>>
         HeapAllocSites;
 
     const MCSymbol *Begin = nullptr;
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index d75e77d4613..7d2ee230ca9 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -447,11 +447,12 @@ MachineFunction::getMachineMemOperand(const MachineMemOperand *MMO,
       MMO->getOrdering(), MMO->getFailureOrdering());
 }
 
-MachineInstr::ExtraInfo *MachineFunction::createMIExtraInfo(
-    ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol,
-    MCSymbol *PostInstrSymbol, MDNode *HeapAllocMarker) {
+MachineInstr::ExtraInfo *
+MachineFunction::createMIExtraInfo(ArrayRef<MachineMemOperand *> MMOs,
+                                   MCSymbol *PreInstrSymbol,
+                                   MCSymbol *PostInstrSymbol) {
   return MachineInstr::ExtraInfo::create(Allocator, MMOs, PreInstrSymbol,
-                                         PostInstrSymbol, HeapAllocMarker);
+                                         PostInstrSymbol);
 }
 
 const char *MachineFunction::createExternalSymbolName(StringRef Name) {
@@ -823,6 +824,17 @@ try_next:;
   return FilterID;
 }
 
+void MachineFunction::addCodeViewHeapAllocSite(MachineInstr *I,
+                                               const MDNode *MD) {
+  MCSymbol *BeginLabel = Ctx.createTempSymbol("heapallocsite", true);
+  MCSymbol *EndLabel = Ctx.createTempSymbol("heapallocsite", true);
+  I->setPreInstrSymbol(*this, BeginLabel);
+  I->setPostInstrSymbol(*this, EndLabel);
+
+  const DIType *DI = dyn_cast<DIType>(MD);
+  CodeViewHeapAllocSites.push_back(std::make_tuple(BeginLabel, EndLabel, DI));
+}
+
 void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
                                        const MachineInstr *New) {
   assert(New->isCall() && "Call site info refers only to call instructions!");
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 9c717d24213..fec20b2b1a0 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -316,48 +316,27 @@ void MachineInstr::RemoveOperand(unsigned OpNo) {
   --NumOperands;
 }
 
-void MachineInstr::setExtraInfo(MachineFunction &MF,
-                                ArrayRef<MachineMemOperand *> MMOs,
-                                MCSymbol *PreInstrSymbol,
-                                MCSymbol *PostInstrSymbol,
-                                MDNode *HeapAllocMarker) {
-  bool HasPreInstrSymbol = PreInstrSymbol != nullptr;
-  bool HasPostInstrSymbol = PostInstrSymbol != nullptr;
-  bool HasHeapAllocMarker = HeapAllocMarker != nullptr;
-  int NumPointers =
-      MMOs.size() + HasPreInstrSymbol + HasPostInstrSymbol + HasHeapAllocMarker;
-
-  // Drop all extra info if there is none.
-  if (NumPointers <= 0) {
+void MachineInstr::dropMemRefs(MachineFunction &MF) {
+  if (memoperands_empty())
+    return;
+
+  // See if we can just drop all of our extra info.
+  if (!getPreInstrSymbol() && !getPostInstrSymbol()) {
     Info.clear();
     return;
   }
-
-  // If more than one pointer, then store out of line.
-  // FIXME: Maybe we should make the symbols in the extra info mutable?
-  else if (NumPointers > 1) {
-    Info.set<EIIK_OutOfLine>(MF.createMIExtraInfo(
-        MMOs, PreInstrSymbol, PostInstrSymbol, HeapAllocMarker));
+  if (!getPostInstrSymbol()) {
+    Info.set<EIIK_PreInstrSymbol>(getPreInstrSymbol());
     return;
   }
-
-  // Otherwise store the single pointer inline.
-  if (HasPreInstrSymbol)
-    Info.set<EIIK_PreInstrSymbol>(PreInstrSymbol);
-  else if (HasPostInstrSymbol)
-    Info.set<EIIK_PostInstrSymbol>(PostInstrSymbol);
-  else if (HasHeapAllocMarker)
-    Info.set<EIIK_HeapAllocMarker>(HeapAllocMarker);
-  else
-    Info.set<EIIK_MMO>(MMOs[0]);
-}
-
-void MachineInstr::dropMemRefs(MachineFunction &MF) {
-  if (memoperands_empty())
+  if (!getPreInstrSymbol()) {
+    Info.set<EIIK_PostInstrSymbol>(getPostInstrSymbol());
     return;
+  }
 
-  setExtraInfo(MF, {}, getPreInstrSymbol(), getPostInstrSymbol(),
-               getHeapAllocMarker());
+  // Otherwise allocate a fresh extra info with just these symbols.
+  Info.set<EIIK_OutOfLine>(
+      MF.createMIExtraInfo({}, getPreInstrSymbol(), getPostInstrSymbol()));
 }
 
 void MachineInstr::setMemRefs(MachineFunction &MF,
@@ -367,8 +346,15 @@ void MachineInstr::setMemRefs(MachineFunction &MF,
     return;
   }
 
-  setExtraInfo(MF, MMOs, getPreInstrSymbol(), getPostInstrSymbol(),
-               getHeapAllocMarker());
+  // Try to store a single MMO inline.
+  if (MMOs.size() == 1 && !getPreInstrSymbol() && !getPostInstrSymbol()) {
+    Info.set<EIIK_MMO>(MMOs[0]);
+    return;
+  }
+
+  // Otherwise create an extra info struct with all of our info.
+  Info.set<EIIK_OutOfLine>(
+      MF.createMIExtraInfo(MMOs, getPreInstrSymbol(), getPostInstrSymbol()));
 }
 
 void MachineInstr::addMemOperand(MachineFunction &MF,
@@ -390,8 +376,7 @@ void MachineInstr::cloneMemRefs(MachineFunction &MF, const MachineInstr &MI) {
   // instruction. We can do this whenever the pre- and post-instruction symbols
   // are the same (including null).
   if (getPreInstrSymbol() == MI.getPreInstrSymbol() &&
-      getPostInstrSymbol() == MI.getPostInstrSymbol() &&
-      getHeapAllocMarker() == MI.getHeapAllocMarker()) {
+      getPostInstrSymbol() == MI.getPostInstrSymbol()) {
     Info = MI.Info;
     return;
   }
@@ -465,48 +450,67 @@ void MachineInstr::cloneMergedMemRefs(MachineFunction &MF,
 }
 
 void MachineInstr::setPreInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
-  // Do nothing if old and new symbols are the same.
-  if (Symbol == getPreInstrSymbol())
+  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;
+    }
 
-  // If there was only one symbol and we're removing it, just clear info.
-  if (!Symbol && Info.is<EIIK_PreInstrSymbol>()) {
-    Info.clear();
+    if (memoperands_empty()) {
+      assert(getPostInstrSymbol() &&
+             "Should never have only a single symbol allocated out-of-line!");
+      Info.set<EIIK_PostInstrSymbol>(getPostInstrSymbol());
+      return;
+    }
+
+    // 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>(Symbol);
     return;
   }
 
-  setExtraInfo(MF, memoperands(), Symbol, getPostInstrSymbol(),
-               getHeapAllocMarker());
+  // 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>(
+      MF.createMIExtraInfo(memoperands(), Symbol, getPostInstrSymbol()));
 }
 
 void MachineInstr::setPostInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
-  // Do nothing if old and new symbols are the same.
-  if (Symbol == getPostInstrSymbol())
-    return;
-
-  // If there was only one symbol and we're removing it, just clear info.
-  if (!Symbol && Info.is<EIIK_PostInstrSymbol>()) {
-    Info.clear();
+  MCSymbol *OldSymbol = getPostInstrSymbol();
+  if (OldSymbol == Symbol)
     return;
-  }
-
-  setExtraInfo(MF, memoperands(), getPreInstrSymbol(), Symbol,
-               getHeapAllocMarker());
-}
+  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;
+    }
 
-void MachineInstr::setHeapAllocMarker(MachineFunction &MF, MDNode *Marker) {
-  // Do nothing if old and new symbols are the same.
-  if (Marker == getHeapAllocMarker())
-    return;
+    if (memoperands_empty()) {
+      assert(getPreInstrSymbol() &&
+             "Should never have only a single symbol allocated out-of-line!");
+      Info.set<EIIK_PreInstrSymbol>(getPreInstrSymbol());
+      return;
+    }
 
-  // If there was only one symbol and we're removing it, just clear info.
-  if (!Marker && Info.is<EIIK_HeapAllocMarker>()) {
-    Info.clear();
+    // 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>(Symbol);
     return;
   }
 
-  setExtraInfo(MF, memoperands(), getPreInstrSymbol(), getPostInstrSymbol(),
-               Marker);
+  // 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>(
+      MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol));
 }
 
 void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
@@ -520,7 +524,6 @@ void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
 
   setPreInstrSymbol(MF, MI.getPreInstrSymbol());
   setPostInstrSymbol(MF, MI.getPostInstrSymbol());
-  setHeapAllocMarker(MF, MI.getHeapAllocMarker());
 }
 
 uint16_t MachineInstr::mergeFlagsWith(const MachineInstr &Other) const {
@@ -1707,13 +1710,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
     OS << " post-instr-symbol ";
     MachineOperand::printSymbol(OS, *PostInstrSymbol);
   }
-  if (MDNode *HeapAllocMarker = getHeapAllocMarker()) {
-    if (!FirstOp) {
-      FirstOp = false;
-      OS << ',';
-    }
-    OS << " heap-alloc-marker";
-  }
 
   if (!SkipDebugLoc) {
     if (const DebugLoc &DL = getDebugLoc()) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index c8304f875f7..6d7260d7aee 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1236,9 +1236,10 @@ bool FastISel::lowerCallTo(CallLoweringInfo &CLI) {
     updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
   // Set labels for heapallocsite call.
-  if (CLI.CS)
-    if (MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite"))
-      CLI.Call->setHeapAllocMarker(*MF, MD);
+  if (CLI.CS && CLI.CS->getInstruction()->hasMetadata("heapallocsite")) {
+    const MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite");
+    MF->addCodeViewHeapAllocSite(CLI.Call, MD);
+  }
 
   return true;
 }
diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
index 0e4d783e350..d4c1fb36475 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -910,9 +910,10 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
       if (HasDbg)
         ProcessSourceNode(N, DAG, Emitter, VRBaseMap, Orders, Seen, NewInsn);
 
-      if (MDNode *MD = DAG->getHeapAllocSite(N))
+      if (MDNode *MD = DAG->getHeapAllocSite(N)) {
         if (NewInsn && NewInsn->isCall())
-          NewInsn->setHeapAllocMarker(MF, MD);
+          MF.addCodeViewHeapAllocSite(NewInsn, MD);
+      }
 
       GluedNodes.pop_back();
     }
@@ -922,10 +923,9 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
     if (HasDbg)
       ProcessSourceNode(SU->getNode(), DAG, Emitter, VRBaseMap, Orders, Seen,
                         NewInsn);
-
     if (MDNode *MD = DAG->getHeapAllocSite(SU->getNode())) {
       if (NewInsn && NewInsn->isCall())
-        NewInsn->setHeapAllocMarker(MF, MD);
+        MF.addCodeViewHeapAllocSite(NewInsn, MD);
     }
   }