Extract the method to begin and end a fragment in AsmPrinterHandler in their own method. NFC

[deadalnix]

Summary

This is extracted from D17555

[davidxl]

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

46

If this is the final override, you can probably call it with

DwarfCFIExeptionBase::endFragment();

88

nit: remove Regular

136

unnecessary code movement -- move endFunction down after endFragment

158

Move this assertion up and make it assert (F->hasPersonalityFn() && "Expect ... ");

lib/CodeGen/AsmPrinter/DwarfException.h

67

Should this and the end function be declared in DwarfCFIExceptionBase? Note that there are two overrider classes to the Base class.

[deadalnix]

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

136

I did not move this. It always was just after beginFunction. I can reorder to keep this out of the diff.

158

P is used down the road, so I'm not sure this makes sense.

lib/CodeGen/AsmPrinter/DwarfException.h

67

There are defined as doing nothing in AsmPrinterHandler, so we are good here.

[deadalnix]

Address various comments

[davidxl]

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

158

if you do not assert F->hasPersonalityFn(), can F->getPersonalityFn() possibly return NULL -- then you will get segfault which is less desirable than asserting.

lib/CodeGen/AsmPrinter/DwarfException.h

67

./lib/CodeGen/AsmPrinter/ARMException.cpp:39:ARMException::ARMException

ARMException::ARMException(AsmPrinter *A) : DwarfCFIExceptionBase(A) { ..)

ARMException class also derives from DwarfExceptionBase. If beginFragment()/endFragment() is not pushed into the base class, this class will have inherited the empty definition which is wrong.

Correction -- only endFragment part is wrong -- endFragment is called in the base class implementation ...

[deadalnix]

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

158

shouldEmitPersonality already gates this, see line 115

lib/CodeGen/AsmPrinter/DwarfException.h

67

Good catch. Let em fix this.

[deadalnix]

Move endFragment to DwarfCFIExceptionBase

[davidxl]

looks like a good NFC clean up. LGTM

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

56

add forceEmitPersonality field here.

