[PATCH] D78778: [AMDGPU] Add SupportsDebugOnlyCFI to MCAsmInfo
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 2 12:16:50 PDT 2021
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
More information about the llvm-commits
mailing list