diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -290,6 +290,7 @@ 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 --- a/llvm/include/llvm/ProfileData/InstrProfWriter.h +++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h @@ -64,11 +64,13 @@ function_ref 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 @@ void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I, uint64_t Weight, function_ref 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 --- 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 @@ 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 --- a/llvm/lib/ProfileData/InstrProfWriter.cpp +++ b/llvm/lib/ProfileData/InstrProfWriter.cpp @@ -285,7 +285,7 @@ TheSummary->setEntry(I, Res[I]); } -void InstrProfWriter::writeImpl(ProfOStream &OS) { +Error InstrProfWriter::writeImpl(ProfOStream &OS) { using namespace IndexedInstrProf; OnDiskChainedHashTableGenerator Generator; @@ -376,12 +376,19 @@ (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 InstrProfWriter::writeBuffer() { @@ -389,7 +396,8 @@ 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 @@ #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 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(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 @@ 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 --- 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 @@ SmallVector 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 @@ 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 --- /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 --- /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 --- a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll +++ b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll @@ -37,20 +37,20 @@ 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 @@ 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 @@ ; 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 @@ !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 --- /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 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -54,6 +54,14 @@ WithColor::note() << Hint << "\n"; } +static void warn(Error E, StringRef Whence = "") { + if (E.isA()) { + 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 @@ 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)); } }