[Propeller]: Use a descriptive temporary symbol name for the end of the basic block.

[rahmanl]

Summary

This patch changes the functionality of AsmPrinter to name the basic block end labels as LBB_END${i}_${j}, with ${i} being the identifier for the function and ${j} being the identifier for the basic block. The new naming scheme is consistent with how basic block labels are named (.LBB${i}_{j}), and how function end symbol are named (.Lfunc_end${i}) and helps to write stronger tests for the upcoming patch for BB-Info section (as proposed in https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html). The end label is used with basicblock-labels (BB-Info section in future) and basicblock-sections to compute the size of basic blocks and basic block sections, respectively. For BB sections, the section containing the entry basic block will not have a BB end label since it already gets the function end-label.
This label is cached for every basic block (CachedEndMCSymbol) like the label for the basic block (CachedMCSymbol).

[tmsriram]

Some inital comments. Expanding the summary to provide a bit more detail on why this symbol naming scheme is better might be useful. Thanks for doing this.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

177

The comment "Used during basic block sections..." is still relevant?

478

Can we name it getCachedEndMCSymbol?

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

3055

Remove this, it does not belong to this patch.

[rahmanl]

Remove unrelated changes.

[rahmanl]

llvm/include/llvm/CodeGen/MachineBasicBlock.h

478

This naming follows the similar function MachineBasicBlock::getSymbol() which also uses a cached symbol (called CachedMCSymbol).

[shenhan]

llvm/include/llvm/CodeGen/MachineBasicBlock.h

479

If all usage of getEndSymbol() accept a const version of "MCSymbol *", maybe change it to "const MCSymbol *" that goes along with the const nature of the function.

[rahmanl]

llvm/include/llvm/CodeGen/MachineBasicBlock.h

479

Unfortunately, that's not the case. MCObjectStreamer::emitLabel takes a non-const reference to a symbol (and actually mutates the symbol).

[snehasish]

LGTM

[rahmanl]

@eli.friedman and @MaskRay Does this small patch sound alright?

[MaskRay]

.Ltmp -> .LBB_END is a good idea. The assembly is more readable. There is no space hit because .L symbols are usually not emitted into the symbol table.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

479

If you delete`const` from getEndSymbol() , can you delete the mutable specifier on CachedEndMCSymbol?

[rahmanl]

Rebase.

[rahmanl]

llvm/include/llvm/CodeGen/MachineBasicBlock.h

479

That's possible. The current pattern is borrowed from MachineBasicBlock::getSymbol() and how it interacts with MachineBasicBlock::CachedMCSymbol*, which are respectively declared const and mutable. WDYT?

[tmsriram]

This patch is simple enough and LGTM.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

private:
 
   /// Indicate that this basic block is the indirect dest of an INLINEASM_BR.
   bool IsInlineAsmBrIndirectTarget = false;
 
   /// since getSymbol is a relatively heavy-weight operation, the symbol
   /// is only computed once and is cached.
   mutable MCSymbol *CachedMCSymbol = nullptr;
 
-  /// Used during basic block sections to mark the end of a basic block.
-  MCSymbol *EndMCSymbol = nullptr;
+  /// Marks the end of the basic block. Used during basic block sections to
+  /// calculate the size of the basic block, or the BB section ending with it.
+  mutable MCSymbol *CachedEndMCSymbol = nullptr;
 
   // Intrusive list support
   MachineBasicBlock() = default;
 
   explicit MachineBasicBlock(MachineFunction &MF, const BasicBlock *BB);
 
   ~MachineBasicBlock();
 
#endif
   unsigned getSectionIDNum() const {
     return ((unsigned)MBBSectionID::SectionType::Cold) -
            ((unsigned)SectionID.Type) + SectionID.Number;
   }
 
   /// Sets the section ID for this basic block.
   void setSectionID(MBBSectionID V) { SectionID = V; }
 
+  /// Returns the MCSymbol marking the end of this basic block.
+  MCSymbol *getEndSymbol() const;
+
   /// Returns true if this block may have an INLINEASM_BR (overestimate, by
   /// checking if any of the successors are indirect targets of any inlineasm_br
   /// in the function).
   bool mayHaveInlineAsmBr() const;
 
   /// Returns true if this is the indirect dest of an INLINEASM_BR.
   bool isInlineAsmBrIndirectTarget() const {
     return IsInlineAsmBrIndirectTarget;

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (auto &MI : MBB) {
           NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
                              HI.TimerGroupName, HI.TimerGroupDescription,
                              TimePassesIsEnabled);
           HI.Handler->endInstruction();
         }
       }
     }
 
