[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
llvm-commits at lists.llvm.org
Fri Jul 28 14:43:53 PDT 2023
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
More information about the llvm-commits
mailing list