[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