-    // We need a temporary symbol for the end of this basic block, if either we
-    // have BBLabels enabled and we want to emit size directive for the BBs, or
-    // if this basic blocks marks the end of a section (except the section
+    // We must emit temporary symbol for the end of this basic block, if either
+    // we have BBLabels enabled and we want to emit size directive for the BBs,
+    // or if this basic blocks marks the end of a section (except the section
     // containing the entry basic block as the end symbol for that section is
     // CurrentFnEnd).
-    MCSymbol *CurrentBBEnd = nullptr;
     if ((MAI->hasDotTypeDotSizeDirective() && MF->hasBBLabels()) ||
-        (MBB.isEndSection() && !MBB.sameSection(&MF->front()))) {
-      CurrentBBEnd = OutContext.createTempSymbol();
-      OutStreamer->emitLabel(CurrentBBEnd);
-    }
+        (MBB.isEndSection() && !MBB.sameSection(&MF->front())))
+      OutStreamer->emitLabel(MBB.getEndSymbol());
 
     // Helper for emitting the size directive associated with a basic block
     // symbol.
     auto emitELFSizeDirective = [&](MCSymbol *SymForSize) {
-      assert(CurrentBBEnd && "Basicblock end symbol not set!");
       const MCExpr *SizeExp = MCBinaryExpr::createSub(
-          MCSymbolRefExpr::create(CurrentBBEnd, OutContext),
+          MCSymbolRefExpr::create(MBB.getEndSymbol(), OutContext),
           MCSymbolRefExpr::create(SymForSize, OutContext), OutContext);
       OutStreamer->emitELFSize(SymForSize, SizeExp);
     };
 
     // Emit size directive for the size of each basic block, if BBLabels is
     // enabled.
     if (MAI->hasDotTypeDotSizeDirective() && MF->hasBBLabels())
       emitELFSizeDirective(MBB.getSymbol());
 
     // Emit size directive for the size of each basic block section once we
     // get to the end of that section.
     if (MBB.isEndSection()) {
       if (!MBB.sameSection(&MF->front())) {
         if (MAI->hasDotTypeDotSizeDirective())
           emitELFSizeDirective(CurrentSectionBeginSym);
         MBBSectionRanges[MBB.getSectionIDNum()] =
-            MBBSectionRange{CurrentSectionBeginSym, CurrentBBEnd};
+            MBBSectionRange{CurrentSectionBeginSym, MBB.getEndSymbol()};
       }
     }
     emitBasicBlockEnd(MBB);
   }
 
   EmittedInsts += NumInstsInFunction;
   MachineOptimizationRemarkAnalysis R(DEBUG_TYPE, "InstructionCount",
                                       MF->getFunction().getSubprogram(),
if (Alignment != Align(1))
     emitAlignment(Alignment);
 
   // If the block has its address taken, emit any labels that were used to
   // reference the block.  It is possible that there is more than one label
   // here, because multiple LLVM BB's may have been RAUW'd to this block after
   // the references were generated.
   if (MBB.hasAddressTaken()) {
     const BasicBlock *BB = MBB.getBasicBlock();
     if (isVerbose())
       OutStreamer->AddComment("Block address taken");
 
     // MBBs can have their address taken as part of CodeGen without having
     // their corresponding BB's address taken in IR
     if (BB->hasAddressTaken())
       for (MCSymbol *Sym : MMI->getAddrLabelSymbolToEmit(BB))
         OutStreamer->emitLabel(Sym);
   }

llvm/lib/CodeGen/MachineBasicBlock.cpp

if (MF->hasBBLabels()) {
       CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" +
                                              Twine(MF->getFunctionNumber()) +
                                              "_" + Twine(getNumber()));
     }
   }
   return CachedMCSymbol;
 }
 
