[PATCH] D78778: [AMDGPU] Add SupportsDebugOnlyCFI to MCAsmInfo

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:51-55
+enum CFISection : unsigned {
+  None = 0, ///< Do not emit both .eh_frame and .debug_frame
+  EH = 1,   ///< Emit .eh_frame
+  Debug = 2 ///< Emit .debug_frame
+};
----------------
Sorry, paging this all back in. Why is this necessary? THis looks like the sort of change that would be necessary to allow /both/ eh_frame and debug_frame to be emitted at the same time. Is that what this is about? That doesn't seem to line up with my reading of the patch description, which seems to be about being able to emit debug_frame even when not using exceptions - but wouldn't require anything to say "I want eh_frame and debug_frame at the same time" I think?


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:52
+enum CFISection : unsigned {
+  None = 0, ///< Do not emit both .eh_frame and .debug_frame
+  EH = 1,   ///< Emit .eh_frame
----------------



================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:395
+  /// Defaults to false.
+  bool SupportsDebugOnlyCFI = false;
+
----------------
at least the naming of this variable seems a bit off to me

Could you explain a bit more why you had to "lie" to get debug_frame in the past? Which other systems exist/does LLVM support where SupportsDebugOnlyCFI would be problematic/wrong (if they opted out of exceptions/don't support exceptions)? Or could we just assume everyone's actually fine with debug_frame even if they don't have exceptions enabled?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:354
   case ExceptionHandling::ARM:
-    isCFIMoveForDebugging = true;
-    if (MAI->getExceptionHandlingType() != ExceptionHandling::DwarfCFI)
-      break;
-    for (auto &F: M.getFunctionList()) {
-      // If the module contains any function with unwind data,
-      // .eh_frame has to be emitted.
-      // Ignore functions that won't get emitted.
-      if (!F.isDeclarationForLinker() && F.needsUnwindTableEntry()) {
-        isCFIMoveForDebugging = false;
-        break;
-      }
-    }
+    for (auto &F : M.getFunctionList())
+      ModuleCFISections |= getCFISectionsToEmit(F);
----------------
Minor thing, I think this can be written more simply


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78778