[llvm] Clean up the GSym error aggregation code, and pass the aggregator by reference (PR #89688)

via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 22 16:47:02 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Kevin Frei (kevinfrei)

<details>
<summary>Changes</summary>

There was a problem with `llvm-gsymutil`s error aggregation code not properly collecting aggregate errors. The was that the output aggregator collecting errors from other threads wasn't being passed by reference, so it was merging them into a copy of the app-wide output aggregator. 

While I was at it, I added a better comment above the "Merge" code and made it a bit more efficient, after learning more details about `emplace` vs. `insert` or `operator[]` on `std::map`'s.

---
Full diff: https://github.com/llvm/llvm-project/pull/89688.diff


2 Files Affected:

- (modified) llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h (+4-5) 
- (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+1-1) 


``````````diff
diff --git a/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
index 8deea3bff1ed76..35ef0a8bc89085 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
@@ -57,13 +57,12 @@ class OutputAggregator {
   }
 
   // For multi-threaded usage, we can collect stuff in another aggregator,
-  // then merge it in here
+  // then merge it in here. Note that this is *not* thread safe. It is up to
+  // the caller to ensure that this is only called from one thread at a time.
   void Merge(const OutputAggregator &other) {
     for (auto &&[name, count] : other.Aggregation) {
-      auto it = Aggregation.find(name);
-      if (it == Aggregation.end())
-        Aggregation.emplace(name, count);
-      else
+      auto [it, inserted] = Aggregation.emplace(name, count);
+      if (!inserted)
         it->second += count;
     }
   }
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index ff6b560d11726b..601686fdd3dd51 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -612,7 +612,7 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
       DWARFDie Die = getDie(*CU);
       if (Die) {
         CUInfo CUI(DICtx, dyn_cast<DWARFCompileUnit>(CU.get()));
-        pool.async([this, CUI, &LogMutex, Out, Die]() mutable {
+        pool.async([this, CUI, &LogMutex, &Out, Die]() mutable {
           std::string storage;
           raw_string_ostream StrStream(storage);
           OutputAggregator ThreadOut(Out.GetOS() ? &StrStream : nullptr);

``````````

</details>


https://github.com/llvm/llvm-project/pull/89688


More information about the llvm-commits mailing list