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 @@ public: 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 @@ private: 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 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 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 InstrProfWriter::writeBuffer() { @@ -389,7 +396,8 @@ std::unique_ptr 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 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 @@ 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 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()) { + 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)); } }