diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp index e2c0080fa07f..c18822015e8e 100644 --- a/bolt/lib/Profile/BoltAddressTranslation.cpp +++ b/bolt/lib/Profile/BoltAddressTranslation.cpp @@ -230,9 +230,8 @@ BoltAddressTranslation::getFallthroughsInTrace(const BinaryFunction &Func, To -= Func.getAddress(); auto Iter = Maps.find(Func.getAddress()); - if (Iter == Maps.end()) { + if (Iter == Maps.end()) return NoneType(); - } const MapTy &Map = Iter->second; auto FromIter = Map.upper_bound(From); @@ -261,9 +260,8 @@ BoltAddressTranslation::getFallthroughsInTrace(const BinaryFunction &Func, } ++Iter; - while (Iter->second & BRANCHENTRY && Iter != ToIter) { + while (Iter->second & BRANCHENTRY && Iter != ToIter) ++Iter; - } if (Iter->second & BRANCHENTRY) break; Res.emplace_back(Src, Iter->first); diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index 2ec8d464508e..b249868896f4 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -122,17 +122,15 @@ DataAggregator::~DataAggregator() { deleteTempFiles(); } namespace { void deleteTempFile(const std::string &FileName) { - if (std::error_code Errc = sys::fs::remove(FileName.c_str())) { + if (std::error_code Errc = sys::fs::remove(FileName.c_str())) errs() << "PERF2BOLT: failed to delete temporary file " << FileName << " with error " << Errc.message() << "\n"; - } } } void DataAggregator::deleteTempFiles() { - for (std::string &FileName : TempFiles) { + for (std::string &FileName : TempFiles) deleteTempFile(FileName); - } TempFiles.clear(); } @@ -155,17 +153,16 @@ void DataAggregator::start() { findPerfExecutable(); - if (opts::BasicAggregation) { + if (opts::BasicAggregation) launchPerfProcess("events without LBR", MainEventsPPI, "script -F pid,event,ip", /*Wait = */false); - } else { + else launchPerfProcess("branch events", MainEventsPPI, "script -F pid,ip,brstack", /*Wait = */false); - } // Note: we launch script for mem events regardless of the option, as the // command fails fairly fast if mem events were not collected. @@ -253,13 +250,12 @@ void DataAggregator::launchPerfProcess(StringRef Name, PerfProcessInfo &PPI, << "\n"; }); - if (Wait) { + if (Wait) PPI.PI.ReturnCode = sys::ExecuteAndWait(PerfPath.data(), Argv, /*envp*/ llvm::None, Redirects); - } else { + else PPI.PI = sys::ExecuteNoWait(PerfPath.data(), Argv, /*envp*/ llvm::None, Redirects); - } free(WritableArgsString); } @@ -307,9 +303,8 @@ void DataAggregator::processFileBuildID(StringRef FileBuildID) { "is not the same recorded by perf when collecting profiling " "data, or there were no samples recorded for the binary. " "Use -ignore-build-id option to override.\n"; - if (!opts::IgnoreBuildID) { + if (!opts::IgnoreBuildID) abort(); - } } else if (*FileName != llvm::sys::path::filename(BC->getFilename())) { errs() << "PERF2BOLT-WARNING: build-id matched a different file name\n"; BuildIDBinaryName = std::string(*FileName); @@ -446,10 +441,10 @@ void DataAggregator::filterBinaryMMapInfo() { << " for binary \"" << BC->getFilename() << "\"."; assert(!BinaryMMapInfo.empty() && "No memory map for matching binary"); errs() << " Profile for the following process is available:\n"; - for (std::pair &MMI : BinaryMMapInfo) { + for (std::pair &MMI : BinaryMMapInfo) outs() << " " << MMI.second.PID << (MMI.second.Forked ? " (forked)\n" : "\n"); - } + if (errs().has_colors()) errs().resetColor(); @@ -529,15 +524,13 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { opts::IgnoreInterruptLBR = false; } else { prepareToParse("mmap events", MMapEventsPPI); - if (parseMMapEvents()) { + if (parseMMapEvents()) errs() << "PERF2BOLT: failed to parse mmap events\n"; - } } prepareToParse("task events", TaskEventsPPI); - if (parseTaskEvents()) { + if (parseTaskEvents()) errs() << "PERF2BOLT: failed to parse task events\n"; - } filterBinaryMMapInfo(); prepareToParse("events", MainEventsPPI); @@ -551,15 +544,14 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { } if ((!opts::BasicAggregation && parseBranchEvents()) || - (opts::BasicAggregation && parseBasicEvents())) { + (opts::BasicAggregation && parseBasicEvents())) errs() << "PERF2BOLT: failed to parse samples\n"; - } // We can finish early if the goal is just to generate data for autofdo if (opts::WriteAutoFDOData) { - if (std::error_code EC = writeAutoFDOData(opts::OutputFilename)) { + if (std::error_code EC = writeAutoFDOData(opts::OutputFilename)) errs() << "Error writing autofdo data to file: " << EC.message() << "\n"; - } + deleteTempFiles(); exit(0); } @@ -597,10 +589,9 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { ParsingBuf = FileBuf->getBuffer(); Col = 0; Line = 1; - if (const std::error_code EC = parseMemEvents()) { + if (const std::error_code EC = parseMemEvents()) errs() << "PERF2BOLT: failed to parse memory events: " << EC.message() << '\n'; - } deleteTempFiles(); @@ -616,9 +607,8 @@ Error DataAggregator::readProfile(BinaryContext &BC) { } if (opts::AggregateOnly) { - if (std::error_code EC = writeAggregatedFile(opts::OutputFilename)) { + if (std::error_code EC = writeAggregatedFile(opts::OutputFilename)) report_error("cannot create output data file", EC); - } } return Error::success(); @@ -844,10 +834,9 @@ bool DataAggregator::doTrace(const LBREntry &First, const LBREntry &Second, << FromFunc->getPrintName() << ":" << Twine::utohexstr(First.To) << " to " << Twine::utohexstr(Second.From) << ".\n"); - for (const std::pair &Pair : *FTs) { + for (const std::pair &Pair : *FTs) doIntraBranch(*FromFunc, Pair.first + FromFunc->getAddress(), Pair.second + FromFunc->getAddress(), Count, false); - } return true; } @@ -885,12 +874,11 @@ bool DataAggregator::recordTrace( BinaryBasicBlock *PrevBB = BF.BasicBlocksLayout[FromBB->getIndex() - 1]; if (PrevBB->getSuccessor(FromBB->getLabel())) { const MCInst *Instr = PrevBB->getLastNonPseudoInstr(); - if (Instr && BC.MIB->isCall(*Instr)) { + if (Instr && BC.MIB->isCall(*Instr)) FromBB = PrevBB; - } else { + else LLVM_DEBUG(dbgs() << "invalid incoming LBR (no call): " << FirstLBR << '\n'); - } } else { LLVM_DEBUG(dbgs() << "invalid incoming LBR: " << FirstLBR << '\n'); } @@ -899,9 +887,8 @@ bool DataAggregator::recordTrace( // Fill out information for fall-through edges. The From and To could be // within the same basic block, e.g. when two call instructions are in the // same block. In this case we skip the processing. - if (FromBB == ToBB) { + if (FromBB == ToBB) return true; - } // Process blocks in the original layout order. BinaryBasicBlock *BB = BF.BasicBlocksLayout[FromBB->getIndex()]; @@ -925,11 +912,11 @@ bool DataAggregator::recordTrace( if (Branches) { const MCInst *Instr = BB->getLastNonPseudoInstr(); uint64_t Offset = 0; - if (Instr) { + if (Instr) Offset = BC.MIB->getAnnotationWithDefault(*Instr, "Offset"); - } else { + else Offset = BB->getOffset(); - } + Branches->emplace_back(Offset, NextBB->getOffset()); } @@ -1039,9 +1026,9 @@ ErrorOr DataAggregator::parseLBREntry() { } bool DataAggregator::checkAndConsumeFS() { - if (ParsingBuf[0] != FieldSeparator) { + if (ParsingBuf[0] != FieldSeparator) return false; - } + ParsingBuf = ParsingBuf.drop_front(1); Col += 1; return true; @@ -1128,9 +1115,8 @@ ErrorOr DataAggregator::parseBasicSample() { } ErrorOr AddrRes = parseHexField(FieldSeparator, true); - if (std::error_code EC = AddrRes.getError()) { + if (std::error_code EC = AddrRes.getError()) return EC; - } if (!checkAndConsumeNewLine()) { reportError("expected end of line"); @@ -1175,9 +1161,8 @@ ErrorOr DataAggregator::parseMemSample() { } ErrorOr AddrRes = parseHexField(FieldSeparator); - if (std::error_code EC = AddrRes.getError()) { + if (std::error_code EC = AddrRes.getError()) return EC; - } while (checkAndConsumeFS()) { } @@ -1363,11 +1348,10 @@ std::error_code DataAggregator::printLBRHeatMap() { } HM.print(opts::HeatmapFile); - if (opts::HeatmapFile == "-") { + if (opts::HeatmapFile == "-") HM.printCDF(opts::HeatmapFile); - } else { + else HM.printCDF(opts::HeatmapFile + ".csv"); - } return std::error_code(); } @@ -1432,11 +1416,10 @@ std::error_code DataAggregator::parseBranchEvents() { getBinaryFunctionContainingAddress(TraceFrom); if (TraceBF && TraceBF->containsAddress(TraceTo)) { FTInfo &Info = FallthroughLBRs[Trace(TraceFrom, TraceTo)]; - if (TraceBF->containsAddress(LBR.From)) { + if (TraceBF->containsAddress(LBR.From)) ++Info.InternCount; - } else { + else ++Info.ExternCount; - } } else { if (TraceBF && getBinaryFunctionContainingAddress(TraceTo)) { LLVM_DEBUG(dbgs() @@ -1497,13 +1480,12 @@ std::error_code DataAggregator::parseBranchEvents() { auto printColored = [](raw_ostream &OS, float Percent, float T1, float T2) { OS << " ("; if (OS.has_colors()) { - if (Percent > T2) { + if (Percent > T2) OS.changeColor(raw_ostream::RED); - } else if (Percent > T1) { + else if (Percent > T1) OS.changeColor(raw_ostream::YELLOW); - } else { + else OS.changeColor(raw_ostream::GREEN); - } } OS << format("%.1f%%", Percent); if (OS.has_colors()) @@ -1527,10 +1509,9 @@ std::error_code DataAggregator::parseBranchEvents() { outs() << "PERF2BOLT: " << IgnoredSamples << " samples"; printColored(outs(), PercentIgnored, 20, 50); outs() << " were ignored\n"; - if (PercentIgnored > 50.0f) { + if (PercentIgnored > 50.0f) errs() << "PERF2BOLT-WARNING: less than 50% of all recorded samples " "were attributed to the input binary\n"; - } } } outs() << "PERF2BOLT: traces mismatching disassembled function contents: " @@ -1541,18 +1522,16 @@ std::error_code DataAggregator::parseBranchEvents() { printColored(outs(), Perc, 5, 10); } outs() << "\n"; - if (Perc > 10.0f) { + if (Perc > 10.0f) outs() << "\n !! WARNING !! This high mismatch ratio indicates the input " "binary is probably not the same binary used during profiling " "collection. The generated data may be ineffective for improving " "performance.\n\n"; - } outs() << "PERF2BOLT: out of range traces involving unknown regions: " << NumLongRangeTraces; - if (NumTraces > 0) { + if (NumTraces > 0) outs() << format(" (%.1f%%)", NumLongRangeTraces * 100.0f / NumTraces); - } outs() << "\n"; if (NumColdSamples > 0) { @@ -1560,12 +1539,11 @@ std::error_code DataAggregator::parseBranchEvents() { outs() << "PERF2BOLT: " << NumColdSamples << format(" (%.1f%%)", ColdSamples) << " samples recorded in cold regions of split functions.\n"; - if (ColdSamples > 5.0f) { + if (ColdSamples > 5.0f) outs() << "WARNING: The BOLT-processed binary where samples were collected " "likely used bad data or your service observed a large shift in " "profile. You may want to audit this.\n"; - } } return std::error_code(); @@ -1581,9 +1559,8 @@ void DataAggregator::processBranchEvents() { const FTInfo &Info = AggrLBR.second; LBREntry First{Loc.From, Loc.From, false}; LBREntry Second{Loc.To, Loc.To, false}; - if (Info.InternCount) { + if (Info.InternCount) doTrace(First, Second, Info.InternCount); - } if (Info.ExternCount) { First.From = 0; doTrace(First, Second, Info.ExternCount); @@ -1646,13 +1623,12 @@ void DataAggregator::processBasicEvents() { outs() << " ("; Perc = OutOfRangeSamples * 100.0f / NumSamples; if (outs().has_colors()) { - if (Perc > 60.0f) { + if (Perc > 60.0f) outs().changeColor(raw_ostream::RED); - } else if (Perc > 40.0f) { + else if (Perc > 40.0f) outs().changeColor(raw_ostream::YELLOW); - } else { + else outs().changeColor(raw_ostream::GREEN); - } } outs() << format("%.1f%%", Perc); if (outs().has_colors()) @@ -1660,12 +1636,11 @@ void DataAggregator::processBasicEvents() { outs() << ")"; } outs() << "\n"; - if (Perc > 80.0f) { + if (Perc > 80.0f) outs() << "\n !! WARNING !! This high mismatch ratio indicates the input " "binary is probably not the same binary used during profiling " "collection. The generated data may be ineffective for improving " "performance.\n\n"; - } } std::error_code DataAggregator::parseMemEvents() { @@ -1784,13 +1759,12 @@ void DataAggregator::processPreAggregated() { outs() << " ("; Perc = NumInvalidTraces * 100.0f / NumTraces; if (outs().has_colors()) { - if (Perc > 10.0f) { + if (Perc > 10.0f) outs().changeColor(raw_ostream::RED); - } else if (Perc > 5.0f) { + else if (Perc > 5.0f) outs().changeColor(raw_ostream::YELLOW); - } else { + else outs().changeColor(raw_ostream::GREEN); - } } outs() << format("%.1f%%", Perc); if (outs().has_colors()) @@ -1798,18 +1772,16 @@ void DataAggregator::processPreAggregated() { outs() << ")"; } outs() << "\n"; - if (Perc > 10.0f) { + if (Perc > 10.0f) outs() << "\n !! WARNING !! This high mismatch ratio indicates the input " "binary is probably not the same binary used during profiling " "collection. The generated data may be ineffective for improving " "performance.\n\n"; - } outs() << "PERF2BOLT: Out of range traces involving unknown regions: " << NumLongRangeTraces; - if (NumTraces > 0) { + if (NumTraces > 0) outs() << format(" (%.1f%%)", NumLongRangeTraces * 100.0f / NumTraces); - } outs() << "\n"; } @@ -1823,9 +1795,8 @@ Optional DataAggregator::parseCommExecEvent() { StringRef Line = ParsingBuf.substr(0, LineEnd); size_t Pos = Line.find("PERF_RECORD_COMM exec"); - if (Pos == StringRef::npos) { + if (Pos == StringRef::npos) return NoneType(); - } Line = Line.drop_front(Pos); // Line: @@ -1848,9 +1819,8 @@ Optional parsePerfTime(const StringRef TimeStr) { uint64_t SecTime; uint64_t USecTime; if (SecTimeStr.getAsInteger(10, SecTime) || - USecTimeStr.getAsInteger(10, USecTime)) { + USecTimeStr.getAsInteger(10, USecTime)) return NoneType(); - } return SecTime * 1000000ULL + USecTime; } } @@ -1930,9 +1900,8 @@ DataAggregator::parseMMapEvent() { const StringRef TimeStr = Line.substr(0, Pos).rsplit(':').first.rsplit(FieldSeparator).second; - if (Optional TimeRes = parsePerfTime(TimeStr)) { + if (Optional TimeRes = parsePerfTime(TimeStr)) ParsedInfo.Time = *TimeRes; - } Line = Line.drop_front(Pos); @@ -2012,12 +1981,11 @@ std::error_code DataAggregator::parseMMapEvents() { LLVM_DEBUG({ dbgs() << "FileName -> mmap info:\n"; - for (const std::pair &Pair : GlobalMMapInfo) { + for (const std::pair &Pair : GlobalMMapInfo) dbgs() << " " << Pair.first << " : " << Pair.second.PID << " [0x" << Twine::utohexstr(Pair.second.BaseAddress) << ", " << Twine::utohexstr(Pair.second.Size) << " @ " << Twine::utohexstr(Pair.second.Offset) << "]\n"; - } }); StringRef NameToUse = llvm::sys::path::filename(BC->getFilename()); @@ -2063,9 +2031,8 @@ std::error_code DataAggregator::parseMMapEvents() { if (!GlobalMMapInfo.empty()) { errs() << " Profile for the following binary name(s) is available:\n"; for (auto I = GlobalMMapInfo.begin(), IE = GlobalMMapInfo.end(); I != IE; - I = GlobalMMapInfo.upper_bound(I->first)) { + I = GlobalMMapInfo.upper_bound(I->first)) errs() << " " << I->first << '\n'; - } errs() << "Please rename the input binary.\n"; } else { errs() << " Failed to extract any binary name from a profile.\n"; @@ -2088,9 +2055,8 @@ std::error_code DataAggregator::parseTaskEvents() { if (Optional CommInfo = parseCommExecEvent()) { // Remove forked child that ran execve auto MMapInfoIter = BinaryMMapInfo.find(*CommInfo); - if (MMapInfoIter != BinaryMMapInfo.end() && MMapInfoIter->second.Forked) { + if (MMapInfoIter != BinaryMMapInfo.end() && MMapInfoIter->second.Forked) BinaryMMapInfo.erase(MMapInfoIter); - } consumeRestOfLine(); continue; } @@ -2122,11 +2088,10 @@ std::error_code DataAggregator::parseTaskEvents() { << BinaryMMapInfo.size() << " PID(s)\n"; LLVM_DEBUG({ - for (std::pair &MMI : BinaryMMapInfo) { + for (std::pair &MMI : BinaryMMapInfo) outs() << " " << MMI.second.PID << (MMI.second.Forked ? " (forked)" : "") << ": (0x" << Twine::utohexstr(MMI.second.BaseAddress) << ": 0x" << Twine::utohexstr(MMI.second.Size) << ")\n"; - } }); return std::error_code(); @@ -2187,9 +2152,8 @@ DataAggregator::writeAggregatedFile(StringRef OutputFilename) const { OutFile << "boltedcollection\n"; if (opts::BasicAggregation) { OutFile << "no_lbr"; - for (const StringMapEntry &Entry : EventNames) { + for (const StringMapEntry &Entry : EventNames) OutFile << " " << Entry.getKey(); - } OutFile << "\n"; for (const StringMapEntry &Func : NamesToSamples) { @@ -2246,9 +2210,8 @@ void DataAggregator::dump(const LBREntry &LBR) const { void DataAggregator::dump(const PerfBranchSample &Sample) const { Diag << "Sample LBR entries: " << Sample.LBR.size() << "\n"; - for (const LBREntry &LBR : Sample.LBR) { + for (const LBREntry &LBR : Sample.LBR) dump(LBR); - } } void DataAggregator::dump(const PerfMemSample &Sample) const { diff --git a/bolt/lib/Profile/DataReader.cpp b/bolt/lib/Profile/DataReader.cpp index e175693e6cd1..65e8e7362e7c 100644 --- a/bolt/lib/Profile/DataReader.cpp +++ b/bolt/lib/Profile/DataReader.cpp @@ -42,23 +42,21 @@ namespace bolt { Optional getLTOCommonName(const StringRef Name) { size_t LTOSuffixPos = Name.find(".lto_priv."); - if (LTOSuffixPos != StringRef::npos) { + if (LTOSuffixPos != StringRef::npos) return Name.substr(0, LTOSuffixPos + 10); - } else if ((LTOSuffixPos = Name.find(".constprop.")) != StringRef::npos) { + if ((LTOSuffixPos = Name.find(".constprop.")) != StringRef::npos) return Name.substr(0, LTOSuffixPos + 11); - } else { - return NoneType(); - } + return NoneType(); } namespace { /// Return true if the function name can change across compilations. bool hasVolatileName(const BinaryFunction &BF) { - for (const StringRef Name : BF.getNames()) { + for (const StringRef Name : BF.getNames()) if (getLTOCommonName(Name)) return true; - } + return false; } @@ -134,9 +132,8 @@ uint64_t FuncSampleData::getSamples(uint64_t Start, uint64_t End) const { uint64_t Result = 0; for (auto I = std::lower_bound(Data.begin(), Data.end(), Start, Compare()), E = std::lower_bound(Data.begin(), Data.end(), End, Compare()); - I != E; ++I) { + I != E; ++I) Result += I->Hits; - } return Result; } @@ -206,10 +203,10 @@ void BranchInfo::print(raw_ostream &OS) const { ErrorOr FuncBranchData::getBranch(uint64_t From, uint64_t To) const { - for (const BranchInfo &I : Data) { + for (const BranchInfo &I : Data) if (I.From.Offset == From && I.To.Offset == To && I.From.Name == I.To.Name) return I; - } + return make_error_code(llvm::errc::invalid_argument); } @@ -226,10 +223,10 @@ FuncBranchData::getDirectCallBranch(uint64_t From) const { } }; auto Range = std::equal_range(Data.begin(), Data.end(), From, Compare()); - for (auto I = Range.first; I != Range.second; ++I) { + for (auto I = Range.first; I != Range.second; ++I) if (I->From.Name != I->To.Name) return *I; - } + return make_error_code(llvm::errc::invalid_argument); } @@ -255,18 +252,15 @@ void FuncMemData::update(const Location &Offset, const Location &Addr) { } Error DataReader::preprocessProfile(BinaryContext &BC) { - if (std::error_code EC = parseInput()) { + if (std::error_code EC = parseInput()) return errorCodeToError(EC); - } - if (opts::DumpData) { + if (opts::DumpData) dump(); - } - if (collectedInBoltedBinary()) { + if (collectedInBoltedBinary()) outs() << "BOLT-INFO: profile collection done on a binary already " "processed by BOLT\n"; - } for (auto &BFI : BC.getBinaryFunctions()) { BinaryFunction &Function = BFI.second; @@ -308,17 +302,15 @@ Error DataReader::readProfilePreCFG(BinaryContext &BC) { BC.MIB->getOrCreateAnnotationAs( II->second, "MemoryAccessProfile"); BinaryData *BD = nullptr; - if (MI.Addr.IsSymbol) { + if (MI.Addr.IsSymbol) BD = BC.getBinaryDataByName(MI.Addr.Name); - } MemAccessProfile.AddressAccessInfo.push_back( {BD, MI.Addr.Offset, MI.Count}); auto NextII = std::next(II); - if (NextII == Function.Instructions.end()) { + if (NextII == Function.Instructions.end()) MemAccessProfile.NextInstrOffset = Function.getSize(); - } else { + else MemAccessProfile.NextInstrOffset = II->first; - } } Function.HasMemoryProfile = true; } @@ -350,13 +342,11 @@ std::error_code DataReader::parseInput() { } FileBuf = std::move(MB.get()); ParsingBuf = FileBuf->getBuffer(); - if (std::error_code EC = parse()) { + if (std::error_code EC = parse()) return EC; - } - if (!ParsingBuf.empty()) { + if (!ParsingBuf.empty()) Diag << "WARNING: invalid profile data detected at line " << Line << ". Possibly corrupted profile.\n"; - } buildLTONameMaps(); @@ -401,9 +391,8 @@ void DataReader::readProfile(BinaryFunction &BF) { uint64_t MismatchedBranches = 0; for (const BranchInfo &BI : FBD->Data) { - if (BI.From.Name != BI.To.Name) { + if (BI.From.Name != BI.To.Name) continue; - } if (!recordBranch(BF, BI.From.Offset, BI.To.Offset, BI.Branches, BI.Mispreds)) { @@ -523,9 +512,8 @@ float DataReader::evaluateProfileData(BinaryFunction &BF, // Until we define a minimal profile, we consider an empty branch data to be // a valid profile. It could happen to a function without branches when we // still have an EntryData for the execution count. - if (BranchData.Data.empty()) { + if (BranchData.Data.empty()) return 1.0f; - } uint64_t NumMatchedBranches = 0; for (const BranchInfo &BI : BranchData.Data) { @@ -549,9 +537,8 @@ float DataReader::evaluateProfileData(BinaryFunction &BF, if (Instr && BC.MIB->isPrefix(*Instr)) Instr = BF.getInstructionAtOffset(BI.From.Offset + 1); if (Instr && (BC.MIB->isCall(*Instr) || BC.MIB->isBranch(*Instr) || - BC.MIB->isReturn(*Instr))) { + BC.MIB->isReturn(*Instr))) IsValid = true; - } } if (IsValid) { @@ -567,12 +554,11 @@ float DataReader::evaluateProfileData(BinaryFunction &BF, } const float MatchRatio = (float)NumMatchedBranches / BranchData.Data.size(); - if (opts::Verbosity >= 2 && NumMatchedBranches < BranchData.Data.size()) { + if (opts::Verbosity >= 2 && NumMatchedBranches < BranchData.Data.size()) errs() << "BOLT-WARNING: profile branches match only " << format("%.1f%%", MatchRatio * 100.0f) << " (" << NumMatchedBranches << '/' << BranchData.Data.size() << ") for function " << BF << '\n'; - } return MatchRatio; } @@ -592,11 +578,11 @@ void DataReader::readSampleData(BinaryFunction &BF) { if (NagUser) { outs() << "BOLT-INFO: operating with basic samples profiling data (no LBR).\n"; - if (NormalizeByInsnCount) { + if (NormalizeByInsnCount) outs() << "BOLT-INFO: normalizing samples by instruction count.\n"; - } else if (NormalizeByCalls) { + else if (NormalizeByCalls) outs() << "BOLT-INFO: normalizing samples by branches.\n"; - } + NagUser = false; } uint64_t LastOffset = BF.getSize(); @@ -608,9 +594,9 @@ void DataReader::readSampleData(BinaryFunction &BF) { // later need to normalize numbers uint64_t NumSamples = SampleDataOrErr->getSamples(CurOffset, LastOffset) * 1000; - if (NormalizeByInsnCount && I->second->getNumNonPseudos()) + if (NormalizeByInsnCount && I->second->getNumNonPseudos()) { NumSamples /= I->second->getNumNonPseudos(); - else if (NormalizeByCalls) { + } else if (NormalizeByCalls) { uint32_t NumCalls = I->second->getNumCalls(); NumSamples /= NumCalls + 1; } @@ -655,11 +641,10 @@ void DataReader::convertBranchData(BinaryFunction &BF) const { continue; auto setOrUpdateAnnotation = [&](StringRef Name, uint64_t Count) { - if (opts::Verbosity >= 1 && BC.MIB->hasAnnotation(*Instr, Name)) { + if (opts::Verbosity >= 1 && BC.MIB->hasAnnotation(*Instr, Name)) errs() << "BOLT-WARNING: duplicate " << Name << " info for offset 0x" << Twine::utohexstr(BI.From.Offset) << " in function " << BF << '\n'; - } auto &Value = BC.MIB->getOrCreateAnnotationAs(*Instr, Name); Value += Count; }; @@ -670,9 +655,8 @@ void DataReader::convertBranchData(BinaryFunction &BF) const { *Instr, "CallProfile"); MCSymbol *CalleeSymbol = nullptr; if (BI.To.IsSymbol) { - if (BinaryData *BD = BC.getBinaryDataByName(BI.To.Name)) { + if (BinaryData *BD = BC.getBinaryDataByName(BI.To.Name)) CalleeSymbol = BD->getSymbol(); - } } CSP.emplace_back(CalleeSymbol, BI.Branches, BI.Mispreds); } else if (BC.MIB->getConditionalTailCall(*Instr)) { @@ -701,20 +685,18 @@ bool DataReader::recordBranch(BinaryFunction &BF, uint64_t From, uint64_t To, // BBs as a result of optimizations. In that case, a branch between these // two will be recorded as a branch from A going to A in the source address // space. Keep processing. - if (From == To) { + if (From == To) return true; - } // Return from a tail call. if (FromBB->succ_size() == 0) return true; // Very rarely we will see ignored branches. Do a linear check. - for (std::pair &Branch : BF.IgnoredBranches) { + for (std::pair &Branch : BF.IgnoredBranches) if (Branch == std::make_pair(static_cast(From), static_cast(To))) return true; - } bool OffsetMatches = !!(To == ToBB->getOffset()); if (!OffsetMatches) { @@ -742,9 +724,8 @@ bool DataReader::recordBranch(BinaryFunction &BF, uint64_t From, uint64_t To, BC.MIB->getAnnotationWithDefault(*LastInstr, "Offset"); // With old .fdata we are getting FT branches for "jcc,jmp" sequences. - if (To == LastInstrOffset && BC.MIB->isUnconditionalBranch(*LastInstr)) { + if (To == LastInstrOffset && BC.MIB->isUnconditionalBranch(*LastInstr)) return true; - } if (To <= LastInstrOffset) { LLVM_DEBUG(dbgs() << "branch recorded into the middle of the block" @@ -1163,15 +1144,13 @@ std::error_code DataReader::parseInNoLBRMode() { I->getValue().Data.emplace_back(std::move(MI)); } - for (StringMapEntry &FuncSamples : NamesToSamples) { + for (StringMapEntry &FuncSamples : NamesToSamples) std::stable_sort(FuncSamples.second.Data.begin(), FuncSamples.second.Data.end()); - } - for (StringMapEntry &MemEvents : NamesToMemEvents) { + for (StringMapEntry &MemEvents : NamesToMemEvents) std::stable_sort(MemEvents.second.Data.begin(), MemEvents.second.Data.end()); - } return std::error_code(); } @@ -1266,15 +1245,13 @@ std::error_code DataReader::parse() { I->getValue().Data.emplace_back(std::move(MI)); } - for (StringMapEntry &FuncBranches : NamesToBranches) { + for (StringMapEntry &FuncBranches : NamesToBranches) std::stable_sort(FuncBranches.second.Data.begin(), FuncBranches.second.Data.end()); - } - for (StringMapEntry &MemEvents : NamesToMemEvents) { + for (StringMapEntry &MemEvents : NamesToMemEvents) std::stable_sort(MemEvents.second.Data.begin(), MemEvents.second.Data.end()); - } return std::error_code(); } @@ -1343,9 +1320,8 @@ std::vector fetchMapEntriesRegex( } } else { auto I = Map.find(Name); - if (I != Map.end()) { + if (I != Map.end()) return {&I->getValue()}; - } } } return AllData; @@ -1419,15 +1395,13 @@ bool DataReader::hasLocalsWithFileName() const { void DataReader::dump() const { for (const StringMapEntry &Func : NamesToBranches) { Diag << Func.getKey() << " branches:\n"; - for (const BranchInfo &BI : Func.getValue().Data) { + for (const BranchInfo &BI : Func.getValue().Data) Diag << BI.From.Name << " " << BI.From.Offset << " " << BI.To.Name << " " << BI.To.Offset << " " << BI.Mispreds << " " << BI.Branches << "\n"; - } Diag << Func.getKey() << " entry points:\n"; - for (const BranchInfo &BI : Func.getValue().EntryData) { + for (const BranchInfo &BI : Func.getValue().EntryData) Diag << BI.From.Name << " " << BI.From.Offset << " " << BI.To.Name << " " << BI.To.Offset << " " << BI.Mispreds << " " << BI.Branches << "\n"; - } } for (auto I = EventNames.begin(), E = EventNames.end(); I != E; ++I) { @@ -1436,20 +1410,18 @@ void DataReader::dump() const { } for (const StringMapEntry &Func : NamesToSamples) { Diag << Func.getKey() << " samples:\n"; - for (const SampleInfo &SI : Func.getValue().Data) { + for (const SampleInfo &SI : Func.getValue().Data) Diag << SI.Loc.Name << " " << SI.Loc.Offset << " " << SI.Hits << "\n"; - } } for (const StringMapEntry &Func : NamesToMemEvents) { Diag << "Memory events for " << Func.getValue().Name; Location LastOffset(0); for (const MemInfo &MI : Func.getValue().Data) { - if (MI.Offset == LastOffset) { + if (MI.Offset == LastOffset) Diag << ", " << MI.Addr << "/" << MI.Count; - } else { + else Diag << "\n" << MI.Offset << ": " << MI.Addr << "/" << MI.Count; - } LastOffset = MI.Offset; } Diag << "\n"; diff --git a/bolt/lib/Profile/Heatmap.cpp b/bolt/lib/Profile/Heatmap.cpp index 220e057fe03e..9910aed777a1 100644 --- a/bolt/lib/Profile/Heatmap.cpp +++ b/bolt/lib/Profile/Heatmap.cpp @@ -41,9 +41,8 @@ void Heatmap::registerAddressRange(uint64_t StartAddress, uint64_t EndAddress, } for (uint64_t Bucket = StartAddress / BucketSize; - Bucket <= EndAddress / BucketSize; ++Bucket) { + Bucket <= EndAddress / BucketSize; ++Bucket) Map[Bucket] += Count; - } } void Heatmap::print(StringRef FileName) const { @@ -72,9 +71,8 @@ void Heatmap::print(raw_ostream &OS) const { // Calculate the max value for scaling. uint64_t MaxValue = 0; - for (const std::pair &Entry : Map) { + for (const std::pair &Entry : Map) MaxValue = std::max(MaxValue, Entry.second); - } // Print start of the line and fill it with an empty space right before // the Address. @@ -89,9 +87,8 @@ void Heatmap::print(raw_ostream &OS) const { if (Empty) Address = LineAddress + BytesPerLine; - for (uint64_t Fill = LineAddress; Fill < Address; Fill += BucketSize) { + for (uint64_t Fill = LineAddress; Fill < Address; Fill += BucketSize) OS << FillChar; - } }; // Finish line after \p Address was printed. @@ -151,11 +148,11 @@ void Heatmap::print(raw_ostream &OS) const { break; } } - if (Value <= Range[0]) { + if (Value <= Range[0]) OS << 'o'; - } else { + else OS << 'O'; - } + if (ResetColor) changeColor(DefaultColor); }; @@ -200,11 +197,10 @@ void Heatmap::print(raw_ostream &OS) const { const std::pair &Entry = *MI; uint64_t Address = Entry.first * BucketSize; - if (PrevAddress) { + if (PrevAddress) fillRange(PrevAddress, Address); - } else { + else startLine(Address); - } printValue(Entry.second); diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp index 7e20cb46ec66..1ccdb2e974f7 100644 --- a/bolt/lib/Profile/YAMLProfileReader.cpp +++ b/bolt/lib/Profile/YAMLProfileReader.cpp @@ -53,17 +53,14 @@ void YAMLProfileReader::buildNameMaps( if (Pos != StringRef::npos) Name = Name.substr(0, Pos); ProfileNameToProfile[Name] = &YamlBF; - if (const Optional CommonName = getLTOCommonName(Name)) { + if (const Optional CommonName = getLTOCommonName(Name)) LTOCommonNameMap[*CommonName].push_back(&YamlBF); - } } for (auto &BFI : Functions) { const BinaryFunction &Function = BFI.second; - for (StringRef Name : Function.getNames()) { - if (const Optional CommonName = getLTOCommonName(Name)) { + for (StringRef Name : Function.getNames()) + if (const Optional CommonName = getLTOCommonName(Name)) LTOCommonNameFunctionMap[*CommonName].insert(&Function); - } - } } } @@ -123,11 +120,11 @@ bool YAMLProfileReader::parseFunctionProfile( continue; } uint64_t NumSamples = YamlBB.EventCount * 1000; - if (NormalizeByInsnCount && BB.getNumNonPseudos()) { + if (NormalizeByInsnCount && BB.getNumNonPseudos()) NumSamples /= BB.getNumNonPseudos(); - } else if (NormalizeByCalls) { + else if (NormalizeByCalls) NumSamples /= BB.getNumCalls() + 1; - } + BB.setExecutionCount(NumSamples); if (BB.isEntryPoint()) FunctionExecutionCount += NumSamples; @@ -142,9 +139,9 @@ bool YAMLProfileReader::parseFunctionProfile( : nullptr; bool IsFunction = Callee ? true : false; MCSymbol *CalleeSymbol = nullptr; - if (IsFunction) { + if (IsFunction) CalleeSymbol = Callee->getSymbolForEntryID(YamlCSI.EntryDiscriminator); - } + BF.getAllCallSites().emplace_back(CalleeSymbol, YamlCSI.Count, YamlCSI.Mispreds, YamlCSI.Offset); @@ -221,10 +218,9 @@ bool YAMLProfileReader::parseFunctionProfile( } // If basic block profile wasn't read it should be 0. - for (BinaryBasicBlock &BB : BF) { + for (BinaryBasicBlock &BB : BF) if (BB.getExecutionCount() == BinaryBasicBlock::COUNT_NO_PROFILE) BB.setExecutionCount(0); - } if (YamlBP.Header.Flags & BinaryFunction::PF_SAMPLE) { BF.setExecutionCount(FunctionExecutionCount); @@ -236,11 +232,10 @@ bool YAMLProfileReader::parseFunctionProfile( if (ProfileMatched) BF.markProfiled(YamlBP.Header.Flags); - if (!ProfileMatched && opts::Verbosity >= 1) { + if (!ProfileMatched && opts::Verbosity >= 1) errs() << "BOLT-WARNING: " << MismatchedBlocks << " blocks, " << MismatchedCalls << " calls, and " << MismatchedEdges << " edges in profile did not match function " << BF << '\n'; - } return ProfileMatched; } @@ -263,16 +258,15 @@ Error YAMLProfileReader::preprocessProfile(BinaryContext &BC) { } // Sanity check. - if (YamlBP.Header.Version != 1) { + if (YamlBP.Header.Version != 1) return make_error( Twine("cannot read profile : unsupported version"), inconvertibleErrorCode()); - } - if (YamlBP.Header.EventNames.find(',') != StringRef::npos) { + + if (YamlBP.Header.EventNames.find(',') != StringRef::npos) return make_error( Twine("multiple events in profile are not supported"), inconvertibleErrorCode()); - } // Match profile to function based on a function name. buildNameMaps(BC.getBinaryFunctions()); @@ -298,9 +292,8 @@ bool YAMLProfileReader::mayHaveProfileData(const BinaryFunction &BF) { if (ProfileNameToProfile.find(Name) != ProfileNameToProfile.end()) return true; if (const Optional CommonName = getLTOCommonName(Name)) { - if (LTOCommonNameMap.find(*CommonName) != LTOCommonNameMap.end()) { + if (LTOCommonNameMap.find(*CommonName) != LTOCommonNameMap.end()) return true; - } } } @@ -336,9 +329,9 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) { for (StringRef FunctionName : Function.getNames()) { auto PI = ProfileNameToProfile.find(FunctionName); - if (PI == ProfileNameToProfile.end()) { + if (PI == ProfileNameToProfile.end()) continue; - } + yaml::bolt::BinaryFunctionProfile &YamlBF = *PI->getValue(); if (profileMatches(YamlBF, Function)) matchProfileToFunction(YamlBF, Function); @@ -394,12 +387,10 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) { } } - for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) { - if (!YamlBF.Used && opts::Verbosity >= 1) { + for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) + if (!YamlBF.Used && opts::Verbosity >= 1) errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name << '\n'; - } - } // Set for parseFunctionProfile(). NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions"); @@ -412,11 +403,10 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) { ++NumUnused; continue; } - if (BinaryFunction *BF = YamlProfileToFunction[YamlBF.Id]) { + if (BinaryFunction *BF = YamlProfileToFunction[YamlBF.Id]) parseFunctionProfile(*BF, YamlBF); - } else { + else ++NumUnused; - } } BC.setNumUnusedProfiledObjects(NumUnused); diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp index 11c0dcc6ff95..2b72de2c817d 100644 --- a/bolt/lib/Profile/YAMLProfileWriter.cpp +++ b/bolt/lib/Profile/YAMLProfileWriter.cpp @@ -68,9 +68,8 @@ void convert(const BinaryFunction &BF, CSI.EntryDiscriminator = 0; if (CSP.Symbol) { const BinaryFunction *Callee = BC.getFunctionForSymbol(CSP.Symbol); - if (Callee) { + if (Callee) CSI.DestId = Callee->getFunctionNumber(); - } } CSI.Count = CSP.Count; CSI.Mispreds = CSP.Mispreds; @@ -115,9 +114,8 @@ void convert(const BinaryFunction &BF, !(BB->isLandingPad() && BB->getKnownExecutionCount() != 0)) { uint64_t SuccessorExecCount = 0; for (const BinaryBasicBlock::BinaryBranchInfo &BranchInfo : - BB->branch_info()) { + BB->branch_info()) SuccessorExecCount += BranchInfo.Count; - } if (!SuccessorExecCount) continue; } @@ -175,9 +173,9 @@ std::error_code YAMLProfileWriter::writeProfile(const RewriteInstance &RI) { const BinaryFunction &BF = BFI.second; if (BF.hasProfile() && !BF.empty()) { assert(BF.getProfileFlags() != BinaryFunction::PF_NONE); - if (ProfileFlags == BinaryFunction::PF_NONE) { + if (ProfileFlags == BinaryFunction::PF_NONE) ProfileFlags = BF.getProfileFlags(); - } + assert(BF.getProfileFlags() == ProfileFlags && "expected consistent profile flags across all functions"); }