+MCSymbol *MachineBasicBlock::getEndSymbol() const {
+  if (!CachedEndMCSymbol) {
+    const MachineFunction *MF = getParent();
+    MCContext &Ctx = MF->getContext();
+    auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
+    CachedEndMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB_END" +
+                                              Twine(MF->getFunctionNumber()) +
+                                              "_" + Twine(getNumber()));
+  }
+  return CachedEndMCSymbol;
+}
 
 raw_ostream &llvm::operator<<(raw_ostream &OS, const MachineBasicBlock &MBB) {
   MBB.print(OS);
   return OS;
 }
 
 Printable llvm::printMBBReference(const MachineBasicBlock &MBB) {
   return Printable([&MBB](raw_ostream &OS) { return MBB.printAsOperand(OS); });

llvm/test/CodeGen/X86/basic-block-sections-clusters.ll

 declare i32 @bar() #1
 
 declare i32 @baz() #1
 
 ; LINUX-SECTIONS1:      .section  .text.foo,"ax",@progbits
 ; LINUX-SECTIONS1-NOT:    .section
 ; LINUX-SECTIONS1-LABEL:  foo:
 ; LINUX-SECTIONS1-NOT:    .section
+; LINUX-SECTIONS1-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS1-LABEL:  # %bb.2:
+; LINUX-SECTIONS1-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS1:    .section        .text.foo,"ax",@progbits,unique,1
 ; LINUX-SECTIONS1-LABEL:  foo.1:
-; LINUX-SECTIONS1-LABEL:  .Ltmp0:
-; LINUX-SECTIONS1-NEXT:        .size   foo.1, .Ltmp0-foo.1
+; LINUX-SECTIONS1-LABEL:  .LBB_END0_1:
+; LINUX-SECTIONS1-NEXT:        .size   foo.1, .LBB_END0_1-foo.1
 ; LINUX-SECTIONS1-NOT:    .section
 ; LINUX-SECTIONS1:    .section        .text.unlikely.foo,"ax",@progbits
 ; LINUX-SECTIONS1-LABEL:  foo.cold:
+; LINUX-SECTIONS1-LABEL:  .LBB_END0_3:
+; LINUX-SECTIONS1-NEXT:        .size   foo.cold, .LBB_END0_3-foo.cold
 ; LINUX-SECTIONS1:      .section  .text.foo,"ax",@progbits
 ; LINUX-SECTIONS1-LABEL:  .Lfunc_end0:
 ; LINUX-SECTIONS1-NEXT:   .size foo, .Lfunc_end0-foo
 
 ; LINUX-SECTIONS2:    .section        .text.foo,"ax",@progbits
 ; LINUX-SECTIONS2-NOT:    .section
 ; LINUX-SECTIONS2-LABEL:  foo:
+; LINUX-SECTIONS2-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS2-NOT:    .section
 ; LINUX-SECTIONS2-LABEL:  # %bb.2:
+; LINUX-SECTIONS2-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS2:    .section        .text.foo,"ax",@progbits,unique,1
 ; LINUX-SECTIONS2-NEXT:   foo.0:
+; LINUX-SECTIONS2-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS2-NOT:    .section
 ; LINUX-SECTIONS2-LABEL:  .LBB0_3:
-; LINUX-SECTIONS2-LABEL:  .Ltmp0:
-; LINUX-SECTIONS2-NEXT:        .size   foo.0, .Ltmp0-foo.0
+; LINUX-SECTIONS2-LABEL:  .LBB_END0_3:
+; LINUX-SECTIONS2-NEXT:        .size   foo.0, .LBB_END0_3-foo.0
 ; LINUX-SECTIONS2:    .section        .text.foo,"ax",@progbits
+; LINUX-SECTIONS2-NOT:    .LBB_END0_{{0-9}}+
 ; LINUX-SECTIONS2-LABEL:  .Lfunc_end0:
 ; LINUX-SECTIONS2-NEXT:   .size foo, .Lfunc_end0-foo

llvm/test/CodeGen/X86/basic-block-sections-listbb.ll

 ; Check that the correct block is found using the call insts for foo and bar.
 ;
 ; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb:
 ; Check that the basic block with id 1 doesn't get a section.
 ; LINUX-SECTIONS-NOT: .section        .text._Z3bazb._Z3bazb.{{[0-9]+}},"ax",@progbits
 ; LINUX-SECTIONS-LABEL: # %bb.1:
 ; LINUX-SECTIONS-NEXT:        callq   _Z3barv
-; LINUX-SECTIONS: .section        .text._Z3bazb._Z3bazb.{{[0-9]+}},"ax",@progbits
-; LINUX-SECTIONS-LABEL: _Z3bazb.{{[0-9]+}}:
+; LINUX-SECTIONS: .section        .text._Z3bazb.[[SECTION_LABEL:_Z3bazb.[0-9]+]],"ax",@progbits
+; LINUX-SECTIONS-NEXT: [[SECTION_LABEL]]:
 ; LINUX-SECTIONS-NEXT:        callq   _Z3foov
-; LINUX-SECTIONS: .Ltmp0:
-; LINUX-SECTIONS-NEXT: .size   _Z3bazb.{{[0-9]+}}, .Ltmp0-_Z3bazb.{{[0-9]+}}
+; LINUX-SECTIONS: .LBB_END0_2:
+; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL]], .LBB_END0_2-[[SECTION_LABEL]]
+; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
+; LINUX-SECTIONS: .Lfunc_end0:
+; LINUX-SECTIONS-NEXT: .size _Z3bazb, .Lfunc_end0-_Z3bazb

