From 0ce723cb228bc1d1a0f5718f3862fb836145a333 Mon Sep 17 00:00:00 2001 From: RamNalamothu Date: Sat, 24 Apr 2021 23:26:52 +0530 Subject: [PATCH] [NFC] Refactor how CFI section types are represented in AsmPrinter In terms of readability, the `enum CFIMoveType` didn't better document what it intends to convey i.e. the type of CFI section that gets emitted. Reviewed By: dblaikie, MaskRay Differential Revision: https://reviews.llvm.org/D76519 --- llvm/include/llvm/CodeGen/AsmPrinter.h | 23 +++++++--- llvm/lib/CodeGen/AsmPrinter/ARMException.cpp | 8 ++-- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 43 +++++++++++-------- .../CodeGen/AsmPrinter/DwarfCFIException.cpp | 17 +++++--- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 2 +- 5 files changed, 56 insertions(+), 37 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h index 52b5ecd1a3bb..2c65a344e384 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinter.h +++ b/llvm/include/llvm/CodeGen/AsmPrinter.h @@ -157,6 +157,13 @@ public: TimerGroupDescription(TimerGroupDescription) {} }; + // Flags representing which CFI section is required for a function/module. + enum class CFISection : unsigned { + None = 0, ///< Do not emit either .eh_frame or .debug_frame + EH = 1, ///< Emit .eh_frame + Debug = 2 ///< Emit .debug_frame + }; + private: MCSymbol *CurrentFnEnd = nullptr; @@ -199,8 +206,8 @@ private: /// context. PseudoProbeHandler *PP = nullptr; - /// If the current module uses dwarf CFI annotations strictly for debugging. - bool isCFIMoveForDebugging = false; + /// CFISection type the module needs i.e. either .eh_frame or .debug_frame. + CFISection ModuleCFISection = CFISection::None; protected: explicit AsmPrinter(TargetMachine &TM, std::unique_ptr Streamer); @@ -357,12 +364,14 @@ public: void emitRemarksSection(remarks::RemarkStreamer &RS); - enum CFIMoveType { CFI_M_None, CFI_M_EH, CFI_M_Debug }; - CFIMoveType needsCFIMoves() const; + /// Get the CFISection type for a function. + CFISection getFunctionCFISectionType(const Function &F) const; - /// Returns false if needsCFIMoves() == CFI_M_EH for any function - /// in the module. - bool needsOnlyDebugCFIMoves() const { return isCFIMoveForDebugging; } + /// Get the CFISection type for a function. + CFISection getFunctionCFISectionType(const MachineFunction &MF) const; + + /// Get the CFISection type for the module. + CFISection getModuleCFISectionType() const { return ModuleCFISection; } bool needsSEHMoves(); diff --git a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp index b634b24377fe..db4215e92d44 100644 --- a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp @@ -39,13 +39,13 @@ void ARMException::beginFunction(const MachineFunction *MF) { if (Asm->MAI->getExceptionHandlingType() == ExceptionHandling::ARM) getTargetStreamer().emitFnStart(); // See if we need call frame info. - AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves(); - assert(MoveType != AsmPrinter::CFI_M_EH && + AsmPrinter::CFISection CFISecType = Asm->getFunctionCFISectionType(*MF); + assert(CFISecType != AsmPrinter::CFISection::EH && "non-EH CFI not yet supported in prologue with EHABI lowering"); - if (MoveType == AsmPrinter::CFI_M_Debug) { + if (CFISecType == AsmPrinter::CFISection::Debug) { if (!hasEmittedCFISections) { - if (Asm->needsOnlyDebugCFIMoves()) + if (Asm->getModuleCFISectionType() == AsmPrinter::CFISection::Debug) Asm->OutStreamer->emitCFISections(false, true); hasEmittedCFISections = true; } diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 0caf96411e27..eb7c7e2797d4 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -351,21 +351,18 @@ bool AsmPrinter::doInitialization(Module &M) { case ExceptionHandling::SjLj: case ExceptionHandling::DwarfCFI: 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; + for (auto &F : M.getFunctionList()) { + // If any function needsUnwindTableEntry(), it needs .eh_frame and hence + // the module needs .eh_frame. If we have found that case, we are done. + if (ModuleCFISection == AsmPrinter::CFISection::EH) break; - } + if (ModuleCFISection == AsmPrinter::CFISection::None) + ModuleCFISection = getFunctionCFISectionType(F); } + assert(MAI->getExceptionHandlingType() == ExceptionHandling::DwarfCFI || + ModuleCFISection != AsmPrinter::CFISection::EH); break; default: - isCFIMoveForDebugging = false; break; } @@ -1037,15 +1034,25 @@ static bool emitDebugLabelComment(const MachineInstr *MI, AsmPrinter &AP) { return true; } -AsmPrinter::CFIMoveType AsmPrinter::needsCFIMoves() const { +AsmPrinter::CFISection +AsmPrinter::getFunctionCFISectionType(const Function &F) const { + // Ignore functions that won't get emitted. + if (F.isDeclarationForLinker()) + return CFISection::None; + if (MAI->getExceptionHandlingType() == ExceptionHandling::DwarfCFI && - MF->getFunction().needsUnwindTableEntry()) - return CFI_M_EH; + F.needsUnwindTableEntry()) + return CFISection::EH; - if (MMI->hasDebugInfo() || MF->getTarget().Options.ForceDwarfFrameSection) - return CFI_M_Debug; + if (MMI->hasDebugInfo() || TM.Options.ForceDwarfFrameSection) + return CFISection::Debug; - return CFI_M_None; + return CFISection::None; +} + +AsmPrinter::CFISection +AsmPrinter::getFunctionCFISectionType(const MachineFunction &MF) const { + return getFunctionCFISectionType(MF.getFunction()); } bool AsmPrinter::needsSEHMoves() { @@ -1058,7 +1065,7 @@ void AsmPrinter::emitCFIInstruction(const MachineInstr &MI) { ExceptionHandlingType != ExceptionHandling::ARM) return; - if (needsCFIMoves() == CFI_M_None) + if (getFunctionCFISectionType(*MF) == AsmPrinter::CFISection::None) return; // If there is no "real" instruction following this CFI instruction, skip diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp index 4b08e7d28468..ae1c80141ff9 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp @@ -93,9 +93,8 @@ void DwarfCFIException::beginFunction(const MachineFunction *MF) { bool hasLandingPads = !MF->getLandingPads().empty(); // See if we need frame move info. - AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves(); - - bool shouldEmitMoves = MoveType != AsmPrinter::CFI_M_None; + bool shouldEmitMoves = + Asm->getFunctionCFISectionType(*MF) != AsmPrinter::CFISection::None; const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering(); unsigned PerEncoding = TLOF.getPersonalityEncoding(); @@ -132,10 +131,14 @@ void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB, return; if (!hasEmittedCFISections) { - if (Asm->needsOnlyDebugCFIMoves()) - Asm->OutStreamer->emitCFISections(false, true); - else if (Asm->TM.Options.ForceDwarfFrameSection) - Asm->OutStreamer->emitCFISections(true, true); + AsmPrinter::CFISection CFISecType = Asm->getModuleCFISectionType(); + // If we don't say anything it implies `.cfi_sections .eh_frame`, so we + // chose not to be verbose in that case. And with `ForceDwarfFrameSection`, + // we should always emit .debug_frame. + if (CFISecType == AsmPrinter::CFISection::Debug || + Asm->TM.Options.ForceDwarfFrameSection) + Asm->OutStreamer->emitCFISections( + CFISecType == AsmPrinter::CFISection::EH, true); hasEmittedCFISections = true; } diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index 3373e6c91b7f..b7b5bf33b9e6 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -1229,7 +1229,7 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) { ExceptionHandlingType != ExceptionHandling::ARM) return; - if (needsCFIMoves() == CFI_M_None) + if (getFunctionCFISectionType(*MF) == CFISection::None) return; OutStreamer->emitCFIBKeyFrame();