Revert "[ELF] Add BPSectionOrderer options (llvm#120514)"

The ELF/bp-section-orderer.s test is failing on some buildbots due to
what seems like non-determinism issues, see comments on the original PR
and llvm#125450

Reverting to green the build.

This reverts commit 0154dce and
follow-up commits 046dd4b and
c92f204.

Author: zmodem

lld/ELF/BPSectionOrderer.cpp

@@ -37,7 +37,6 @@ add_lld_library(lldELF
   Arch/X86.cpp
   Arch/X86_64.cpp
   ARMErrataFix.cpp
-  BPSectionOrderer.cpp
   CallGraphSort.cpp
   DWARF.cpp
   Driver.cpp
@@ -73,7 +72,6 @@ add_lld_library(lldELF
   Object
   Option
   Passes
-  ProfileData
   Support
   TargetParser
   TransformUtils

lld/ELF/BPSectionOrderer.h

@@ -264,12 +264,6 @@ struct Config {
   bool armBe8 = false;
   BsymbolicKind bsymbolic = BsymbolicKind::None;
   CGProfileSortKind callGraphProfileSort;
-  llvm::StringRef irpgoProfilePath;
-  bool bpStartupFunctionSort = false;
-  bool bpCompressionSortStartupFunctions = false;
-  bool bpFunctionOrderForCompression = false;
-  bool bpDataOrderForCompression = false;
-  bool bpVerboseSectionOrderer = false;
   bool checkSections;
   bool checkDynamicRelocs;
   std::optional<llvm::DebugCompressionType> compressDebugSections;

lld/ELF/CMakeLists.txt

@@ -1121,53 +1121,6 @@ static CGProfileSortKind getCGProfileSortKind(Ctx &ctx,
   return CGProfileSortKind::None;
 }
 
-static void parseBPOrdererOptions(Ctx &ctx, opt::InputArgList &args) {
-  if (auto *arg = args.getLastArg(OPT_bp_compression_sort)) {
-    StringRef s = arg->getValue();
-    if (s == "function") {
-      ctx.arg.bpFunctionOrderForCompression = true;
-    } else if (s == "data") {
-      ctx.arg.bpDataOrderForCompression = true;
-    } else if (s == "both") {
-      ctx.arg.bpFunctionOrderForCompression = true;
-      ctx.arg.bpDataOrderForCompression = true;
-    } else if (s != "none") {
-      ErrAlways(ctx) << arg->getSpelling()
-                     << ": expected [none|function|data|both]";
-    }
-    if (s != "none" && args.hasArg(OPT_call_graph_ordering_file))
-      ErrAlways(ctx) << "--bp-compression-sort is incompatible with "
-                        "--call-graph-ordering-file";
-  }
-  if (auto *arg = args.getLastArg(OPT_bp_startup_sort)) {
-    StringRef s = arg->getValue();
-    if (s == "function") {
-      ctx.arg.bpStartupFunctionSort = true;
-    } else if (s != "none") {
-      ErrAlways(ctx) << arg->getSpelling() << ": expected [none|function]";
-    }
-    if (s != "none" && args.hasArg(OPT_call_graph_ordering_file))
-      ErrAlways(ctx) << "--bp-startup-sort=function is incompatible with "
-                        "--call-graph-ordering-file";
-  }
-
-  ctx.arg.bpCompressionSortStartupFunctions =
-      args.hasFlag(OPT_bp_compression_sort_startup_functions,
-                   OPT_no_bp_compression_sort_startup_functions, false);
-  ctx.arg.bpVerboseSectionOrderer = args.hasArg(OPT_verbose_bp_section_orderer);
-
-  ctx.arg.irpgoProfilePath = args.getLastArgValue(OPT_irpgo_profile);
-  if (ctx.arg.irpgoProfilePath.empty()) {
-    if (ctx.arg.bpStartupFunctionSort)
-      ErrAlways(ctx) << "--bp-startup-sort=function must be used with "
-                        "--irpgo-profile";
-    if (ctx.arg.bpCompressionSortStartupFunctions)
-      ErrAlways(ctx)
-          << "--bp-compression-sort-startup-functions must be used with "
-             "--irpgo-profile";
-  }
-}
-
 static DebugCompressionType getCompressionType(Ctx &ctx, StringRef s,
                                                StringRef option) {
   DebugCompressionType type = StringSwitch<DebugCompressionType>(s)
@@ -1309,7 +1262,6 @@ static void readConfigs(Ctx &ctx, opt::InputArgList &args) {
       ctx.arg.bsymbolic = BsymbolicKind::All;
   }
   ctx.arg.callGraphProfileSort = getCGProfileSortKind(ctx, args);
-  parseBPOrdererOptions(ctx, args);
   ctx.arg.checkSections =
       args.hasFlag(OPT_check_sections, OPT_no_check_sections, true);
   ctx.arg.chroot = args.getLastArgValue(OPT_chroot);

lld/ELF/Config.h

@@ -141,19 +141,6 @@ def call_graph_profile_sort: JJ<"call-graph-profile-sort=">,
 def : FF<"no-call-graph-profile-sort">, Alias<call_graph_profile_sort>, AliasArgs<["none"]>,
   Flags<[HelpHidden]>;
 
-defm irpgo_profile: EEq<"irpgo-profile",
-  "Read a temporary profile file for use with --bp-startup-sort=">;
-def bp_compression_sort: JJ<"bp-compression-sort=">, MetaVarName<"[none,function,data,both]">,
-  HelpText<"Improve Lempel-Ziv compression by grouping similar sections together, resulting in a smaller compressed app size">;
-def bp_startup_sort: JJ<"bp-startup-sort=">, MetaVarName<"[none,function]">,
-  HelpText<"Utilize a temporal profile file to reduce page faults during program startup">;
-
-// Auxiliary options related to balanced partition
-defm bp_compression_sort_startup_functions: BB<"bp-compression-sort-startup-functions",
-  "When --irpgo-profile is pecified, prioritize function similarity for compression in addition to startup time", "">;
-def verbose_bp_section_orderer: FF<"verbose-bp-section-orderer">,
-  HelpText<"Print information on balanced partitioning">;
-
 // --chroot doesn't have a help text because it is an internal option.
 def chroot: Separate<["--"], "chroot">;
 

lld/ELF/Driver.cpp

@@ -9,7 +9,6 @@
 #include "Writer.h"
 #include "AArch64ErrataFix.h"
 #include "ARMErrataFix.h"
-#include "BPSectionOrderer.h"
 #include "CallGraphSort.h"
 #include "Config.h"
 #include "InputFiles.h"
@@ -1081,18 +1080,8 @@ static void maybeShuffle(Ctx &ctx,
 // that don't appear in the order file.
 static DenseMap<const InputSectionBase *, int> buildSectionOrder(Ctx &ctx) {
   DenseMap<const InputSectionBase *, int> sectionOrder;
-  if (ctx.arg.bpStartupFunctionSort || ctx.arg.bpFunctionOrderForCompression ||
-      ctx.arg.bpDataOrderForCompression) {
-    TimeTraceScope timeScope("Balanced Partitioning Section Orderer");
-    sectionOrder = runBalancedPartitioning(
-        ctx, ctx.arg.bpStartupFunctionSort ? ctx.arg.irpgoProfilePath : "",
-        ctx.arg.bpFunctionOrderForCompression,
-        ctx.arg.bpDataOrderForCompression,
-        ctx.arg.bpCompressionSortStartupFunctions,
-        ctx.arg.bpVerboseSectionOrderer);
-  } else if (!ctx.arg.callGraphProfile.empty()) {
+  if (!ctx.arg.callGraphProfile.empty())
     sectionOrder = computeCallGraphProfileOrder(ctx);
-  }
 
   if (ctx.arg.symbolOrderingFile.empty())
     return sectionOrder;

lld/ELF/Options.td

@@ -110,7 +110,7 @@ DenseMap<const InputSection *, int> lld::macho::runBalancedPartitioning(
     bool compressionSortStartupFunctions, bool verbose) {
   // Collect candidate sections and associated symbols.
   SmallVector<InputSection *> sections;
-  DenseMap<CachedHashStringRef, std::set<unsigned>> rootSymbolToSectionIdxs;
+  DenseMap<CachedHashStringRef, DenseSet<unsigned>> rootSymbolToSectionIdxs;
   for (const auto *file : inputFiles) {
     for (auto *sec : file->sections) {
       for (auto &subsec : sec->subsections) {

lld/ELF/Writer.cpp

@@ -22,7 +22,7 @@
 #include "lld/Common/ErrorHandler.h"
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -35,7 +35,6 @@
 #include "llvm/Support/VirtualFileSystem.h"
 #include <memory>
 #include <optional>
-#include <set>
 
 #define DEBUG_TYPE "bp-section-orderer"
 
@@ -61,13 +60,9 @@ template <class D> struct BPOrderer {
                     bool forDataCompression,
                     bool compressionSortStartupFunctions, bool verbose,
                     llvm::ArrayRef<Section *> sections,
-                    const DenseMap<CachedHashStringRef, std::set<unsigned>>
+                    const DenseMap<CachedHashStringRef, DenseSet<unsigned>>
                         &rootSymbolToSectionIdxs)
       -> llvm::DenseMap<const Section *, int>;
-
-  std::optional<StringRef> static getResolvedLinkageName(llvm::StringRef name) {
-    return {};
-  }
 };
 } // namespace lld
 
@@ -78,7 +73,7 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
     ArrayRef<const typename D::Section *> sections,
     const DenseMap<const void *, uint64_t> &sectionToIdx,
     ArrayRef<unsigned> sectionIdxs,
-    DenseMap<unsigned, SmallVector<unsigned, 0>> *duplicateSectionIdxs,
+    DenseMap<unsigned, SmallVector<unsigned>> *duplicateSectionIdxs,
     BPFunctionNode::UtilityNodeT &maxUN) {
   TimeTraceScope timeScope("Build nodes for compression");
 
@@ -93,7 +88,7 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
     hashes.clear();
   }
 
-  MapVector<uint64_t, unsigned> hashFrequency;
+  DenseMap<uint64_t, unsigned> hashFrequency;
   for (auto &[sectionIdx, hashes] : sectionHashes)
     for (auto hash : hashes)
       ++hashFrequency[hash];
@@ -102,11 +97,10 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
     // Merge sections that are nearly identical
     SmallVector<std::pair<unsigned, SmallVector<uint64_t>>> newSectionHashes;
     DenseMap<uint64_t, unsigned> wholeHashToSectionIdx;
-    unsigned threshold = sectionHashes.size() > 10000 ? 5 : 0;
     for (auto &[sectionIdx, hashes] : sectionHashes) {
       uint64_t wholeHash = 0;
       for (auto hash : hashes)
-        if (hashFrequency[hash] > threshold)
+        if (hashFrequency[hash] > 5)
           wholeHash ^= hash;
       auto [it, wasInserted] =
           wholeHashToSectionIdx.insert(std::make_pair(wholeHash, sectionIdx));
@@ -162,7 +156,7 @@ auto BPOrderer<D>::computeOrder(
     StringRef profilePath, bool forFunctionCompression, bool forDataCompression,
     bool compressionSortStartupFunctions, bool verbose,
     ArrayRef<Section *> sections,
-    const DenseMap<CachedHashStringRef, std::set<unsigned>>
+    const DenseMap<CachedHashStringRef, DenseSet<unsigned>>
         &rootSymbolToSectionIdxs) -> DenseMap<const Section *, int> {
   TimeTraceScope timeScope("Setup Balanced Partitioning");
   DenseMap<const void *, uint64_t> sectionToIdx;
@@ -186,7 +180,7 @@ auto BPOrderer<D>::computeOrder(
     }
     auto &traces = reader->getTemporalProfTraces();
 
-    MapVector<unsigned, BPFunctionNode::UtilityNodeT> sectionIdxToFirstUN;
+    DenseMap<unsigned, BPFunctionNode::UtilityNodeT> sectionIdxToFirstUN;
     for (size_t traceIdx = 0; traceIdx < traces.size(); traceIdx++) {
       uint64_t currentSize = 0, cutoffSize = 1;
       size_t cutoffTimestamp = 1;
@@ -263,7 +257,7 @@ auto BPOrderer<D>::computeOrder(
 
   // Map a section index (order directly) to a list of duplicate section indices
   // (not ordered directly).
-  DenseMap<unsigned, SmallVector<unsigned, 0>> duplicateSectionIdxs;
+  DenseMap<unsigned, SmallVector<unsigned>> duplicateSectionIdxs;
   auto unsForFunctionCompression = getUnsForCompression<D>(
       sections, sectionToIdx, sectionIdxsForFunctionCompression,
       &duplicateSectionIdxs, maxUN);