[PATCH] D135359: [lld-macho] Don't fold DWARFs with CompactUnwinds
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 6 10:41:37 PDT 2022
int3 added a comment.
Please put the "drive-by cleanup" in a separate diff... it would have made it a lot easier to figure out what this diff is doing
> because there's no 0x05000000 in the enum set, but as soon as it's defined, it'll stop working
I can see why there might be an issue if 0x11000000 was a defined mode, but I don't think `0x05000000` would be a problem?
================
Comment at: lld/MachO/InputFiles.cpp:1074-1077
+ static_assert(static_cast<uint32_t>(UNWIND_X86_64_MODE_MASK) ==
+ static_cast<uint32_t>(UNWIND_X86_MODE_MASK) &&
+ static_cast<uint32_t>(UNWIND_ARM64_MODE_MASK) ==
+ static_cast<uint32_t>(UNWIND_X86_64_MODE_MASK));
----------------
can we hoist to the top level scope and then define a `constexpr UNWIND_MODE_MASK = ...`?
then we can use in it `canFoldEncoding` too (and remove the first `static_assert()` in that function)
================
Comment at: lld/MachO/UnwindInfoSection.cpp:400-403
+ // Can't fold DWARF sections.
+ if ((encoding & static_cast<uint32_t>(UNWIND_X86_64_MODE_MASK)) ==
+ target->modeDwarfEncoding)
+ return false;
----------------
Unless there's an obvious runtime issue, I would rather we keep things the way they are. Is there any observable behavioral difference with this change?
DWARF mode encodings will almost (always?) never get folded anyway since each one includes an offset to the corresponding EH frame. And since we don't do ICF on EH frames, they should all be distinct...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135359/new/
https://reviews.llvm.org/D135359
More information about the llvm-commits
mailing list