[PATCH] D156374: [BOLT][DWARF] Don't convert low_pc/high_pc to ranges for size 1, and fix handling of sub-program with ranges

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added a comment.

Thanks for the fix. Please make the title more concise. No need to list all added test cases in the summary.



================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:254
+             "convert low_pc/high_pc to ranges always."),
+    cl::Hidden, cl::init(false), cl::cat(BoltCategory));
 } // namespace opts
----------------



================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:702
 
+  auto updateLowPCHighPC = [&](DIE *Die, const DIEValue &LowPCVal,
+                               const DIEValue &HighPCVal, uint64_t LowPC,
----------------
Add documentation for the function.


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:816
+        if (LowPCVal && HighPCVal) {
+          FunctionRanges.push_back({0, HighPCVal.getDIEInteger().getValue()});
+        } else {
----------------
Do we need to assert here that `LowPCVal.getDIEInteger().getValue() == 0`?


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:818
+        } else {
+          // I haven't seen this case, but who knows what other compilers
+          // generate.
----------------
I wonder if the compiler is aggressively splitting, e.g. all exception-handling code, this case will be quite common?


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:822
+          errs() << "BOLT-WARNING: [internal-dwarf-error]: subprogram got GCed "
+                    "by the linker, DW_AT_ranges is used.\n";
+        }
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156374/new/

https://reviews.llvm.org/D156374