[PATCH] D109398: [SampleFDO] Allow forward compatibility when adding a new section for extbinary format.

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: wenlei, hoy, davidxl.
wmi requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Currently when we add a new section in the profile format and generate a profile containing the new section. Older compiler which reads the new profile will issue an error. The forward incompatibility can cause unnecessary churn when extending the profile. This patch removes the incompatibility when adding a new section for extbinary format.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109398

Files:
  llvm/include/llvm/ProfileData/SampleProf.h
  llvm/include/llvm/ProfileData/SampleProfReader.h
  llvm/test/tools/llvm-profdata/Inputs/unknown.section.extbin.profdata
  llvm/test/tools/llvm-profdata/forward-compatible.test


Index: llvm/test/tools/llvm-profdata/forward-compatible.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-profdata/forward-compatible.test
@@ -0,0 +1,26 @@
+RUN: llvm-profdata show -sample %S/Inputs/unknown.section.extbin.profdata | FileCheck %s
+RUN: llvm-profdata show -sample -show-sec-info-only %S/Inputs/unknown.section.extbin.profdata | FileCheck %s -check-prefix=HDR
+
+# The input unknown.section.extbin.profdata contains an unknown section type
+# which uses an enum value which won't be used in the near future. Check
+# llvm-profdata can read it without problem. This is to ensure adding new
+# section in extbinary format for SampleFDO won't cause forward compatibility
+# issue.
+
+CHECK: Function: main: 366846, 0, 3 sampled lines
+CHECK-NEXT: Samples collected in the function's body {
+CHECK-NEXT:   2.1: 60401
+CHECK-NEXT:   3: 0
+CHECK-NEXT:   4: 0
+CHECK-NEXT: }
+
+HDR: ProfileSummarySection - Offset: 242, Size: 121, Flags: {}
+HDR-NEXT: NameTableSection - Offset: 363, Size: 6, Flags: {}
+HDR-NEXT: UnknownSection - Offset: 369, Size: 1, Flags: {}
+HDR-NEXT: FuncOffsetTableSection - Offset: 391, Size: 3, Flags: {}
+HDR-NEXT: LBRProfileSection - Offset: 370, Size: 21, Flags: {}
+HDR-NEXT: ProfileSymbolListSection - Offset: 391, Size: 0, Flags: {}
+HDR-NEXT: FunctionMetadata - Offset: 394, Size: 0, Flags: {}
+HDR-NEXT: Header Size: 242
+HDR-NEXT: Total Sections Size: 152
+HDR-NEXT: File Size: 394
Index: llvm/include/llvm/ProfileData/SampleProfReader.h
===================================================================
--- llvm/include/llvm/ProfileData/SampleProfReader.h
+++ llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -786,6 +786,8 @@
   virtual std::error_code verifySPMagic(uint64_t Magic) override;
   virtual std::error_code
   readCustomSection(const SecHdrTableEntry &Entry) override {
+    // Update the data reader pointer to the end of the section.
+    Data += Entry.Size;
     return sampleprof_error::success;
   };
 
Index: llvm/include/llvm/ProfileData/SampleProf.h
===================================================================
--- llvm/include/llvm/ProfileData/SampleProf.h
+++ llvm/include/llvm/ProfileData/SampleProf.h
@@ -149,8 +149,9 @@
     return "CSNameTableSection";
   case SecLBRProfile:
     return "LBRProfileSection";
+  default:
+    return "UnknownSection";
   }
-  llvm_unreachable("A SecType has no name for output");
 }
 
 // Entry type of section header table used by SampleProfileExtBinaryBaseReader


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109398.371201.patch
Type: text/x-patch
Size: 2542 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210907/1deb142e/attachment.bin>