Revert "Restore "[MemProf] ThinLTO summary support" with fixes"

This reverts commit 00c2235.
This reverts commit 98ed423.

Seemingly MSVC has some kind of issue with this patch, in terms of linking:

  https://lab.llvm.org/buildbot/#/builders/123/builds/14137

I'll post more detail on D135714 momentarily.

Author: jmorse

llvm/include/llvm/Analysis/MemoryProfileInfo.h

@@ -17,12 +17,18 @@
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/ModuleSummaryIndex.h"
 #include <map>
 
 namespace llvm {
 namespace memprof {
 
+// Allocation type assigned to an allocation reached by a given context.
+// More can be added but initially this is just noncold and cold.
+// Values should be powers of two so that they can be ORed, in particular to
+// track allocations that have different behavior with different calling
+// contexts.
+enum class AllocationType : uint8_t { None = 0, NotCold = 1, Cold = 2 };
+
 /// Return the allocation type for a given set of memory profile values.
 AllocationType getAllocType(uint64_t MaxAccessCount, uint64_t MinSize,
                             uint64_t MinLifetime);
@@ -100,62 +106,6 @@ class CallStackTrie {
   bool buildAndAttachMIBMetadata(CallBase *CI);
 };
 
-/// Helper class to iterate through stack ids in both metadata (memprof MIB and
-/// callsite) and the corresponding ThinLTO summary data structures
-/// (CallsiteInfo and MIBInfo). This simplifies implementation of client code
-/// which doesn't need to worry about whether we are operating with IR (Regular
-/// LTO), or summary (ThinLTO).
-template <class NodeT, class IteratorT> class CallStack {
-public:
-  CallStack(const NodeT *N = nullptr) : N(N) {}
-
-  // Implement minimum required methods for range-based for loop.
-  // The default implementation assumes we are operating on ThinLTO data
-  // structures, which have a vector of StackIdIndices. There are specialized
-  // versions provided to iterate through metadata.
-  struct CallStackIterator {
-    const NodeT *N = nullptr;
-    IteratorT Iter;
-    CallStackIterator(const NodeT *N, bool End) : N(N) {
-      if (!N)
-        return;
-      Iter = End ? N->StackIdIndices.end() : N->StackIdIndices.begin();
-    }
-    uint64_t operator*() {
-      assert(Iter != N->StackIdIndices.end());
-      return *Iter;
-    }
-    bool operator==(const CallStackIterator &rhs) { return Iter == rhs.Iter; }
-    bool operator!=(const CallStackIterator &rhs) { return !(*this == rhs); }
-    void operator++() { ++Iter; }
-  };
-
-  bool empty() const { return N == nullptr; }
-
-  CallStackIterator begin() const {
-    return CallStackIterator(N, /*End*/ false);
-  }
-  CallStackIterator end() const { return CallStackIterator(N, /*End*/ true); }
-
-  CallStackIterator beginAfterSharedPrefix(CallStack &Other) {
-    CallStackIterator Cur = begin();
-    for (CallStackIterator OtherCur = Other.begin();
-         Cur != end() && OtherCur != Other.end(); ++Cur, ++OtherCur)
-      assert(*Cur == *OtherCur);
-    return Cur;
-  }
-
-private:
-  const NodeT *N = nullptr;
-};
-
-/// Specializations for iterating through IR metadata stack contexts.
-template <>
-CallStack<MDNode, MDNode::op_iterator>::CallStackIterator::CallStackIterator(
-    const MDNode *N, bool End);
-template <>
-uint64_t CallStack<MDNode, MDNode::op_iterator>::CallStackIterator::operator*();
-
 } // end namespace memprof
 } // end namespace llvm
 

llvm/include/llvm/AsmParser/LLParser.h

