[llvm] [MemProf] Change the STACK_ID record to fixed width values (PR #116448)
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 15 15:21:08 PST 2024
https://github.com/teresajohnson updated https://github.com/llvm/llvm-project/pull/116448
>From cffb35170a710d6e3ac71dc39b645edddb43330b Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson at google.com>
Date: Fri, 15 Nov 2024 14:50:40 -0800
Subject: [PATCH 1/3] [MemProf] Change the STACK_ID record to fixed width
values
The stack ids are hashes that are close to 64 bits in size, so emitting
as a pair of 32-bit fixed-width values is more efficient than a VBR.
This reduced the summary bitcode size for a large target by about 1%.
Bump the index version and ensure we can read the old format.
---
llvm/include/llvm/IR/ModuleSummaryIndex.h | 2 +-
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 11 ++++++-
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 27 +++++++++++++-----
.../X86/Inputs/memprof-old-stackid-summary.bc | Bin 0 -> 2128 bytes
.../X86/memprof-old-stackid-summary.ll | 20 +++++++++++++
5 files changed, 51 insertions(+), 9 deletions(-)
create mode 100644 llvm/test/ThinLTO/X86/Inputs/memprof-old-stackid-summary.bc
create mode 100644 llvm/test/ThinLTO/X86/memprof-old-stackid-summary.ll
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 50def0eaf78867..39c60229aa1d81 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1463,7 +1463,7 @@ class ModuleSummaryIndex {
// in the way some record are interpreted, like flags for instance.
// Note that incrementing this may require changes in both BitcodeReader.cpp
// and BitcodeWriter.cpp.
- static constexpr uint64_t BitcodeSummaryVersion = 11;
+ static constexpr uint64_t BitcodeSummaryVersion = 12;
// Regular LTO module name for ASM writer
static constexpr const char *getRegularLTOModuleName() {
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 9ca76b54a88d9d..3e6abacac27261 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7997,7 +7997,16 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
case bitc::FS_STACK_IDS: { // [n x stackid]
// Save stack ids in the reader to consult when adding stack ids from the
// lists in the stack node and alloc node entries.
- StackIds = ArrayRef<uint64_t>(Record);
+ if (Version <= 11) {
+ StackIds = ArrayRef<uint64_t>(Record);
+ break;
+ }
+ // This is an array of 32-bit fixed-width values, holding each 64-bit
+ // context id as a pair of adjacent (most significant first) 32-bit words.
+ assert(Record.size() % 2 == 0);
+ StackIds.reserve(Record.size() / 2);
+ for (auto R = Record.begin(); R != Record.end(); R += 2)
+ StackIds.push_back(*R << 32 | *(R + 1));
break;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 5829af39cf5e2c..039e9e865b3449 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4429,12 +4429,17 @@ void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
StackIdAbbv->Add(BitCodeAbbrevOp(bitc::FS_STACK_IDS));
// numids x stackid
StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
- // FIXME: The stack ids are hashes that are close to 64 bits in size, so
- // emitting as a pair of 32-bit fixed-width values, as we do for context
- // ids, would be more efficient.
- StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
+ // The stack ids are hashes that are close to 64 bits in size, so emitting
+ // as a pair of 32-bit fixed-width values is more efficient than a VBR.
+ StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
unsigned StackIdAbbvId = Stream.EmitAbbrev(std::move(StackIdAbbv));
- Stream.EmitRecord(bitc::FS_STACK_IDS, Index->stackIds(), StackIdAbbvId);
+ SmallVector<uint32_t> Vals;
+ Vals.reserve(Index->stackIds().size()*2);
+ for (auto Id : Index->stackIds()) {
+ Vals.push_back(static_cast<uint32_t>(Id >> 32));
+ Vals.push_back(static_cast<uint32_t>(Id));
+ }
+ Stream.EmitRecord(bitc::FS_STACK_IDS, Vals, StackIdAbbvId);
}
// n x context id
@@ -4624,9 +4629,17 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
StackIdAbbv->Add(BitCodeAbbrevOp(bitc::FS_STACK_IDS));
// numids x stackid
StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
- StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
+ // The stack ids are hashes that are close to 64 bits in size, so emitting
+ // as a pair of 32-bit fixed-width values is more efficient than a VBR.
+ StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
unsigned StackIdAbbvId = Stream.EmitAbbrev(std::move(StackIdAbbv));
- Stream.EmitRecord(bitc::FS_STACK_IDS, StackIds, StackIdAbbvId);
+ SmallVector<uint32_t> Vals;
+ Vals.reserve(StackIds.size()*2);
+ for (auto Id : StackIds) {
+ Vals.push_back(static_cast<uint32_t>(Id >> 32));
+ Vals.push_back(static_cast<uint32_t>(Id));
+ }
+ Stream.EmitRecord(bitc::FS_STACK_IDS, Vals, StackIdAbbvId);
}
// Abbrev for FS_COMBINED_PROFILE.
diff --git a/llvm/test/ThinLTO/X86/Inputs/memprof-old-stackid-summary.bc b/llvm/test/ThinLTO/X86/Inputs/memprof-old-stackid-summary.bc
new file mode 100644
index 0000000000000000000000000000000000000000..78b134cb44216e9dd72e1c140830d50fd83825bc
GIT binary patch
literal 2128
zcmX|CeM}q46(6(UHQ?+n33su>db<lGMe30*OJeNUR>8~Vd^)1 at B^QaRjbiiRgz{yK
z0Yk2>ja{vySSOQOI&@7Xb4jmh`wzL}k5o-7U at 7$xU+Sc%3MmNL<s+ox6569xs-CWW
zYm%GQyf^b^X5V|i_uh<MoLxO$j!+Ros7=dPpZe|}K0Ni`nKvs^)y*oQ2yt437E}n8
z7I3Hl_UNJGp{j|dRN?DtN-$qlMYM;DG<v4+N?TC-YJvS~sl9^|%1s*G8zq7eb$A!J
zs^<MtjB2`C(2>&IDrd!$QT7v+AVgY;_ypG+pBn$5KriEWU(yS)*kQ_W!k<FuG7CCE
zSEZ+r`p7>133VDsd|=+`C at DT7E*9B^BO2`?RZWw+U{f>edKZBY2l#hpG0U?eOEPS{
ziI246BLV4>S2DUJzgHTYm5e5Q<fgRmLnqOjA;y3r(&xp!NqmsS$9UYAQSY$$2$dh_
zg?k%(Nqo2wA9LaUjQTOq{Iu6a1Q0$(;l3c=n<WBCe8BbmAZ+gCO9xmykSv>Dr at G&J
z=>5;bmG}Q5uMNGwBK-0nogZEKlsT;mgLhvy9*MtpQ^Bw22&o&}!j1Q{2nAoEK&*i-
zFq2w+SxY=teB`yoLb0tlhMac~t{Mk)A2<B;I|$v<A(VlA^_n2PkfS~2DqB=EMdX-0
zh>r&F!N~#bCL%wlh}|GjueCGtbB23Uw1+RsZwptXF<VNs#V}hs5c7NSOB#ICA$h&{
zm=_xkNM2Ubr^|@#IFaMYKAv3LA$FWZJ+)w1<>+P3Fz;qO{AJ6Xuq7j!XE5uXur-2N
z(mUnDF8tC(yy2qc0~(aPamnk#himZRXGQSak)U*5BMs;z-=s7;8G7Z(4)G*Q{A-rj
z=CQWa6umx0r}K2hO)qiuLddY>R{u3Fnp2{AGEZ;Iri5sUVwQybZBY8<xaBj<GA(QW
z)FpXaC7(C!zlaAQ2XvA at De3Q>A)g at fpH5;uPOLSOIhNQ4FW4Hx${aJ_#5~~Wl at PO%
zrPp(YzpEIJ?k&@_c;Z^vl6F=<cf=))1n|J*TP?eBQbB<E0kh<$w>ZP<oZ&|*MlqN(
z?6}o$|0HZm%a&=;27r7yZ(SEHQJBiHKbC-}(g at h2!N+QF-(=|fYZS3POKdoaFZimt
z2Tja+?%;CHunGn^^p@$cZ6;h1kxentoR+P(=gcV?e6*$=GXokNtQr6STP6L&MPl1U
zex8LiCO3F;gC+m%B>E^~jVECGMOOVdORPJIXMEM)w%zo6$grF{xXl?hIl}^HfCz4J
z-|(<Mx7-nJw=r`fY)-e{f#`#?V_>I48u3c{yN`&cJekW9j~mIyNzfvzutBIbopgQ@
zQbj at 7pMg+@B2DJ34A9viQt8Hpq+w~wkZodC=V(uLJ5S`4_>%CG4PNBJLpprWA)QZ3
z?*!rOt%S-NldTEalo3xrI;8<It%G~X?q<N$dvk`<p>j{l1)g}yl25bhr%rNL`4zE|
zRd2FnE(6Y9ZwwD##78xf{`8^SvIX++hG<FyhL|}Xwx%&FfSHiBukFiB(eo-M4~1dT
z%{&M(ODZ(V?%i`f#=(kFt-jbNIM}LCY+~YmbCRm*u+|CocjMf39rZHW5B2tHpS!K6
z^NhEr%ir?$n`isF{0LQPBK&xx7RroIIb7pHTl>e|gI$-Ze$)P~!MgTR%g-tfL$xCG
zE1<Q&OG|t;Pz(52t9KIcSD-Uz<nc{<acbXIlTxbS`(BQ{d~D4{JaR(nlIuzWlaHLl
z?sPDOkJjWXD<DBuD0$WDy(XJxRF)fITS6}Ah9ad`a`XyIue%kx6=LSyG$4 at LO9uCx
zirfVN0nZ>&8r=&@(VtBaf at LT)8B)Rm83^}73bQD---nAy4IEEfZ-gz^pa3yjGHiun
zjLYz3AUBH;JRj&lVUtzi%KLF3GQLc~U0tOvMB^@&s-UpE{CjGbwglFdHpL=z2>A8F
zyG|8bU8oP)lkxkq=IqYp-<_hDwm;qa=9*Mo{y_;<*&NlBRW`@sHxEQoRHZ$lDXXg9
zK%8*Wk%o6Ds^d6PGr?8=qKLDfbU5|_N$smVc at rxLVhF6K6qC>Z=)-^u1XUS9?<qrc
zrR29qyXu**Zf&1A2Wj}@v$@aufB!@LtHikkqpWv=Lx+Ly#}YM)ugW0>3julybVb$*
zU1<te6h)N at G!t|PJ90kfd4({3u at 1xheuXRT#k#`(2Mty4$&~qvuE;eYSK2wS%>W#1
z+fwFjZl9;WU*M0zccIJAnvZpMws{(aI=A348SD9DbwWcu?>S!Aahz|cGu0X0Mw7AA
z%?C`z^TuPfLw((S{Ud#~Jzaf6f!YgwL%p57USGebcF^x`@9yfT^>hU~JC64BD7yX+
DUEs5-
literal 0
HcmV?d00001
diff --git a/llvm/test/ThinLTO/X86/memprof-old-stackid-summary.ll b/llvm/test/ThinLTO/X86/memprof-old-stackid-summary.ll
new file mode 100644
index 00000000000000..10048f8674a080
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/memprof-old-stackid-summary.ll
@@ -0,0 +1,20 @@
+;; Check that we can read the old STACK_ID summary format that encoded the id as
+;; a VBR8 instead of as a pair of 32-bit fixed-width values.
+;;
+;; The old bitcode was generated by the older compiler from `opt -thinlto-bc`
+;; on the following LLVM assembly:
+;;
+;; target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+;; target triple = "x86_64-unknown-linux-gnu"
+;;
+;; define void @bar() {
+;; call void @foo(), !callsite !0
+;; ret void
+;; }
+;;
+;; declare void @foo()
+;;
+;; !0 = !{i64 9086428284934609951}
+
+; RUN: llvm-dis %S/Inputs/memprof-old-stackid-summary.bc -o - | FileCheck %s
+; CHECK: stackIds: (9086428284934609951)
>From bc213f537d6a1aced13e7f72841ea2a8f25032fc Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson at google.com>
Date: Fri, 15 Nov 2024 14:58:59 -0800
Subject: [PATCH 2/3] Fix clang format
---
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 039e9e865b3449..24a4c2e8303d5a 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4434,7 +4434,7 @@ void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
unsigned StackIdAbbvId = Stream.EmitAbbrev(std::move(StackIdAbbv));
SmallVector<uint32_t> Vals;
- Vals.reserve(Index->stackIds().size()*2);
+ Vals.reserve(Index->stackIds().size() * 2);
for (auto Id : Index->stackIds()) {
Vals.push_back(static_cast<uint32_t>(Id >> 32));
Vals.push_back(static_cast<uint32_t>(Id));
@@ -4634,7 +4634,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
StackIdAbbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
unsigned StackIdAbbvId = Stream.EmitAbbrev(std::move(StackIdAbbv));
SmallVector<uint32_t> Vals;
- Vals.reserve(StackIds.size()*2);
+ Vals.reserve(StackIds.size() * 2);
for (auto Id : StackIds) {
Vals.push_back(static_cast<uint32_t>(Id >> 32));
Vals.push_back(static_cast<uint32_t>(Id));
>From 3a5e12b1a4fa771217a3348c3b83816cf5669aec Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson at google.com>
Date: Fri, 15 Nov 2024 15:20:51 -0800
Subject: [PATCH 3/3] Fix summary version test
---
llvm/test/Bitcode/summary_version.ll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/test/Bitcode/summary_version.ll b/llvm/test/Bitcode/summary_version.ll
index c8d36f812c208a..c95c145a08788b 100644
--- a/llvm/test/Bitcode/summary_version.ll
+++ b/llvm/test/Bitcode/summary_version.ll
@@ -2,7 +2,7 @@
; RUN: opt -module-summary %s -o - | llvm-bcanalyzer -dump | FileCheck %s
; CHECK: <GLOBALVAL_SUMMARY_BLOCK
-; CHECK: <VERSION op0=11/>
+; CHECK: <VERSION op0=12/>
More information about the llvm-commits
mailing list