[PATCH] D146853: [Pseudo Probe] Placing .pseudoprobe section in the same comdat group with .txt

[Hongtao Yu via Phabricator via llvm-commits]

hoy created this revision.
Herald added subscribers: modimo, wenlei, hiraditya.
Herald added a project: All.
hoy requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Previously the .pseudoprobe data section of a function was supposed to be placed in its own comdat group so that it can de deduplicated against same-named data (such as weak symbols). Unfortunately a bug in the group name setting blocked such, therefore probes for different copies of the same function were not deduplicated and duplicated probes were reported for the final binary code.

Fixing the bug should unblock the deduplication. However, it may cause another issue where the final function code and the probe data are mismatched, because the code and the probes are deduplicated independently. I'm fixing this be placing the probes in the same comdat group as the code so that they can be deduplicated synchronously.

As an example below, with the current change we have section layout :

COMDAT group section [    3] `.group' [foo] contains 3 sections:

  [Index]    Name
  [    4]   .text.foo
  [    5]   .rela.text.foo
  [   17]   .pseudo_probe.foo

COMDAT group section [    6] `.group' [foo2] contains 3 sections:

  [Index]    Name
  [    7]   .text.foo2
  [    8]   .rela.text.foo2
  [   18]   .pseudo_probe.foo2

COMDAT group section [   10] `.group' [.pseudo_probe_desc_foo] contains 1 sections:

  [Index]    Name
  [   11]   .pseudo_probe_desc

COMDAT group section [   12] `.group' [.pseudo_probe_desc_foo2] contains 1 sections:

  [Index]    Name
  [   13]   .pseudo_probe_desc

Note that the probe desc data are still placed in separate groups because they are not bound to the final code unlike the probe data. Once a function is inlined, its probe data are included in the caller's probe data, but its probe desc is still there in separate. The probe descs do not need to be deduplicated with the code together.

Test Plan:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146853

Files:
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll


Index: llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll
===================================================================
--- llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll
+++ llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll
@@ -6,6 +6,7 @@
 ; RUN: FileCheck %s < %t1 --check-prefix=CHECK-ASM
 ; RUN: llc %t -function-sections -filetype=obj -o %t2
 ; RUN: llvm-objdump --section-headers  %t2 | FileCheck %s --check-prefix=CHECK-OBJ
+; RUN: llvm-readelf -g  %t2 | FileCheck %s --check-prefix=CHECK-ELF
 ; RUN: llvm-mc %t1 -filetype=obj -o %t3
 ; RUN: llvm-objdump --section-headers  %t3 | FileCheck %s --check-prefix=CHECK-OBJ
 
@@ -16,7 +17,10 @@
 
 @a = dso_local global i32 0, align 4
 
-define void @foo(i32 %x) !dbg !3 {
+$foo = comdat any
+$foo2 = comdat any
+
+define void @foo(i32 %x) comdat !dbg !3 {
 bb0:
   %cmp = icmp eq i32 %x, 0
 ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID:]], i64 1, i32 0, i64 -1), !dbg ![[#FAKELINE:]]
@@ -49,7 +53,7 @@
 
 declare void @bar(i32 %x)
 
-define internal void @foo2(void (i32)* %f) !dbg !4 {
+define internal void @foo2(void (i32)* %f) comdat !dbg !4 {
 entry:
 ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2:]], i64 1, i32 0, i64 -1)
 ; CHECK-MIR: PSEUDO_PROBE [[#GUID2:]], 1, 0, 0
@@ -97,9 +101,27 @@
 ; CHECK-ASM-NEXT: .ascii	"foo2"
 
 ; CHECK-OBJ-COUNT-2: .pseudo_probe_desc
-; CHECK-OBJ: .pseudo_probe
+; CHECK-OBH-COUNT-2: .pseudo_probe
 ; CHECK-OBJ-NOT: .rela.pseudo_probe
 
+; CHECK-ELF: COMDAT group section [    3] `.group' [foo] contains 3 sections:
+; CHECK-ELF-NEXT:   [Index]    Name
+; CHECK-ELF-NEXT:   [    4]   .text.foo
+; CHECK-ELF-NEXT:   [    5]   .rela.text.foo
+; CHECK-ELF-NEXT:   [   17]   .pseudo_probe.foo
+; CHECK-ELF: COMDAT group section [    6] `.group' [foo2] contains 3 sections:
+; CHECK-ELF-NEXT:   [Index]    Name
+; CHECK-ELF-NEXT:   [    7]   .text.foo2
+; CHECK-ELF-NEXT:   [    8]   .rela.text.foo2
+; CHECK-ELF-NEXT:   [   18]   .pseudo_probe.foo2
+; CHECK-ELF: COMDAT group section [   10] `.group' [.pseudo_probe_desc_foo] contains 1 sections:
+; CHECK-ELF-NEXT:   [Index]    Name
+; CHECK-ELF-NEXT:   [   11]   .pseudo_probe_desc
+
+; CHECK-ELF: COMDAT group section [   12] `.group' [.pseudo_probe_desc_foo2] contains 1 sections:
+; CHECK-ELF-NEXT:   [Index]    Name
+; CHECK-ELF-NEXT:   [   13]   .pseudo_probe_desc
+
 ; Check the generation of .pseudo_probe_desc section with CFG encoding.
 ; CHECK-CFG: .section	.pseudo_probe_desc,"", at progbits
 ; CHECK-CFG-NEXT: .quad [[#GUID:]]
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===================================================================
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -1040,8 +1040,9 @@
     if (const MCSymbol *Group = ElfSec.getGroup()) {
       auto *S = static_cast<MCSectionELF *>(PseudoProbeSection);
       auto Flags = S->getFlags() | ELF::SHF_GROUP;
-      return Ctx->getELFSection(S->getName(), S->getType(), Flags,
-                                S->getEntrySize(), Group->getName());
+      return Ctx->getELFSection(S->getName() + "." + Group->getName(),
+                                S->getType(), Flags, S->getEntrySize(),
+                                Group->getName());
     }
   }
   return PseudoProbeSection;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146853.508243.patch
Type: text/x-patch
Size: 3270 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230324/2eb54188/attachment.bin>