[CodeGen] Remove trailing dot of name of text section with prefix

[TaoPan]

Summary

TargetLoweringObjectFileELF::getSectionForMachineBasicBlock() may
connect name of text section with prefix, dot and symbol name, there may
be two dots between name of text section with prefix and symbol name.

[TaoPan]

Could you please have a review?

[TaoPan]

Add Summary

[pengfei]

It seems the last dot in .text.hot. was added intentionally. Add @MaskRay to have a look.

[MaskRay]

In D96219#2547649, @pengfei wrote:

It seems the last dot in .text.hot. was added intentionally. Add @MaskRay to have a look.

.text.hot. and .text.exit etc are intentional, otherwise with -ffunction-sections, a function named exit (in .text.exit) will be shuffled to a different place.

// Default ld.bfd --verbose
 .text           :
  {
    *(.text.unlikely .text.*_unlikely .text.unlikely.*)
    *(.text.exit .text.exit.*)
    *(.text.startup .text.startup.*)
    *(.text.hot .text.hot.*)
    *(SORT(.text.sorted.*))
    *(.text .stub .text.* .gnu.linkonce.t.*)
    /* .gnu.warning sections are handled specially by elf.em.  */
    *(.gnu.warning)
  }

[TaoPan]

Thank MaskRay for the review comment!
I don’t understand the intention of the last dot of “.text.hot.”, it’s “.text.${text-section-prefix}.”.
The example of function name exit with –ffunction-sections doesn’t include text-section-prefix, it’s “.text.${function-name}”.

[MaskRay]

In D96219#2550219, @TaoPan wrote:

Thank MaskRay for the review comment!
I don’t understand the intention of the last dot of “.text.hot.”, it’s “.text.${text-section-prefix}.”.
The example of function name exit with –ffunction-sections doesn’t include text-section-prefix, it’s “.text.${function-name}”.

Assuming normal function names don't end with ., their -ffunction-section section names can be distinguished from .text.${text-section-prefix}.

[TaoPan]

Thanks! I got the trailing dot is for distinguishing text section name with prefix and function name with --function-sections.
Is it ok adding comment to the code of adding trailing dot, like "For distinguishing between .text.${text-section-prefix}. (with trailing dot) and .text.${function-name}"?

I'll submit another patch to fix the two dots issue of Summary in the place that connects text section name with prefix and function name.

[MaskRay]

In D96219#2550331, @TaoPan wrote:

Thanks! I got the trailing dot is for distinguishing text section name with prefix and function name with --function-sections.
Is it ok adding comment to the code of adding trailing dot, like "For distinguishing between .text.${text-section-prefix}. (with trailing dot) and .text.${function-name}"?

I'll submit another patch to fix the two dots issue of Summary in the place that connects text section name with prefix and function name.

Clarification patch sounds good..

[TaoPan]

Thank MaskRay!
I submitted a new patch https://reviews.llvm.org/D96327 to add the comment and fix the issue of Summary.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

if (Kind.isMergeableCString()) {
     Name = SizeSpec + utostr(Alignment.value());
   } else if (Kind.isMergeableConst()) {
     Name = ".rodata.cst";
     Name += utostr(EntrySize);
   } else {
     Name = getSectionPrefixForGlobal(Kind);
   }
 
