[PATCH] D129540: [lld-macho] Enable EH frame parsing / pruning

Tue Jul 12 00:12:43 PDT 2022

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added projects: lld-macho, All.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This just removes the code that gates the logic. The main issue here is
perf impact: without D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible <https://reviews.llvm.org/D122258>, LLD takes a significant perf hit because
it now has to do a lot more work in the input parsing phase. But with
that change to eliminate unnecessary EH frames from input object files,
the perf overhead here is minimal. Concretely, here are the numbers for
some builds as measured on my 16-core Mac Pro:

**chromium_framework**

             base           diff           difference (95% CI)
  sys_time   1.286 ± 0.025  1.348 ± 0.028  [  +3.8% ..   +5.8%]
  user_time  3.513 ± 0.031  3.859 ± 0.035  [  +9.4% ..  +10.3%]
  wall_time  4.205 ± 0.066  4.618 ± 0.065  [  +9.1% ..  +10.6%]
  samples    34             36

This is our good ol' benchmark, without the use of
`-femit-dwarf-unwind`. Since I'm benchmarking this from the repro.tar
and don't have the source build handy, I don't have numbers for this
benchmark with that flag enabled. But I do have those numbers for one of
our internal app builds...

Without `-femit-dwarf-unwind`, this is the perf hit:

**Unnamed internal app**

             base           diff           difference (95% CI)
  sys_time   1.372 ± 0.029  1.317 ± 0.024  [  -4.6% ..   -3.5%]
  user_time  2.835 ± 0.028  2.980 ± 0.027  [  +4.8% ..   +5.4%]
  wall_time  3.205 ± 0.079  3.383 ± 0.066  [  +4.9% ..   +6.2%]
  samples    102            83

With `-femit-dwarf-unwind=no-compact-unwind`, the perf hit almost
disappears:

**Unnamed internal app**

             base           diff           difference (95% CI)
  sys_time   1.274 ± 0.026  1.270 ± 0.025  [  -0.9% ..   +0.3%]
  user_time  2.812 ± 0.023  2.822 ± 0.035  [  +0.1% ..   +0.7%]
  wall_time  3.166 ± 0.047  3.174 ± 0.059  [  -0.2% ..   +0.7%]
  samples    95             97

Just for fun, I measured the impact of `-femit-dwarf-unwind` on ld64
(`base` has the extra DWARF unwind info in the input object files,
`diff` doesn't):

**Unnamed internal app**

             base           diff           difference (95% CI)
  sys_time   1.128 ± 0.010  1.124 ± 0.023  [  -1.3% ..   +0.6%]
  user_time  7.176 ± 0.030  7.106 ± 0.094  [  -1.5% ..   -0.4%]
  wall_time  7.874 ± 0.041  7.795 ± 0.121  [  -1.7% ..   -0.3%]
  samples    16             25

And for LLD:

**Unnamed internal app**

             base           diff           difference (95% CI)
  sys_time   1.315 ± 0.019  1.280 ± 0.019  [  -3.2% ..   -2.0%]
  user_time  2.980 ± 0.022  2.822 ± 0.016  [  -5.5% ..   -5.0%]
  wall_time  3.369 ± 0.038  3.175 ± 0.033  [  -6.2% ..   -5.3%]
  samples    47             47

So parsing the extra EH frames is a lot more expensive for us than for
ld64. But given that we are quite a lot faster than ld64 to begin with,
I guess this isn't entirely unexpected...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129540

Files:
  lld/MachO/Config.h
  lld/MachO/Driver.cpp
  lld/MachO/InputFiles.cpp


Index: lld/MachO/InputFiles.cpp
===================================================================

--- lld/MachO/InputFiles.cpp
+++ lld/MachO/InputFiles.cpp
@@ -347,7 +347,7 @@
       section.subsections.push_back({0, isec});
     } else if (auto recordSize = getRecordSize(segname, name)) {
       splitRecords(*recordSize);
-    } else if (config->parseEhFrames && name == section_names::ehFrame &&
+    } else if (name == section_names::ehFrame &&
                segname == segment_names::text) {
       splitEhFrames(data, *sections.back());
     } else if (segname == segment_names::llvm) {
@@ -1117,7 +1117,7 @@
   }
   if (compactUnwindSection)
     registerCompactUnwind(*compactUnwindSection);
-  if (config->parseEhFrames && ehFrameSection)
+  if (ehFrameSection)
     registerEhFrames(*ehFrameSection);
 }
 
Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -1305,7 +1305,6 @@
   config->callGraphProfileSort = args.hasFlag(
       OPT_call_graph_profile_sort, OPT_no_call_graph_profile_sort, true);
   config->printSymbolOrder = args.getLastArgValue(OPT_print_symbol_order);
-  config->parseEhFrames = static_cast<bool>(getenv("LLD_IN_TEST"));
 
   // FIXME: Add a commandline flag for this too.
   config->zeroModTime = getenv("ZERO_AR_DATE");
Index: lld/MachO/Config.h
===================================================================
--- lld/MachO/Config.h
+++ lld/MachO/Config.h
@@ -131,9 +131,6 @@
   bool omitDebugInfo = false;
   bool warnDylibInstallName = false;
   bool ignoreOptimizationHints = false;
-  // Temporary config flag that will be removed once we have fully implemented
-  // support for __eh_frame.
-  bool parseEhFrames = false;
   uint32_t headerPad;
   uint32_t dylibCompatibilityVersion = 0;
   uint32_t dylibCurrentVersion = 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129540.443840.patch
Type: text/x-patch
Size: 1872 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220712/c3f2f836/attachment.bin>
