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

[Alexander Yermolovich via Phabricator via llvm-commits]

ayermolo marked 3 inline comments as done.
ayermolo added inline comments.


================
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
----------------
maksfb wrote:
> 
Didn't realize there was ReallyHidden option. Is there a ReallyReallyRallyLikeForRealItsHidden one? lol


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:816
+        if (LowPCVal && HighPCVal) {
+          FunctionRanges.push_back({0, HighPCVal.getDIEInteger().getValue()});
+        } else {
----------------
maksfb wrote:
> Do we need to assert here that `LowPCVal.getDIEInteger().getValue() == 0`?
Well it might not be actually. Should we just use LowPCVal.getDIEInteger().getValue(), even thought address is wrong?
My thought was is that LLDB can deal with low_pc=0, not sure what it does when address is wrong.


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:818
+        } else {
+          // I haven't seen this case, but who knows what other compilers
+          // generate.
----------------
maksfb wrote:
> I wonder if the compiler is aggressively splitting, e.g. all exception-handling code, this case will be quite common?
no idea. I haven't actually seen this in the wild. Thus a warning, and just putting something that "makes sense".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156374