lib/CodeGen/AsmPrinter/AsmPrinterHandler.h

 
 #ifndef LLVM_LIB_CODEGEN_ASMPRINTER_ASMPRINTERHANDLER_H
 #define LLVM_LIB_CODEGEN_ASMPRINTER_ASMPRINTERHANDLER_H
 
 #include "llvm/Support/DataTypes.h"
 
 namespace llvm {
 
+class AsmPrinter;
 class MachineBasicBlock;
 class MachineFunction;
 class MachineInstr;
 class MCSymbol;
 
+typedef MCSymbol *ExceptionSymbolProvider(AsmPrinter *Asm);
+
 /// \brief Collects and handles AsmPrinter objects required to build debug
 /// or EH information.
 class AsmPrinterHandler {
 public:
   virtual ~AsmPrinterHandler();
 
   /// \brief For symbols that have a size designated (e.g. common symbols),
   /// this tracks that size.
public:
   // before endFunction and cannot switch sections.
   virtual void markFunctionEnd();
 
   /// \brief Gather post-function debug information.
   /// Please note that some AsmPrinter implementations may not call
   /// beginFunction at all.
   virtual void endFunction(const MachineFunction *MF) = 0;
 
+  virtual void beginFragment(const MachineBasicBlock *MBB,
+                             ExceptionSymbolProvider ESP) {}
+  virtual void endFragment() {}
+
   /// \brief Emit target-specific EH funclet machinery.
   virtual void beginFunclet(const MachineBasicBlock &MBB,
                             MCSymbol *Sym = nullptr) {}
   virtual void endFunclet() {}
 
   /// \brief Process beginning of an instruction.
   virtual void beginInstruction(const MachineInstr *MI) = 0;
 
   /// \brief Process end of an instruction.
   virtual void endInstruction() = 0;
 };
 } // End of namespace llvm
 
 #endif

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 using namespace llvm;
 
 DwarfCFIExceptionBase::DwarfCFIExceptionBase(AsmPrinter *A)
     : EHStreamer(A), shouldEmitCFI(false) {}
 
 void DwarfCFIExceptionBase::markFunctionEnd() {
-  if (shouldEmitCFI)
-    Asm->OutStreamer->EmitCFIEndProc();
+  endFragment();
 
   if (MMI->getLandingPads().empty())
     return;
 
   // Map all labels and get rid of any dead landing pads.
   MMI->TidyLandingPads();
 }
 
 DwarfCFIException::DwarfCFIException(AsmPrinter *A)
     : DwarfCFIExceptionBase(A), shouldEmitPersonality(false),
       shouldEmitLSDA(false), shouldEmitMoves(false),
       moveTypeModule(AsmPrinter::CFI_M_None) {}
 
 DwarfCFIException::~DwarfCFIException() {}
 
 /// endModule - Emit all exception information that should come after the
 /// content.
 void DwarfCFIException::endModule() {
void DwarfCFIException::endModule() {
   for (const Function *Personality : MMI->getPersonalities()) {
     if (!Personality)
       continue;
     MCSymbol *Sym = Asm->getSymbol(Personality);
     TLOF.emitPersonalityValue(*Asm->OutStreamer, Asm->getDataLayout(), Sym);
   }
 }
 
+static MCSymbol *getRegularExceptionSym(AsmPrinter *Asm) {
+  return Asm->getCurExceptionSym();
+}
+
 void DwarfCFIException::beginFunction(const MachineFunction *MF) {
   shouldEmitMoves = shouldEmitPersonality = shouldEmitLSDA = false;
   const Function *F = MF->getFunction();
 
   // If any landing pads survive, we need an EH table.
   bool hasLandingPads = !MMI->getLandingPads().empty();
 
   // See if we need frame move info.
   AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves();
   if (MoveType == AsmPrinter::CFI_M_EH ||
       (MoveType == AsmPrinter::CFI_M_Debug &&
        moveTypeModule == AsmPrinter::CFI_M_None))
     moveTypeModule = MoveType;
 
   shouldEmitMoves = MoveType != AsmPrinter::CFI_M_None;
 
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
   unsigned PerEncoding = TLOF.getPersonalityEncoding();
   const Function *Per = nullptr;
   if (F->hasPersonalityFn())
     Per = dyn_cast<Function>(F->getPersonalityFn()->stripPointerCasts());
 
   // Emit a personality function even when there are no landing pads
-  bool forceEmitPersonality =
+  forceEmitPersonality =
       // ...if a personality function is explicitly specified
       F->hasPersonalityFn() &&
       // ... and it's not known to be a noop in the absence of invokes
       !isNoOpWithoutInvoke(classifyEHPersonality(Per)) &&
       // ... and we're not explicitly asked not to emit it
       F->needsUnwindTableEntry();
 
   shouldEmitPersonality =
       (forceEmitPersonality ||
        (hasLandingPads && PerEncoding != dwarf::DW_EH_PE_omit)) &&
       Per;
 
   unsigned LSDAEncoding = TLOF.getLSDAEncoding();
   shouldEmitLSDA = shouldEmitPersonality &&
     LSDAEncoding != dwarf::DW_EH_PE_omit;
 
   shouldEmitCFI = shouldEmitPersonality || shouldEmitMoves;
+  beginFragment(&*MF->begin(), getRegularExceptionSym);
+}
+
+/// endFunction - Gather and emit post-function exception information.
+///
+void DwarfCFIException::endFunction(const MachineFunction *) {
+  if (!shouldEmitPersonality)
+    return;
+
+  emitExceptionTable();
+}
+
+void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
+                                      ExceptionSymbolProvider ESP) {
   if (!shouldEmitCFI)
     return;
 
   Asm->OutStreamer->EmitCFIStartProc(/*IsSimple=*/false);
 
   // Indicate personality routine, if any.
   if (!shouldEmitPersonality)
     return;
 
+  auto *F = MBB->getParent()->getFunction();
+  auto *P = dyn_cast<Function>(F->getPersonalityFn()->stripPointerCasts());
+  assert(P && "Expected personality function");
+
   // If we are forced to emit this personality, make sure to record
   // it because it might not appear in any landingpad
   if (forceEmitPersonality)
-    MMI->addPersonality(Per);
+    MMI->addPersonality(P);
 
+  const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
+  unsigned PerEncoding = TLOF.getPersonalityEncoding();
   const MCSymbol *Sym =
-      TLOF.getCFIPersonalitySymbol(Per, *Asm->Mang, Asm->TM, MMI);
+      TLOF.getCFIPersonalitySymbol(P, *Asm->Mang, Asm->TM, MMI);
   Asm->OutStreamer->EmitCFIPersonality(Sym, PerEncoding);
 
   // Provide LSDA information.
-  if (!shouldEmitLSDA)
-    return;
-
-  Asm->OutStreamer->EmitCFILsda(Asm->getCurExceptionSym(), LSDAEncoding);
+  if (shouldEmitLSDA)
+    Asm->OutStreamer->EmitCFILsda(ESP(Asm), TLOF.getLSDAEncoding());
 }
 
