[PATCH] D73739: Exception support for basic block sections

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1133-1134
+        if (MBB.isBeginSection() && MBB.isEHPad() &&
+            ((*std::prev(MI.getIterator())).getOpcode() ==
+             TargetOpcode::CFI_INSTRUCTION)) {
+          // Emit a NOP here to avoid zero-offset landing pads with
----------------
Is there a specific case that this check is necessary for or will the previous 2 suffice?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp:636-653
+      if (HaveTTData) {
+        // N.B.: There is a dependency loop between the size of the TTBase
+        // uleb128 here and the amount of padding before the aligned type table.
+        // The assembler must sometimes pad this uleb128 or insert extra padding
+        // before the type table. See PR35809 or GNU as bug 4029.
+        MCSymbol *TTBaseRefLabel = Asm->createTempSymbol("ttbaseref");
+        Asm->emitLabelDifferenceAsULEB128(TTBaseLabel, TTBaseRefLabel);
----------------
This and the its exact copy above (in `if (IsSJLJ || IsWasm) {`) IMO should be combined into a function/lambda to ensure they stay in sync.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/EHStreamer.h:73
+  /// Structure describing a contiguous range of call-sites which reside
+  /// in the same procedure fragment (BB section).
+  struct CallSiteRange {
----------------
MaskRay wrote:
> The canonical name has been changed to basic block sections (not "BB section") if I remember correctly.
> You can just write -fbasic-block-sections= to make readers know this is related to -fbasic-block-sections=
This structure is also used for all cases now not just basic block sections so is the annotation necessary?


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

https://reviews.llvm.org/D73739