[llvm] 6da7d31 - [llvm-profdata] Emit Error when Invalid MemOpSize Section is Created by llvm-profdata
Matthew Voss via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 12:52:55 PST 2021
Author: Matthew Voss
Date: 2021-02-23T12:51:54-08:00
New Revision: 6da7d3141651ed3ef2b5f369e8ca0eb2e5c66778
URL: https://github.com/llvm/llvm-project/commit/6da7d3141651ed3ef2b5f369e8ca0eb2e5c66778
DIFF: https://github.com/llvm/llvm-project/commit/6da7d3141651ed3ef2b5f369e8ca0eb2e5c66778.diff
LOG: [llvm-profdata] Emit Error when Invalid MemOpSize Section is Created by llvm-profdata
Under certain (currently unknown) conditions, llvm-profdata is outputting
profiles that have two consecutive entries in the MemOPSize section for the
value 0. This causes the PGOMemOPSizeOpt pass to output an invalid switch
instruction with two cases for 0. As mentioned, we’re not quite sure what’s
causing this to happen, but this patch prevents llvm-profdata from outputting a
profile that has this problem and gives an error with a request for a
reproducible.
Differential Revision: https://reviews.llvm.org/D92074
Added:
llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext
llvm/test/Transforms/PGOProfile/consecutive-zeros.ll
llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext
Modified:
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfWriter.h
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
llvm/test/Transforms/PGOProfile/memop_size_opt.ll
llvm/tools/llvm-profdata/llvm-profdata.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index b48438d08981..1c225de3ac72 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -291,6 +291,7 @@ enum class instrprof_error {
truncated,
malformed,
unknown_function,
+ invalid_prof,
hash_mismatch,
count_mismatch,
counter_overflow,
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
index 35c2669d55a6..97c80de6aa23 100644
--- a/llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -64,11 +64,13 @@ class InstrProfWriter {
function_ref<void(Error)> Warn);
/// Write the profile to \c OS
- void write(raw_fd_ostream &OS);
+ Error write(raw_fd_ostream &OS);
/// Write the profile in text format to \c OS
Error writeText(raw_fd_ostream &OS);
+ Error validateRecord(const InstrProfRecord &Func);
+
/// Write \c Record in text format to \c OS
static void writeRecordInText(StringRef Name, uint64_t Hash,
const InstrProfRecord &Counters,
@@ -114,7 +116,8 @@ class InstrProfWriter {
void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
uint64_t Weight, function_ref<void(Error)> Warn);
bool shouldEncodeData(const ProfilingData &PD);
- void writeImpl(ProfOStream &OS);
+
+ Error writeImpl(ProfOStream &OS);
};
} // end namespace llvm
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 52adfeeb97fe..4757e8f521fe 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -18,6 +18,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Triple.h"
+#include "llvm/Config/config.h"
#include "llvm/IR/Constant.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Function.h"
@@ -95,6 +96,10 @@ static std::string getInstrProfErrString(instrprof_error Err) {
return "Truncated profile data";
case instrprof_error::malformed:
return "Malformed instrumentation profile data";
+ case instrprof_error::invalid_prof:
+ return "Invalid profile created. Please file a bug "
+ "at: " BUG_REPORT_URL
+ " and include the profraw files that caused this error.";
case instrprof_error::unknown_function:
return "No profile data available for function";
case instrprof_error::hash_mismatch:
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index d07668322354..987c0b175d3c 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -285,7 +285,7 @@ static void setSummary(IndexedInstrProf::Summary *TheSummary,
TheSummary->setEntry(I, Res[I]);
}
-void InstrProfWriter::writeImpl(ProfOStream &OS) {
+Error InstrProfWriter::writeImpl(ProfOStream &OS) {
using namespace IndexedInstrProf;
OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
@@ -376,12 +376,19 @@ void InstrProfWriter::writeImpl(ProfOStream &OS) {
(int)CSSummarySize}};
OS.patch(PatchItems, sizeof(PatchItems) / sizeof(*PatchItems));
+
+ for (const auto &I : FunctionData)
+ for (const auto &F : I.getValue())
+ if (Error E = validateRecord(F.second))
+ return E;
+
+ return Error::success();
}
-void InstrProfWriter::write(raw_fd_ostream &OS) {
+Error InstrProfWriter::write(raw_fd_ostream &OS) {
// Write the hash table.
ProfOStream POS(OS);
- writeImpl(POS);
+ return writeImpl(POS);
}
std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {
@@ -389,7 +396,8 @@ std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {
raw_string_ostream OS(Data);
ProfOStream POS(OS);
// Write the hash table.
- writeImpl(POS);
+ if (Error E = writeImpl(POS))
+ return nullptr;
// Return this in an aligned memory buffer.
return MemoryBuffer::getMemBufferCopy(Data);
}
@@ -399,6 +407,27 @@ static const char *ValueProfKindStr[] = {
#include "llvm/ProfileData/InstrProfData.inc"
};
+Error InstrProfWriter::validateRecord(const InstrProfRecord &Func) {
+ for (uint32_t VK = 0; VK <= IPVK_Last; VK++) {
+ uint32_t NS = Func.getNumValueSites(VK);
+ if (!NS)
+ continue;
+ for (uint32_t S = 0; S < NS; S++) {
+ uint32_t ND = Func.getNumValueDataForSite(VK, S);
+ std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
+ bool WasZero = false;
+ for (uint32_t I = 0; I < ND; I++)
+ if ((VK != IPVK_IndirectCallTarget) && (VD[I].Value == 0)) {
+ if (WasZero)
+ return make_error<InstrProfError>(instrprof_error::invalid_prof);
+ WasZero = true;
+ }
+ }
+ }
+
+ return Error::success();
+}
+
void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
const InstrProfRecord &Func,
InstrProfSymtab &Symtab,
@@ -473,5 +502,11 @@ Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
writeRecordInText(Name, Func.first, Func.second, Symtab, OS);
}
+ for (const auto &record : OrderedFuncData) {
+ const FuncPair &Func = record.second;
+ if (Error E = validateRecord(Func.second))
+ return E;
+ }
+
return Error::success();
}
diff --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
index 40d41cb024d3..24ebd8a6a151 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
@@ -46,6 +46,7 @@
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/WithColor.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -338,6 +339,7 @@ bool MemOPSizeOpt::perform(MemOp MO) {
SmallVector<uint64_t, 16> CaseCounts;
uint64_t MaxCount = 0;
unsigned Version = 0;
+ int64_t LastV = -1;
// Default case is in the front -- save the slot here.
CaseCounts.push_back(0);
for (auto &VD : VDs) {
@@ -354,6 +356,15 @@ bool MemOPSizeOpt::perform(MemOp MO) {
if (!isProfitable(C, RemainCount))
break;
+ if (V == LastV) {
+ LLVM_DEBUG(dbgs() << "Invalid Profile Data in Function " << Func.getName()
+ << ": Two consecutive, identical values in MemOp value"
+ "counts.\n");
+ return false;
+ }
+
+ LastV = V;
+
SizeIds.push_back(V);
CaseCounts.push_back(C);
if (C > MaxCount)
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext b/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext
new file mode 100644
index 000000000000..93211cd9b079
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext
@@ -0,0 +1,47 @@
+# IR level Instrumentation Flag
+:ir
+foo
+# Func Hash:
+687116424982578944
+# Num Counters:
+3
+# Counter Values:
+523
+20
+1
+# Num Value Kinds:
+1
+# ValueKind = IPVK_MemOPSize:
+1
+# NumValueSites:
+3
+9
+0:99
+0:88
+3:77
+9:72
+4:66
+5:55
+6:44
+7:33
+8:22
+9
+7:33
+2:88
+9:72
+4:66
+1:99
+5:55
+6:44
+3:77
+8:22
+9
+7:33
+2:88
+9:72
+4:66
+1:99
+5:55
+6:44
+3:77
+8:22
diff --git a/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll b/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll
new file mode 100644
index 000000000000..cbe29b775c0f
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll
@@ -0,0 +1,58 @@
+; RUN: llvm-profdata merge %S/Inputs/consecutive-zeros.proftext -o %t.profdata
+; RUN: opt < %s -debug -pgo-instr-use -pgo-memop-opt -pgo-memop-count-threshold=0 -pgo-memop-percent-threshold=0 -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s
+
+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 @foo(i8* %dst, i8* %src, i32* %a, i32 %n) {
+; CHECK: Invalid Profile
+entry:
+ br label %for.cond
+
+for.cond:
+ %i.0 = phi i32 [ 0, %entry ], [ %inc5, %for.inc4 ]
+ %cmp = icmp slt i32 %i.0, %n
+ br i1 %cmp, label %for.body, label %for.end6
+
+for.body:
+ br label %for.cond1
+
+for.cond1:
+ %j.0 = phi i32 [ 0, %for.body ], [ %inc, %for.inc ]
+ %idx.ext = sext i32 %i.0 to i64
+ %add.ptr = getelementptr inbounds i32, i32* %a, i64 %idx.ext
+ %0 = load i32, i32* %add.ptr, align 4
+ %cmp2 = icmp slt i32 %j.0, %0
+ br i1 %cmp2, label %for.body3, label %for.end
+
+for.body3:
+ %add = add nsw i32 %i.0, 1
+ %conv = sext i32 %add to i64
+ call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false)
+ %memcmp = call i32 @memcmp(i8* %dst, i8* %src, i64 %conv)
+ %bcmp = call i32 @bcmp(i8* %dst, i8* %src, i64 %conv)
+ br label %for.inc
+
+for.inc:
+ %inc = add nsw i32 %j.0, 1
+ br label %for.cond1
+
+for.end:
+ br label %for.inc4
+
+for.inc4:
+ %inc5 = add nsw i32 %i.0, 1
+ br label %for.cond
+
+for.end6:
+ ret void
+}
+
+declare void @llvm.lifetime.start(i64, i8* nocapture)
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
+
+declare i32 @memcmp(i8*, i8*, i64)
+declare i32 @bcmp(i8*, i8*, i64)
+
+declare void @llvm.lifetime.end(i64, i8* nocapture)
diff --git a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
index bc79fbc3e37e..db11b87dda16 100644
--- a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
+++ b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
@@ -37,20 +37,20 @@ for.body3:
br label %for.inc
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL:.*]] [
-; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL:.*]]
+; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL]]:
-; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 1, i1 false)
+; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 0, i1 false)
; MEMOP_OPT: br label %[[MERGE_LABEL:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false), !prof [[NEWVP:![0-9]+]]
; MEMOP_OPT: br label %[[MERGE_LABEL]]
; MEMOP_OPT: [[MERGE_LABEL]]:
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL2:.*]] [
-; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL2:.*]]
+; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL2]]:
-; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 1, i1 false)
+; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 0, i1 false)
; MEMOP_OPT: br label %[[MERGE_LABEL2:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL2]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 %conv, i1 false), !prof [[NEWVP]]
@@ -104,10 +104,10 @@ for.body3:
br label %for.inc
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL:.*]] [
-; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL:.*]]
+; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL]]:
-; MEMOP_OPT: %[[RV:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 1)
+; MEMOP_OPT: %[[RV:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 0)
; MEMOP_OPT: br label %[[MERGE_LABEL:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL]]:
; MEMOP_OPT: %[[RVD:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 %conv), !prof [[NEWVP:![0-9]+]]
@@ -115,10 +115,10 @@ for.body3:
; MEMOP_OPT: [[MERGE_LABEL]]:
; MEMOP_OPT: %[[PHI:.*]] = phi i32 [ %[[RVD]], %[[DEFAULT_LABEL]] ], [ %[[RV]], %[[CASE_1_LABEL]] ]
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL2:.*]] [
-; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL2:.*]]
+; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL2]]:
-; MEMOP_OPT: %[[RV2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 1)
+; MEMOP_OPT: %[[RV2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 0)
; MEMOP_OPT: br label %[[MERGE_LABEL2:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL2]]:
; MEMOP_OPT: %[[RVD2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 %conv), !prof [[NEWVP]]
@@ -182,8 +182,8 @@ for.end6:
!27 = !{!"function_entry_count", i64 1}
!28 = !{!"branch_weights", i32 20, i32 1}
!29 = !{!"branch_weights", i32 556, i32 20}
-!30 = !{!"VP", i32 1, i64 556, i64 1, i64 99, 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}
-!31 = !{!"VP", i32 1, i64 556, i64 1, i64 99, 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}
+!30 = !{!"VP", i32 1, i64 556, i64 0, i64 99, 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}
+!31 = !{!"VP", i32 1, i64 556, i64 0, i64 99, 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}
declare void @llvm.lifetime.start(i64, i8* nocapture)
diff --git a/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext b/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext
new file mode 100644
index 000000000000..63a7dd91d17b
--- /dev/null
+++ b/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext
@@ -0,0 +1,30 @@
+# RUN: llvm-profdata merge --text -j 4 %s %s %s %s -o %t 2>&1 | FileCheck %s
+# RUN: llvm-profdata merge --binary -j 4 %s %s %s %s -o %t 2>&1 | FileCheck %s
+# IR level Instrumentation Flag
+# CHECK: Invalid profile
+:ir
+foo
+# Func Hash:
+35277121310
+# Num Counters:
+3
+# Counter Values:
+20
+556
+1
+# Num Value Kinds:
+1
+# ValueKind = IPVK_MemOPSize:
+1
+# NumValueSites:
+1
+9
+0:99
+0:88
+3:77
+9:72
+4:66
+5:55
+6:44
+7:33
+8:22
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 7e53c30c7579..0066765106db 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -54,6 +54,14 @@ static void warn(Twine Message, std::string Whence = "",
WithColor::note() << Hint << "\n";
}
+static void warn(Error E, StringRef Whence = "") {
+ if (E.isA<InstrProfError>()) {
+ handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
+ warn(IPE.message(), std::string(Whence), std::string(""));
+ });
+ }
+}
+
static void exitWithError(Twine Message, std::string Whence = "",
std::string Hint = "") {
WithColor::error();
@@ -304,9 +312,10 @@ static void writeInstrProfile(StringRef OutputFilename,
if (OutputFormat == PF_Text) {
if (Error E = Writer.writeText(Output))
- exitWithError(std::move(E));
+ warn(std::move(E));
} else {
- Writer.write(Output);
+ if (Error E = Writer.write(Output))
+ warn(std::move(E));
}
}
More information about the llvm-commits
mailing list