-/// endFunction - Gather and emit post-function exception information.
-///
-void DwarfCFIException::endFunction(const MachineFunction *) {
-  if (!shouldEmitPersonality)
-    return;
-
-  emitExceptionTable();
+void DwarfCFIException::endFragment() {
+  if (shouldEmitCFI)
+    Asm->OutStreamer->EmitCFIEndProc();
 }

lib/CodeGen/AsmPrinter/DwarfException.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_CODEGEN_ASMPRINTER_DWARFEXCEPTION_H
 #define LLVM_LIB_CODEGEN_ASMPRINTER_DWARFEXCEPTION_H
 
 #include "EHStreamer.h"
 #include "llvm/CodeGen/AsmPrinter.h"
+#include "llvm/MC/MCDwarf.h"
 
 namespace llvm {
 class MachineFunction;
 class ARMTargetStreamer;
 
 class LLVM_LIBRARY_VISIBILITY DwarfCFIExceptionBase : public EHStreamer {
 protected:
   DwarfCFIExceptionBase(AsmPrinter *A);
 
   /// Per-function flag to indicate if frame CFI info should be emitted.
   bool shouldEmitCFI;
 
   void markFunctionEnd() override;
 };
 
 class LLVM_LIBRARY_VISIBILITY DwarfCFIException : public DwarfCFIExceptionBase {
   /// Per-function flag to indicate if .cfi_personality should be emitted.
   bool shouldEmitPersonality;
 
+  /// Per-function flag to indicate if .cfi_personality must be emitted.
+  bool forceEmitPersonality;
+
   /// Per-function flag to indicate if .cfi_lsda should be emitted.
   bool shouldEmitLSDA;
 
   /// Per-function flag to indicate if frame moves info should be emitted.
   bool shouldEmitMoves;
 
   AsmPrinter::CFIMoveType moveTypeModule;
 
 public:
   //===--------------------------------------------------------------------===//
   // Main entry points.
   //
   DwarfCFIException(AsmPrinter *A);
   ~DwarfCFIException() override;
 
   /// Emit all exception information that should come after the content.
   void endModule() override;
 
   /// Gather pre-function exception information.  Assumes being emitted
   /// immediately after the function entry point.
   void beginFunction(const MachineFunction *MF) override;
 
   /// Gather and emit post-function exception information.
   void endFunction(const MachineFunction *) override;
+
+  void beginFragment(const MachineBasicBlock *MBB,
+                     ExceptionSymbolProvider ESP) override;
+  void endFragment() override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ARMException : public DwarfCFIExceptionBase {
   void emitTypeInfos(unsigned TTypeEncoding) override;
   ARMTargetStreamer &getTargetStreamer();
 
 public:
   //===--------------------------------------------------------------------===//

lib/CodeGen/AsmPrinter/EHStreamer.h

 #include "AsmPrinterHandler.h"
 #include "llvm/ADT/DenseMap.h"
 
 namespace llvm {
 struct LandingPadInfo;
 class MachineModuleInfo;
 class MachineInstr;
 class MachineFunction;
-class AsmPrinter;
 class MCSymbol;
 class MCSymbolRefExpr;
 
 template <typename T>
 class SmallVectorImpl;
 
 /// Emits exception handling directives.
 class LLVM_LIBRARY_VISIBILITY EHStreamer : public AsmPrinterHandler {