@@ -406,10 +406,6 @@ namespace llvm {
     void addGlobalValueToIndex(std::string Name, GlobalValue::GUID,
                                GlobalValue::LinkageTypes Linkage, unsigned ID,
                                std::unique_ptr<GlobalValueSummary> Summary);
-    bool parseOptionalAllocs(std::vector<AllocInfo> &Allocs);
-    bool parseMemProfs(std::vector<MIBInfo> &MIBs);
-    bool parseAllocType(uint8_t &AllocType);
-    bool parseOptionalCallsites(std::vector<CallsiteInfo> &Callsites);
 
     // Type Parsing.
     bool parseType(Type *&Result, const Twine &Msg, bool AllowVoid = false);

llvm/include/llvm/AsmParser/LLToken.h

@@ -406,15 +406,6 @@ enum Kind {
   kw_byte,
   kw_bit,
   kw_varFlags,
-  // The following are used by MemProf summary info.
-  kw_callsites,
-  kw_clones,
-  kw_stackIds,
-  kw_allocs,
-  kw_versions,
-  kw_memProf,
-  kw_notcold,
-  kw_notcoldandcold,
 
   // GV's with __attribute__((no_sanitize("address"))), or things in
   // -fsanitize-ignorelist when built with ASan.

llvm/include/llvm/AsmParser/Parser.h

@@ -105,17 +105,6 @@ ParsedModuleAndIndex parseAssemblyFileWithIndexNoUpgradeDebugInfo(
 std::unique_ptr<ModuleSummaryIndex>
 parseSummaryIndexAssemblyFile(StringRef Filename, SMDiagnostic &Err);
 
-/// The function is a secondary interface to the LLVM Assembly Parser. It parses
-/// an ASCII string that (presumably) contains LLVM Assembly code for a module
-/// summary. It returns a a ModuleSummaryIndex with the corresponding features.
-/// Note that this does not verify that the generated Index is valid, so you
-/// should run the verifier after parsing the file to check that it is okay.
-/// Parse LLVM Assembly from a string
-/// \param AsmString The string containing assembly
-/// \param Err Error result info.
-std::unique_ptr<ModuleSummaryIndex>
-parseSummaryIndexAssemblyString(StringRef AsmString, SMDiagnostic &Err);
-
 /// parseAssemblyFile and parseAssemblyString are wrappers around this function.
 /// Parse LLVM Assembly from a MemoryBuffer.
 /// \param F The MemoryBuffer containing assembly

llvm/include/llvm/Bitcode/BitcodeReader.h

@@ -16,7 +16,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitCodeEnums.h"
-#include "llvm/IR/GlobalValue.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
@@ -118,10 +117,8 @@ typedef llvm::function_ref<Optional<std::string>(StringRef)>
 
     /// Parse the specified bitcode buffer and merge its module summary index
     /// into CombinedIndex.
-    Error
-    readSummary(ModuleSummaryIndex &CombinedIndex, StringRef ModulePath,
-                uint64_t ModuleId,
-                std::function<bool(GlobalValue::GUID)> IsPrevailing = nullptr);
+    Error readSummary(ModuleSummaryIndex &CombinedIndex, StringRef ModulePath,
+                      uint64_t ModuleId);
   };
 
   struct BitcodeFileContents {

llvm/include/llvm/Bitcode/LLVMBitCodes.h

@@ -301,22 +301,6 @@ enum GlobalValueSummarySymtabCodes {
   // Range information for accessed offsets for every argument.
   // [n x (paramno, range, numcalls, numcalls x (callee_guid, paramno, range))]
   FS_PARAM_ACCESS = 25,
-  // Summary of per-module memprof callsite metadata.
-  // [valueid, n x stackidindex]
-  FS_PERMODULE_CALLSITE_INFO = 26,
-  // Summary of per-module allocation memprof metadata.
-  // [n x (alloc type, nummib, nummib x stackidindex)]
-  FS_PERMODULE_ALLOC_INFO = 27,
-  // Summary of combined index memprof callsite metadata.
-  // [valueid, numstackindices, numver,
-  //  numstackindices x stackidindex, numver x version]
-  FS_COMBINED_CALLSITE_INFO = 28,
-  // Summary of combined index allocation memprof metadata.
-  // [nummib, numver,
-  //  nummib x (alloc type, numstackids, numstackids x stackidindex),
-  //  numver x version]
-  FS_COMBINED_ALLOC_INFO = 29,
-  FS_STACK_IDS = 30,
 };
 
 enum MetadataCodes {

llvm/include/llvm/IR/ModuleSummaryIndex.h

@@ -19,7 +19,6 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -285,79 +284,6 @@ template <> struct DenseMapInfo<ValueInfo> {
   static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.getRef(); }
 };
 
-/// Summary of memprof callsite metadata.
-struct CallsiteInfo {
-  // Actual callee function.
-  ValueInfo Callee;
-
-  // Used to record whole program analysis cloning decisions.
-  // The ThinLTO backend will need to create as many clones as there are entries
-  // in the vector (it is expected and should be confirmed that all such
-  // summaries in the same FunctionSummary have the same number of entries).
-  // Each index records version info for the corresponding clone of this
-  // function. The value is the callee clone it calls (becomes the appended
-  // suffix id). Index 0 is the original version, and a value of 0 calls the
-  // original callee.
-  SmallVector<unsigned> Clones{0};
-
-  // Represents stack ids in this context, recorded as indices into the
-  // StackIds vector in the summary index, which in turn holds the full 64-bit
-  // stack ids. This reduces memory as there are in practice far fewer unique
-  // stack ids than stack id references.
-  SmallVector<unsigned> StackIdIndices;
-
-  CallsiteInfo(ValueInfo Callee, SmallVector<unsigned> StackIdIndices)
-      : Callee(Callee), StackIdIndices(std::move(StackIdIndices)) {}
-  CallsiteInfo(ValueInfo Callee, SmallVector<unsigned> Clones,
-               SmallVector<unsigned> StackIdIndices)
-      : Callee(Callee), Clones(std::move(Clones)),
-        StackIdIndices(std::move(StackIdIndices)) {}
-};
-
-// Allocation type assigned to an allocation reached by a given context.
-// More can be added but initially this is just noncold and cold.
-// Values should be powers of two so that they can be ORed, in particular to
-// track allocations that have different behavior with different calling
-// contexts.
-enum class AllocationType : uint8_t { None = 0, NotCold = 1, Cold = 2 };
-
-/// Summary of a single MIB in a memprof metadata on allocations.
-struct MIBInfo {
-  // The allocation type for this profiled context.
-  AllocationType AllocType;
-
-  // Represents stack ids in this context, recorded as indices into the
-  // StackIds vector in the summary index, which in turn holds the full 64-bit
-  // stack ids. This reduces memory as there are in practice far fewer unique
-  // stack ids than stack id references.
-  SmallVector<unsigned> StackIdIndices;
-
-  MIBInfo(AllocationType AllocType, SmallVector<unsigned> StackIdIndices)
-      : AllocType(AllocType), StackIdIndices(std::move(StackIdIndices)) {}
-};
-
-/// Summary of memprof metadata on allocations.
-struct AllocInfo {
-  // Used to record whole program analysis cloning decisions.
-  // The ThinLTO backend will need to create as many clones as there are entries
-  // in the vector (it is expected and should be confirmed that all such
-  // summaries in the same FunctionSummary have the same number of entries).
-  // Each index records version info for the corresponding clone of this
-  // function. The value is the allocation type of the corresponding allocation.
-  // Index 0 is the original version. Before cloning, index 0 may have more than
-  // one allocation type.
-  SmallVector<uint8_t> Versions;
-
-  // Vector of MIBs in this memprof metadata.
-  std::vector<MIBInfo> MIBs;
-
-  AllocInfo(std::vector<MIBInfo> MIBs) : MIBs(std::move(MIBs)) {
-    Versions.push_back(0);
-  }
-  AllocInfo(SmallVector<uint8_t> Versions, std::vector<MIBInfo> MIBs)
-      : Versions(std::move(Versions)), MIBs(std::move(MIBs)) {}
-};
-
 /// Function and variable summary information to aid decisions and
 /// implementation of importing.
 class GlobalValueSummary {
@@ -752,8 +678,7 @@ class FunctionSummary : public GlobalValueSummary {
         std::vector<FunctionSummary::VFuncId>(),
         std::vector<FunctionSummary::ConstVCall>(),
         std::vector<FunctionSummary::ConstVCall>(),
-        std::vector<FunctionSummary::ParamAccess>(),
-        std::vector<CallsiteInfo>(), std::vector<AllocInfo>());
+        std::vector<FunctionSummary::ParamAccess>());
   }
 
   /// A dummy node to reference external functions that aren't in the index
@@ -781,25 +706,6 @@ class FunctionSummary : public GlobalValueSummary {
   using ParamAccessesTy = std::vector<ParamAccess>;
   std::unique_ptr<ParamAccessesTy> ParamAccesses;
 
-  /// Optional list of memprof callsite metadata summaries. The correspondence
-  /// between the callsite summary and the callsites in the function is implied
-  /// by the order in the vector (and can be validated by comparing the stack
-  /// ids in the CallsiteInfo to those in the instruction callsite metadata).
-  /// As a memory savings optimization, we only create these for the prevailing
-  /// copy of a symbol when creating the combined index during LTO.
-  using CallsitesTy = std::vector<CallsiteInfo>;
-  std::unique_ptr<CallsitesTy> Callsites;
-
-  /// Optional list of allocation memprof metadata summaries. The correspondence
-  /// between the alloc memprof summary and the allocation callsites in the
-  /// function is implied by the order in the vector (and can be validated by
-  /// comparing the stack ids in the AllocInfo to those in the instruction
-  /// memprof metadata).
-  /// As a memory savings optimization, we only create these for the prevailing
-  /// copy of a symbol when creating the combined index during LTO.
-  using AllocsTy = std::vector<AllocInfo>;
-  std::unique_ptr<AllocsTy> Allocs;
-
 public:
   FunctionSummary(GVFlags Flags, unsigned NumInsts, FFlags FunFlags,
                   uint64_t EntryCount, std::vector<ValueInfo> Refs,
@@ -809,8 +715,7 @@ class FunctionSummary : public GlobalValueSummary {
                   std::vector<VFuncId> TypeCheckedLoadVCalls,
                   std::vector<ConstVCall> TypeTestAssumeConstVCalls,
                   std::vector<ConstVCall> TypeCheckedLoadConstVCalls,
-                  std::vector<ParamAccess> Params, CallsitesTy CallsiteList,
-                  AllocsTy AllocList)
+                  std::vector<ParamAccess> Params)
       : GlobalValueSummary(FunctionKind, Flags, std::move(Refs)),
         InstCount(NumInsts), FunFlags(FunFlags), EntryCount(EntryCount),
         CallGraphEdgeList(std::move(CGEdges)) {
@@ -824,10 +729,6 @@ class FunctionSummary : public GlobalValueSummary {
                      std::move(TypeCheckedLoadConstVCalls)});
     if (!Params.empty())
       ParamAccesses = std::make_unique<ParamAccessesTy>(std::move(Params));
-    if (!CallsiteList.empty())
-      Callsites = std::make_unique<CallsitesTy>(std::move(CallsiteList));
-    if (!AllocList.empty())
-      Allocs = std::make_unique<AllocsTy>(std::move(AllocList));
   }
   // Gets the number of readonly and writeonly refs in RefEdgeList
   std::pair<unsigned, unsigned> specialRefCounts() const;
@@ -931,18 +832,6 @@ class FunctionSummary : public GlobalValueSummary {
 
   const TypeIdInfo *getTypeIdInfo() const { return TIdInfo.get(); };
 
-  ArrayRef<CallsiteInfo> callsites() const {
-    if (Callsites)
-      return *Callsites;
-    return {};
-  }
-
-  ArrayRef<AllocInfo> allocs() const {
-    if (Allocs)
-      return *Allocs;
-    return {};
-  }
-
   friend struct GraphTraits<ValueInfo>;
 };
 
@@ -1274,16 +1163,6 @@ class ModuleSummaryIndex {
   // the total number of basic blocks in the LTO unit in the combined index.
   uint64_t BlockCount;
 
-  // List of unique stack ids (hashes). We use a 4B index of the id in the
-  // stack id lists on the alloc and callsite summaries for memory savings,
-  // since the number of unique ids is in practice much smaller than the
-  // number of stack id references in the summaries.
-  std::vector<uint64_t> StackIds;
-
-  // Temporary map while building StackIds list. Clear when index is completely
-  // built via releaseTemporaryMemory.
-  std::map<uint64_t, unsigned> StackIdToIndex;
-
   // YAML I/O support.
   friend yaml::MappingTraits<ModuleSummaryIndex>;
 
@@ -1326,31 +1205,6 @@ class ModuleSummaryIndex {
   const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
   size_t size() const { return GlobalValueMap.size(); }
 
-  const std::vector<uint64_t> &stackIds() const { return StackIds; }
-
-  unsigned addOrGetStackIdIndex(uint64_t StackId) {
-    auto Inserted = StackIdToIndex.insert({StackId, StackIds.size()});
-    if (Inserted.second)
-      StackIds.push_back(StackId);
-    return Inserted.first->second;
-  }
-
-  uint64_t getStackIdAtIndex(unsigned Index) const {
-    assert(StackIds.size() > Index);
-    return StackIds[Index];
-  }
-
-  // Facility to release memory from data structures only needed during index
-  // construction (including while building combined index). Currently this only
-  // releases the temporary map used while constructing a correspondence between
-  // stack ids and their index in the StackIds vector. Mostly impactful when
-  // building a large combined index.
-  void releaseTemporaryMemory() {
-    assert(StackIdToIndex.size() == StackIds.size());
-    StackIdToIndex.clear();
-    StackIds.shrink_to_fit();
-  }
-
   /// Convenience function for doing a DFS on a ValueInfo. Marks the function in
   /// the FunctionHasParent map.
   static void discoverNodes(ValueInfo V,

llvm/include/llvm/IR/ModuleSummaryIndexYAML.h

@@ -234,8 +234,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
           std::move(FSum.TypeCheckedLoadVCalls),
           std::move(FSum.TypeTestAssumeConstVCalls),
           std::move(FSum.TypeCheckedLoadConstVCalls),
-          ArrayRef<FunctionSummary::ParamAccess>{}, ArrayRef<CallsiteInfo>{},
-          ArrayRef<AllocInfo>{}));
+          ArrayRef<FunctionSummary::ParamAccess>{}));
     }
   }
   static void output(IO &io, GlobalValueSummaryMapTy &V) {