[PATCH] D122258: [MC][RFC] Omit DWARF unwind info if compact unwind is present for all archs

[Jez Ng via Phabricator via cfe-commits]

int3 created this revision.
int3 added reviewers: davide, kledzik, lhames.
Herald added subscribers: StephenFan, cmtice, hiraditya, kristof.beyls, emaste.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: jhenderson.
Herald added a project: All.
int3 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

Diff is not yet complete, but I'm looking for some feedback...

Previously, omitting unnecessary DWARF unwinds was only done for
watchOS, but I've heard that it might be possible to do this for all
archs. @davide, @kledzik -- do you see any issues with this?

Motivation: I'm working on adding support for `__eh_frame` to LLD, but
I'm concerned that we would suffer a perf hit. Processing compact unwind
is already expensive, and that's a simpler format than EH frames. Given
that MC currently produces one EH frame entry for every compact unwind
entry, I don't think processing them will be cheap. I tried to do
something clever on LLD's end to drop the unnecessary EH frames at parse
time, but this made the code significantly more complex. So I'm looking
at fixing this at the MC level instead.

Code-wise, I'm a bit uncertain about the right approach here. A good
number of tests are failing. Some of them are tests that expected EH
frames to always be generated, and I figure we have two options here:
change the tests to have unwind info that can't be encoded with compact
unwind, or pass a flag to force EH frames to always be emitted. I've
done the former because it seems slightly easier than figuring out what
each test was supposed to do.

The other source of failure appears to stem from `lli` failing to catch
exceptions. Based on this comment <https://github.com/llvm/llvm-project/blob/bfd0cbd4eb9fef85da96bb799038ba49a52d08f8/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp#L713>,
I believe the root cause is that libunwind is unable to load compact
unwind dynamically. (I believe this limitation applies not just to MCJIT but to
ORC and the regular interpreter as well -- @lhames, would you be able to
confirm?)

In both cases, we need some way to tell `MCObjectFileInfo` to emit EH
frames. I'm currently doing that via
`MCContext::getGenDwarfForAssembly`. I'm not super familiar with the MC
code, though, so I'm wondering if I should use a different flag, or if I
should even be using `MCContext` here. Pointers will be appreciated!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122258

Files:
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
  llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
  llvm/lib/MC/MCDwarf.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/ExecutionEngine/MCJIT/eh-lg-pic.ll
  llvm/test/ExecutionEngine/MCJIT/eh.ll
  llvm/test/ExecutionEngine/MCJIT/multi-module-eh-a.ll
  llvm/test/ExecutionEngine/MCJIT/remote/eh.ll
  llvm/test/ExecutionEngine/OrcLazy/minimal-throw-catch.ll
  llvm/test/ExecutionEngine/RuntimeDyld/X86/MachO_x86-64_PIC_relocations.s
  llvm/test/MC/ELF/cfi-version.ll
  llvm/test/MC/X86/i386-darwin-frame-register.ll
  llvm/test/tools/dsymutil/X86/frame-1.test
  llvm/test/tools/dsymutil/X86/frame-2.test
  llvm/test/tools/llvm-dwarfdump/X86/debug_frame_offset.test
  llvm/tools/llc/llc.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122258.417390.patch
Type: text/x-patch
Size: 11920 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220322/4a1371c1/attachment-0001.bin>