[PATCH] D89423: Explicitly check for entry basic block, rather than relying on MachineBasicBlock::pred_empty.

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3092
   // Print the main label for the block.
-  if (MBB.pred_empty() ||
-      (!MF->hasBBLabels() && isBlockOnlyReachableByFallthrough(&MBB) &&
-       !MBB.isEHFuncletEntry() && !MBB.hasLabelMustBeEmitted())) {
+  if ((!MBB.isEntryBlock() && (MF->hasBBLabels() || MBB.isBeginSection())) ||
+      (!MBB.pred_empty() &&
----------------
tmsriram wrote:
> I think this would require a lot more comments.  I see that you have flipped the if-else here, is there any reason you did that. Plus, there are a lot more checks than earlier.  Could you add  more comments that explain what is going on here?  Thanks.  
+1 perhaps even consider refactoring it out into a static helper function.


================
Comment at: llvm/test/CodeGen/X86/basic-block-sections-unreachable.ll:17
+; CHECK-SECTIONS:       .section .text,"ax", at progbits,unique,2
+; CHECK-SECTIONS-NEXT:  foo.2:       # %default
+}
----------------
Should we also check for the appropriate CFI directives?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89423