[compiler-rt] 365b225 - [PGO] Fix two issues in PGOMemOPSizeOpt.
Hiroshi Yamauchi via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 11 09:53:22 PST 2021
Author: Hiroshi Yamauchi
Date: 2021-03-11T09:53:05-08:00
New Revision: 365b225d461899abc437f4a291181de5c80d03d4
URL: https://github.com/llvm/llvm-project/commit/365b225d461899abc437f4a291181de5c80d03d4
DIFF: https://github.com/llvm/llvm-project/commit/365b225d461899abc437f4a291181de5c80d03d4.diff
LOG: [PGO] Fix two issues in PGOMemOPSizeOpt.
1. PGOMemOPSizeOpt grabs only the first, up to five (by default) entries from
the value profile metadata and preserves the remaining entries for the fallback
memop call site. If there are more than five entries, the rest of the entries
would get dropped. This is fine for PGOMemOPSizeOpt itself as it only promotes
up to 3 (by default) values, but potentially not for other downstream passes
that may use the value profile metadata.
2. PGOMemOPSizeOpt originally assumed that only values 0 through 8 are kept
track of. When the range buckets were introduced, it was changed to skip the
range buckets, but since it does not grab all entries (only five), if some range
buckets exist in the first five entries, it could potentially cause fewer
promotion opportunities (eg. if 4 out of 5 were range buckets, it may be able to
promote up to one non-range bucket, as opposed to 3.) Also, combined with 1, it
means that wrong entries may be preserved, as it didn't correctly keep track of
which were entries were skipped.
To fix this, PGOMemOPSizeOpt now grabs all the entries (up to the maximum number
of value profile buckets), keeps track of which entries were skipped, and
preserves all the remaining entries.
Differential Revision: https://reviews.llvm.org/D97592
Added:
llvm/test/Transforms/PGOProfile/memop_size_opt_skip_ranges_promote_three.ll
Modified:
compiler-rt/include/profile/InstrProfData.inc
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
llvm/test/Transforms/PGOProfile/memop_size_opt.ll
Removed:
################################################################################
diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index c676dc255619..ffc7dee4ed6d 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -813,6 +813,7 @@ typedef struct InstrProfValueData {
* range and used to store in the runtime profile data records and the VP
* metadata. For example, it's 2 for [2, 2] and 64 for [65, 127].
*/
+#define INSTR_PROF_NUM_BUCKETS 22
/*
* Clz and Popcount. This code was copied from
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
index 9112c9fa12b2..6126a61efb72 100644
--- a/llvm/include/llvm/ProfileData/InstrProfData.inc
+++ b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -813,6 +813,7 @@ typedef struct InstrProfValueData {
* range and used to store in the runtime profile data records and the VP
* metadata. For example, it's 2 for [2, 2] and 64 for [65, 127].
*/
+#define INSTR_PROF_NUM_BUCKETS 22
/*
* Clz and Popcount. This code was copied from
diff --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
index 24ebd8a6a151..1af891c51bc1 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
@@ -225,7 +225,7 @@ class MemOPSizeOpt : public InstVisitor<MemOPSizeOpt> {
TargetLibraryInfo &TLI)
: Func(Func), BFI(BFI), ORE(ORE), DT(DT), TLI(TLI), Changed(false) {
ValueDataArray =
- std::make_unique<InstrProfValueData[]>(MemOPMaxVersion + 2);
+ std::make_unique<InstrProfValueData[]>(INSTR_PROF_NUM_BUCKETS);
}
bool isChanged() const { return Changed; }
void perform() {
@@ -298,9 +298,9 @@ bool MemOPSizeOpt::perform(MemOp MO) {
if (!MemOPOptMemcmpBcmp && (MO.isMemcmp(TLI) || MO.isBcmp(TLI)))
return false;
- uint32_t NumVals, MaxNumPromotions = MemOPMaxVersion + 2;
+ uint32_t NumVals, MaxNumVals = INSTR_PROF_NUM_BUCKETS;
uint64_t TotalCount;
- if (!getValueProfDataFromInst(*MO.I, IPVK_MemOPSize, MaxNumPromotions,
+ if (!getValueProfDataFromInst(*MO.I, IPVK_MemOPSize, MaxNumVals,
ValueDataArray.get(), NumVals, TotalCount))
return false;
@@ -342,19 +342,25 @@ bool MemOPSizeOpt::perform(MemOp MO) {
int64_t LastV = -1;
// Default case is in the front -- save the slot here.
CaseCounts.push_back(0);
- for (auto &VD : VDs) {
+ SmallVector<InstrProfValueData, 24> RemainingVDs;
+ for (auto I = VDs.begin(), E = VDs.end(); I != E; ++I) {
+ auto &VD = *I;
int64_t V = VD.Value;
uint64_t C = VD.Count;
if (MemOPScaleCount)
C = getScaledCount(C, ActualCount, SavedTotalCount);
- if (!InstrProfIsSingleValRange(V) || V > MemOpMaxOptSize)
+ if (!InstrProfIsSingleValRange(V) || V > MemOpMaxOptSize) {
+ RemainingVDs.push_back(VD);
continue;
+ }
// ValueCounts are sorted on the count. Break at the first un-profitable
// value.
- if (!isProfitable(C, RemainCount))
+ if (!isProfitable(C, RemainCount)) {
+ RemainingVDs.insert(RemainingVDs.end(), I, E);
break;
+ }
if (V == LastV) {
LLVM_DEBUG(dbgs() << "Invalid Profile Data in Function " << Func.getName()
@@ -375,8 +381,10 @@ bool MemOPSizeOpt::perform(MemOp MO) {
assert(SavedRemainCount >= VD.Count);
SavedRemainCount -= VD.Count;
- if (++Version > MemOPMaxVersion && MemOPMaxVersion != 0)
+ if (++Version >= MemOPMaxVersion && MemOPMaxVersion != 0) {
+ RemainingVDs.insert(RemainingVDs.end(), I + 1, E);
break;
+ }
}
if (Version == 0)
@@ -441,10 +449,12 @@ bool MemOPSizeOpt::perform(MemOp MO) {
// Clear the value profile data.
MO.I->setMetadata(LLVMContext::MD_prof, nullptr);
// If all promoted, we don't need the MD.prof metadata.
- if (SavedRemainCount > 0 || Version != NumVals)
+ if (SavedRemainCount > 0 || Version != NumVals) {
// Otherwise we need update with the un-promoted records back.
- annotateValueSite(*Func.getParent(), *MO.I, VDs.slice(Version),
- SavedRemainCount, IPVK_MemOPSize, NumVals);
+ ArrayRef<InstrProfValueData> RemVDs(RemainingVDs);
+ annotateValueSite(*Func.getParent(), *MO.I, RemVDs, SavedRemainCount,
+ IPVK_MemOPSize, NumVals);
+ }
LLVM_DEBUG(dbgs() << "\n\n== Basic Block After==\n");
diff --git a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
index db11b87dda16..d9d43de4289e 100644
--- a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
+++ b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
@@ -145,10 +145,8 @@ for.end6:
; MEMOP_OPT: [[SWITCH_BW]] = !{!"branch_weights", i32 457, i32 99}
; Should be 457 total left (original total count 556, minus 99 from specialized
-; value 1, which is removed from VP array. Also, we only end up with 5 total
-; values, since the default max number of promotions is 5 and therefore
-; the rest of the values are ignored when extracting the VP metadata.
-; MEMOP_OPT: [[NEWVP]] = !{!"VP", i32 1, i64 457, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66}
+; value 0, which is removed from VP array. This should preserve all unpromoted values.
+; MEMOP_OPT: [[NEWVP]] = !{!"VP", i32 1, i64 457, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
!llvm.module.flags = !{!0}
diff --git a/llvm/test/Transforms/PGOProfile/memop_size_opt_skip_ranges_promote_three.ll b/llvm/test/Transforms/PGOProfile/memop_size_opt_skip_ranges_promote_three.ll
new file mode 100644
index 000000000000..1acb65c88db5
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/memop_size_opt_skip_ranges_promote_three.ll
@@ -0,0 +1,67 @@
+; RUN: opt < %s -pgo-memop-opt -pgo-memop-count-threshold=100 -pgo-memop-percent-threshold=10 -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-memop-opt -pgo-memop-count-threshold=100 -pgo-memop-percent-threshold=10 -S | FileCheck %s
+
+define void @foo(i8* %dst, i8* %src, i8* %dst2, i8* %src2, i64 %n) !prof !27 {
+entry:
+ call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %n, i1 false), !prof !28
+ ret void
+}
+
+; CHECK: switch i64 %n, label %[[DEFAULT_LABEL:.*]] [
+; CHECK: i64 0, label %[[CASE_0_LABEL:.*]]
+; CHECK: i64 1, label %[[CASE_1_LABEL:.*]]
+; CHECK: i64 2, label %[[CASE_2_LABEL:.*]]
+; CHECK: ], !prof [[SWITCH_BW:![0-9]+]]
+; CHECK: [[CASE_0_LABEL]]:
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 0, i1 false)
+; CHECK: br label %[[MERGE_LABEL:.*]]
+; CHECK: [[CASE_1_LABEL]]:
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 1, i1 false)
+; CHECK: br label %[[MERGE_LABEL:.*]]
+; CHECK: [[CASE_2_LABEL]]:
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 2, i1 false)
+; CHECK: br label %[[MERGE_LABEL:.*]]
+; CHECK: [[DEFAULT_LABEL]]:
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %n, i1 false), !prof [[NEWVP:![0-9]+]]
+; CHECK: br label %[[MERGE_LABEL]]
+; CHECK: [[MERGE_LABEL]]:
+; CHECK: ret void
+
+; It should skip range values 9, 17, 33, 65, 129 and promote (up to) three values, 0,
+; 1, 2 (not 3), and preserve all unpromoted values in the new VP metadata.
+; CHECK: [[SWITCH_BW]] = !{!"branch_weights", i32 524, i32 101, i32 101, i32 101}
+; CHECK: [[NEWVP]] = !{!"VP", i32 1, i64 524, i64 9, i64 104, i64 17, i64 103, i64 33, i64 103, i64 65, i64 102, i64 129, i64 102, i64 3, i64 101}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"ProfileSummary", !1}
+!1 = !{!2, !3, !4, !5, !6, !7, !8, !9}
+!2 = !{!"ProfileFormat", !"InstrProf"}
+!3 = !{!"TotalCount", i64 579}
+!4 = !{!"MaxCount", i64 556}
+!5 = !{!"MaxInternalCount", i64 20}
+!6 = !{!"MaxFunctionCount", i64 556}
+!7 = !{!"NumCounts", i64 6}
+!8 = !{!"NumFunctions", i64 3}
+!9 = !{!"DetailedSummary", !10}
+!10 = !{!11, !12, !13, !14, !15, !16, !16, !17, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26}
+!11 = !{i32 10000, i64 556, i32 1}
+!12 = !{i32 100000, i64 556, i32 1}
+!13 = !{i32 200000, i64 556, i32 1}
+!14 = !{i32 300000, i64 556, i32 1}
+!15 = !{i32 400000, i64 556, i32 1}
+!16 = !{i32 500000, i64 556, i32 1}
+!17 = !{i32 600000, i64 556, i32 1}
+!18 = !{i32 700000, i64 556, i32 1}
+!19 = !{i32 800000, i64 556, i32 1}
+!20 = !{i32 900000, i64 556, i32 1}
+!21 = !{i32 950000, i64 556, i32 1}
+!22 = !{i32 990000, i64 20, i32 2}
+!23 = !{i32 999000, i64 1, i32 5}
+!24 = !{i32 999900, i64 1, i32 5}
+!25 = !{i32 999990, i64 1, i32 5}
+!26 = !{i32 999999, i64 1, i32 5}
+!27 = !{!"function_entry_count", i64 827}
+!28 = !{!"VP", i32 1, i64 827, i64 9, i64 104, i64 17, i64 103, i64 33, i64 103, i64 65, i64 102, i64 129, i64 102, i64 0, i64 101, i64 1, i64 101, i64 2, i64 101, i64 3, i64 101}
More information about the llvm-commits
mailing list