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

[Jez Ng via Phabricator via cfe-commits]

int3 added a comment.

I'm currently working on the eh_frame support in LLD itself, but will circle back to this diff in a week or so. Thanks for the feedback!



================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:48
 
+// FIXME
 TEST(InterpreterTest, CatchException) {
----------------
lhames wrote:
> I guess this is just a placeholder? The final commit should include more detail.
yep, just a placeholder


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp:131-132
+                                           size_t Size) {
+  if (Size == 0)
+    return;
   registerEHFramesInProcess(Addr, Size);
----------------
lhames wrote:
> Is this needed? With your change, registerEHFramesInProcess should be a no-op if `Size == 0`.
might not be yeah. I wasn't sure whether the extra entry in `EHFrames` would be an issue, but I will double check


================
Comment at: llvm/tools/llc/llc.cpp:697-700
+    auto &Ctx = MMIWP->getMMI().getContext();
+    Ctx.setGenDwarfForAssembly(Target->Options.ForceDwarfFrameSection);
     const_cast<TargetLoweringObjectFile *>(LLVMTM.getObjFileLowering())
+        ->Initialize(Ctx, *Target);
----------------
lhames wrote:
> This option hand-off feels very manual. I wonder if we should add a `propagateTargetOptionsToMC` function, but maybe the set of options that it would apply to is small enough not to bother for now?
> 
> I would add something like this to `LLVMTargetMachine::addPassesToEmitMC` too, for the the case where the caller passes in a null `MCContext*`. 
thanks for the pointers! I'll look into that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122258