llvm/test/CodeGen/X86/basic-block-sections.ll

 
 declare i32 @_Z3foov() #1
 
 
 ; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb:
 ; LINUX-SECTIONS: .section        .text._Z3bazb._Z3bazb.1,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb.1:
-; LINUX-SECTIONS: .Ltmp0:
-; LINUX-SECTIONS-NEXT: .size   _Z3bazb.1, .Ltmp0-_Z3bazb.1
+; LINUX-SECTIONS: .LBB_END0_1:
+; LINUX-SECTIONS-NEXT: .size   _Z3bazb.1, .LBB_END0_1-_Z3bazb.1
 ; LINUX-SECTIONS: .section        .text._Z3bazb._Z3bazb.2,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb.2:
-; LINUX-SECTIONS: .Ltmp1:
-; LINUX-SECTIONS-NEXT: .size   _Z3bazb.2, .Ltmp1-_Z3bazb.2
+; LINUX-SECTIONS: .LBB_END0_2:
+; LINUX-SECTIONS-NEXT: .size   _Z3bazb.2, .LBB_END0_2-_Z3bazb.2

llvm/test/DebugInfo/X86/basic-block-sections_1.ll

 ; BB-SECTIONS-NEXT: [{{.*}}) ".text._Z3fooi.3"
 ; BB-SECTIONS-NEXT: [{{.*}}) ".text"
 ; BB-SECTIONS-ASM: _Z3fooi:
 ; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
 ; BB-SECTIONS-ASM-NEXT: .loc 1 2 9 prologue_end
 ; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
 ; BB-SECTIONS-ASM-NEXT: .loc 1 2 7 is_stmt
 ; BB-SECTIONS-ASM: _Z3fooi.1:
-; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
-; BB-SECTIONS-ASM: .size  _Z3fooi.1, .Ltmp{{[0-9]+}}-_Z3fooi.1
+; BB-SECTIONS-ASM: .LBB_END0_{{[0-9]+}}:
+; BB-SECTIONS-ASM: .size  _Z3fooi.1, .LBB_END0_{{[0-9]+}}-_Z3fooi.1
 ; BB-SECTIONS-ASM: _Z3fooi.2:
 ; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
-; BB-SECTIONS-ASM-NEXT: .Ltmp{{[0-9]+}}:
-; BB-SECTIONS-ASM: .size  _Z3fooi.2, .Ltmp{{[0-9]+}}-_Z3fooi.2
+; BB-SECTIONS-ASM-NEXT: .LBB_END0_{{[0-9]+}}:
+; BB-SECTIONS-ASM: .size  _Z3fooi.2, .LBB_END0_{{[0-9]+}}-_Z3fooi.2
 ; BB-SECTIONS-ASM: _Z3fooi.3:
 ; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
-; BB-SECTIONS-ASM-NEXT: .Ltmp{{[0-9]+}}:
-; BB-SECTIONS-ASM: .size  _Z3fooi.3, .Ltmp{{[0-9]+}}-_Z3fooi.3
+; BB-SECTIONS-ASM-NEXT: .LBB_END0_{{[0-9]+}}:
+; BB-SECTIONS-ASM: .size  _Z3fooi.3, .LBB_END0_{{[0-9]+}}-_Z3fooi.3
 ; BB-SECTIONS-ASM: .Lfunc_end0:
 ; BB-SECTIONS-ASM: .Ldebug_ranges0:
 ; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.1
-; BB-SECTIONS-ASM-NEXT: .quad .Ltmp{{[0-9]+}}
+; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
 ; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.2
-; BB-SECTIONS-ASM-NEXT: .quad .Ltmp{{[0-9]+}}
+; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
 ; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.3
-; BB-SECTIONS-ASM-NEXT: .quad .Ltmp{{[0-9]+}}
+; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
 ; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_begin0
 ; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_end0
 ; BB-SECTIONS-ASM-NEXT: .quad 0
 ; BB-SECTIONS-ASM-NEXT: .quad 0
 
 ; Function Attrs: noinline nounwind optnone uwtable
 define dso_local i32 @_Z3fooi(i32 %0) !dbg !7 {
   %2 = alloca i32, align 4