-  bool HasPrefix = false;
   if (const auto *F = dyn_cast<Function>(GO)) {
     if (Optional<StringRef> Prefix = F->getSectionPrefix()) {
       raw_svector_ostream(Name) << '.' << *Prefix;
-      HasPrefix = true;
     }
   }
 
   if (UniqueSectionName) {
     Name.push_back('.');
     TM.getNameWithPrefix(Name, GO, Mang, /*MayAlwaysUsePrivate*/true);
-  } else if (HasPrefix)
-    Name.push_back('.');
+  }
   return Name;
 }
 
 namespace {
 class LoweringDiagnosticInfo : public DiagnosticInfo {
   const Twine &Msg;
 
 public:

llvm/test/CodeGen/X86/hot-unlikely-section-prefix.ll

 ; Test hot or unlikely section postfix based on profile and user annotation.
 ; RUN: llc < %s | FileCheck %s
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: inlinehint norecurse nounwind readnone uwtable
 define dso_local i32 @hot1() #0 !prof !31 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.hot.,"ax",@progbits
+; CHECK: .section        .text.hot,"ax",@progbits
 ; CHECK: .globl  hot1
 
 ; Function Attrs: cold norecurse nounwind readnone uwtable
 define dso_local i32 @cold1() #1 !prof !32 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.unlikely.,"ax",@progbits
+; CHECK: .section        .text.unlikely,"ax",@progbits
 ; CHECK: .globl  cold1
 
 ; Function Attrs: cold inlinehint noinline norecurse nounwind optsize readnone uwtable
 define dso_local i32 @hot2() #2 !prof !31 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.hot.,"ax",@progbits
+; CHECK: .section        .text.hot,"ax",@progbits
 ; CHECK: .globl  hot2
 
 define dso_local i32 @normal() {
 entry:
   ret i32 1
 }
 ; CHECK: text
 ; CHECK: .globl  normal
 
 ; Function Attrs: hot noinline norecurse nounwind readnone uwtable
 define dso_local i32 @hot3() #3 !prof !32 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.hot.,"ax",@progbits
+; CHECK: .section        .text.hot,"ax",@progbits
 ; CHECK: .globl  hot3
 
 ; Function Attrs: cold noinline norecurse nounwind optsize readnone uwtable
 define dso_local i32 @cold2() #4 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.unlikely.,"ax",@progbits
+; CHECK: .section        .text.unlikely,"ax",@progbits
 ; CHECK: .globl  cold2
 
 ; Function Attrs: hot noinline norecurse nounwind readnone uwtable
 define dso_local i32 @hot4() #3 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.hot.,"ax",@progbits
+; CHECK: .section        .text.hot,"ax",@progbits
 ; CHECK: .globl  hot4
 
 attributes #0 = { inlinehint norecurse nounwind readnone uwtable }
 attributes #1 = { cold norecurse nounwind readnone uwtable }
 attributes #2 = { cold inlinehint noinline norecurse nounwind optsize readnone uwtable }
 attributes #3 = { hot noinline norecurse nounwind readnone uwtable }
 attributes #4 = { cold noinline norecurse nounwind optsize readnone uwtable }
 

llvm/test/CodeGen/X86/text-section-prefix.ll

 ; RUN: llc -mtriple x86_64-linux-gnu -function-sections %s -o - | FileCheck %s --check-prefix=ELF
 ; RUN: llc -mtriple x86_64-linux-gnu -unique-section-names=0 -function-sections %s -o - | FileCheck %s --check-prefix=ELF-NOUNIQ
 ; RUN: llc -mtriple x86_64-windows-msvc -function-sections %s -o - | FileCheck %s --check-prefix=COFF-MSVC
 ; RUN: llc -mtriple x86_64-windows-gnu -function-sections %s -o - | FileCheck %s --check-prefix=COFF-GNU
 
 define void @foo1(i1 zeroext %0) nounwind !section_prefix !0 {
 ;; CHECK hot section name
 ; ELF:        .section  .text.hot.foo1,"ax",@progbits
-; ELF-NOUNIQ: .section  .text.hot.,"ax",@progbits,unique,1
+; ELF-NOUNIQ: .section  .text.hot,"ax",@progbits,unique,1
 ; COFF-MSVC:  .section  .text$hot,"xr",one_only,foo1
 ; COFF-GNU:   .section  .text$hot$foo1,"xr",one_only,foo1
   ret void
 }
 
 define void @foo2(i1 zeroext %0) nounwind !section_prefix !1 {
 ;; CHECK unlikely section name
 ; ELF:        .section  .text.unlikely.foo2,"ax",@progbits
-; ELF-NOUNIQ: .section  .text.unlikely.,"ax",@progbits,unique,2
+; ELF-NOUNIQ: .section  .text.unlikely,"ax",@progbits,unique,2
 ; COFF-MSVC:  .section  .text$unlikely,"xr",one_only,foo2
 ; COFF-GNU:   .section  .text$unlikely$foo2,"xr",one_only,foo2
   ret void
 }
 
 !0 = !{!"function_section_prefix", !"hot"}
 !1 = !{!"function_section_prefix", !"unlikely"}
 

llvm/test/MC/AsmParser/function_hot_attr.ll

 ; Test hot function attribute
 ; RUN: llc < %s | FileCheck %s
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: hot noinline norecurse nounwind readnone uwtable
 define dso_local i32 @hot4() #0 {
 entry:
   ret i32 1
 }
-; CHECK: .section        .text.hot.,"ax",@progbits
+; CHECK: .section        .text.hot,"ax",@progbits
 ; CHECK: .globl  hot4
 
 attributes #0 = { hot noinline norecurse nounwind readnone uwtable }

llvm/test/Transforms/CodeGenPrepare/X86/section.ll

 ; RUN: opt < %s -codegenprepare -S | FileCheck %s
 ; RUN: llc < %s | FileCheck --check-prefix=ASM1 %s
 ; RUN: llc < %s -function-sections | FileCheck --check-prefix=ASM2 %s
 
 target triple = "x86_64-pc-linux-gnu"
 
 ; This tests that hot/cold functions get correct section prefix assigned
 
 ; CHECK: hot_func1{{.*}}!section_prefix ![[HOT_ID:[0-9]+]]
-; ASM1: .section .text.hot.,"ax",@progbits
+; ASM1: .section .text.hot,"ax",@progbits
 ; ASM2: .section .text.hot.hot_func1,"ax",@progbits
 ; The entry is hot
 define void @hot_func1() !prof !15 {
   ret void
 }
 
 ; CHECK: hot_func2{{.*}}!section_prefix ![[HOT_ID:[0-9]+]]
 ; Entry is cold but inner block is hot

 for.end:
   ret void
 }
 
 ; For instrumentation based PGO, we should only look at block counts,
 ; not call site VP metadata (which can exist on value profiled memcpy,
 ; or possibly left behind after static analysis based devirtualization).
 ; CHECK: cold_func1{{.*}}!section_prefix ![[COLD_ID:[0-9]+]]
-; ASM1: .section .text.unlikely.,"ax",@progbits
+; ASM1: .section .text.unlikely,"ax",@progbits
 ; ASM2: .section .text.unlikely.cold_func1,"ax",@progbits
 define void @cold_func1() !prof !16 {
   call void @hot_func1(), !prof !17
   call void @hot_func1(), !prof !17
   ret void
 }
 
 ; CHECK: cold_func2{{.*}}!section_prefix ![[COLD_ID]]