From 6c7639b380a3830c60c52525e470767a64f57248 Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Thu, 12 May 2016 18:50:01 +0000 Subject: [PATCH] Cleanup rejection log handling [NFC] This patch cleans up the rejection log handling during the ScopDetection. It consists of two interconnected parts: - We keep all detection contexts for a function in order to provide more information to the user, e.g., about the rejection of extended/intermediate regions. - We remove the mutable "RejectLogs" member as the information is available through the detection contexts. llvm-svn: 269323 --- polly/include/polly/ScopDetection.h | 42 ++-------- polly/include/polly/ScopDetectionDiagnostic.h | 58 +++---------- polly/lib/Analysis/ScopDetection.cpp | 82 +++++++------------ .../lib/Analysis/ScopDetectionDiagnostic.cpp | 27 ++++-- polly/lib/Analysis/ScopInfo.cpp | 2 +- .../ReportLoopBound-01.ll | 8 +- 6 files changed, 75 insertions(+), 144 deletions(-) diff --git a/polly/include/polly/ScopDetection.h b/polly/include/polly/ScopDetection.h index 0431af0150de..a3ef86d14bfe 100644 --- a/polly/include/polly/ScopDetection.h +++ b/polly/include/polly/ScopDetection.h @@ -136,6 +136,8 @@ public: Region &CurRegion; // The region to check. AliasSetTracker AST; // The AliasSetTracker to hold the alias information. bool Verifying; // If we are in the verification phase? + + /// @brief Container to remember rejection reasons for this region. RejectLog Log; /// @brief Map a base pointer to all access functions accessing it. @@ -214,13 +216,10 @@ private: /// BLACK - Visited and completely processed BB. enum Color { WHITE, GREY, BLACK }; - /// @brief Map to remember detection contexts for valid regions. - using DetectionContextMapTy = DenseMap; + /// @brief Map to remember detection contexts for all regions. + using DetectionContextMapTy = DenseMap; mutable DetectionContextMapTy DetectionContextMap; - // Remember a list of errors for every region. - mutable RejectLogsContainer RejectLogs; - /// @brief Remove cached results for @p R. void removeCachedResults(const Region &R); @@ -551,6 +550,9 @@ public: /// @brief Return the set of required invariant loads for @p R. const InvariantLoadsSetTy *getRequiredInvariantLoads(const Region *R) const; + /// @brief Return the set of rejection causes for @p R. + const RejectLog *lookupRejectionLog(const Region *R) const; + /// @brief Return true if @p SubR is a non-affine subregion in @p ScopR. bool isNonAffineSubRegion(const Region *SubR, const Region *ScopR) const; @@ -576,36 +578,10 @@ public: const_iterator end() const { return ValidRegions.end(); } //@} - /// @name Reject log iterators - /// - /// These iterators iterate over the logs of all rejected regions of this - // function. - //@{ - typedef std::map::iterator reject_iterator; - typedef std::map::const_iterator - const_reject_iterator; - - reject_iterator reject_begin() { return RejectLogs.begin(); } - reject_iterator reject_end() { return RejectLogs.end(); } - - const_reject_iterator reject_begin() const { return RejectLogs.begin(); } - const_reject_iterator reject_end() const { return RejectLogs.end(); } - //@} - - /// @brief Emit rejection remarks for all smallest invalid regions. + /// @brief Emit rejection remarks for all rejected regions. /// /// @param F The function to emit remarks for. - /// @param R The region to start the region tree traversal for. - void emitMissedRemarksForLeaves(const Function &F, const Region *R); - - /// @brief Emit rejection remarks for the parent regions of all valid regions. - /// - /// Emitting rejection remarks for the parent regions of all valid regions - /// may give the end-user clues about how to increase the size of the - /// detected Scops. - /// - /// @param F The function to emit remarks for. - void emitMissedRemarksForValidRegions(const Function &F); + void emitMissedRemarks(const Function &F); /// @brief Mark the function as invalid so we will not extract any scop from /// the function. diff --git a/polly/include/polly/ScopDetectionDiagnostic.h b/polly/include/polly/ScopDetectionDiagnostic.h index 6db80964b081..76938b5f78e5 100644 --- a/polly/include/polly/ScopDetectionDiagnostic.h +++ b/polly/include/polly/ScopDetectionDiagnostic.h @@ -43,17 +43,23 @@ class Region; namespace polly { -/// @brief Set the begin and end source location for the given region @p R. -void getDebugLocations(const Region *R, DebugLoc &Begin, DebugLoc &End); +/// @brief Type to hold region delimitors (entry & exit block). +using BBPair = std::pair; + +/// @brief Return the region delimitors (entry & exit block) of @p R. +BBPair getBBPairForRegion(const Region *R); + +/// @brief Set the begin and end source location for the region limited by @p P. +void getDebugLocations(const BBPair &P, DebugLoc &Begin, DebugLoc &End); class RejectLog; /// @brief Emit optimization remarks about the rejected regions to the user. /// /// This emits the content of the reject log as optimization remarks. /// Remember to at least track failures (-polly-detect-track-failures). -/// @param F The function we emit remarks for. +/// @param P The region delimitors (entry & exit) we emit remarks for. /// @param Log The error log containing all messages being emitted as remark. -void emitRejectionRemarks(const llvm::Function &F, const RejectLog &Log); +void emitRejectionRemarks(const BBPair &P, const RejectLog &Log); // Discriminator for LLVM-style RTTI (dyn_cast<> et al.) enum RejectReasonKind { @@ -165,50 +171,6 @@ public: void report(RejectReasonPtr Reject) { ErrorReports.push_back(Reject); } }; -/// @brief Store reject logs -class RejectLogsContainer { - std::map Logs; - -public: - typedef std::map::iterator iterator; - typedef std::map::const_iterator const_iterator; - - iterator begin() { return Logs.begin(); } - iterator end() { return Logs.end(); } - - const_iterator begin() const { return Logs.begin(); } - const_iterator end() const { return Logs.end(); } - - std::pair - insert(const std::pair &New) { - return Logs.insert(New); - } - - std::map::mapped_type at(const Region *R) { - return Logs.at(R); - } - - void clear() { Logs.clear(); } - - size_t count(const Region *R) const { return Logs.count(R); } - - size_t size(const Region *R) const { - if (!Logs.count(R)) - return 0; - return Logs.at(R).size(); - } - - bool hasErrors(const Region *R) const { - if (!Logs.count(R)) - return false; - - RejectLog Log = Logs.at(R); - return Log.hasErrors(); - } - - bool hasErrors(Region *R) const { return hasErrors((const Region *)R); } -}; - //===----------------------------------------------------------------------===// /// @brief Base class for CFG related reject reasons. /// diff --git a/polly/lib/Analysis/ScopDetection.cpp b/polly/lib/Analysis/ScopDetection.cpp index 34f439bae0bb..fedca6fd2062 100644 --- a/polly/lib/Analysis/ScopDetection.cpp +++ b/polly/lib/Analysis/ScopDetection.cpp @@ -262,10 +262,10 @@ bool ScopDetection::isMaxRegionInScop(const Region &R, bool Verify) const { return false; if (Verify) { - DetectionContextMap.erase(&R); - const auto &It = DetectionContextMap.insert( - std::make_pair(&R, DetectionContext(const_cast(R), *AA, - false /*verifying*/))); + DetectionContextMap.erase(getBBPairForRegion(&R)); + const auto &It = DetectionContextMap.insert(std::make_pair( + getBBPairForRegion(&R), + DetectionContext(const_cast(R), *AA, false /*verifying*/))); DetectionContext &Context = It.first->second; return isValidRegion(Context); } @@ -274,19 +274,16 @@ bool ScopDetection::isMaxRegionInScop(const Region &R, bool Verify) const { } std::string ScopDetection::regionIsInvalidBecause(const Region *R) const { - if (!RejectLogs.count(R)) - return ""; - // Get the first error we found. Even in keep-going mode, this is the first // reason that caused the candidate to be rejected. - RejectLog Errors = RejectLogs.at(R); + auto *Log = lookupRejectionLog(R); // This can happen when we marked a region invalid, but didn't track // an error for it. - if (Errors.size() == 0) + if (!Log || !Log->hasErrors()) return ""; - RejectReasonPtr RR = *Errors.begin(); + RejectReasonPtr RR = *Log->begin(); return RR->getMessage(); } @@ -1093,7 +1090,7 @@ Region *ScopDetection::expandRegion(Region &R) { while (ExpandedRegion) { const auto &It = DetectionContextMap.insert(std::make_pair( - ExpandedRegion.get(), + getBBPairForRegion(ExpandedRegion.get()), DetectionContext(*ExpandedRegion, *AA, false /*verifying*/))); DetectionContext &Context = It.first->second; DEBUG(dbgs() << "\t\tTrying " << ExpandedRegion->getNameStr() << "\n"); @@ -1156,12 +1153,11 @@ unsigned ScopDetection::removeCachedResultsRecursively(const Region &R) { void ScopDetection::removeCachedResults(const Region &R) { ValidRegions.remove(&R); - DetectionContextMap.erase(&R); } void ScopDetection::findScops(Region &R) { - const auto &It = DetectionContextMap.insert( - std::make_pair(&R, DetectionContext(R, *AA, false /*verifying*/))); + const auto &It = DetectionContextMap.insert(std::make_pair( + getBBPairForRegion(&R), DetectionContext(R, *AA, false /*verifying*/))); DetectionContext &Context = It.first->second; bool RegionIsValid = false; @@ -1172,9 +1168,6 @@ void ScopDetection::findScops(Region &R) { bool HasErrors = !RegionIsValid || Context.Log.size() > 0; - if (PollyTrackFailures && HasErrors) - RejectLogs.insert(std::make_pair(&R, Context.Log)); - if (HasErrors) { removeCachedResults(R); } else { @@ -1198,16 +1191,16 @@ void ScopDetection::findScops(Region &R) { ToExpand.push_back(SubRegion.get()); for (Region *CurrentRegion : ToExpand) { - // Skip regions that had errors. - bool HadErrors = RejectLogs.hasErrors(CurrentRegion); - if (HadErrors) - continue; - // Skip invalid regions. Regions may become invalid, if they are element of // an already expanded region. if (!ValidRegions.count(CurrentRegion)) continue; + // Skip regions that had errors. + bool HadErrors = lookupRejectionLog(CurrentRegion)->hasErrors(); + if (HadErrors) + continue; + Region *ExpandedR = expandRegion(*CurrentRegion); if (!ExpandedR) @@ -1295,6 +1288,7 @@ bool ScopDetection::isProfitableRegion(DetectionContext &Context) const { if (PollyProcessUnprofitable) return true; + errs() << "Context: " << Context.CurRegion << "\n"; // We can probably not do a lot on scops that only write or only read // data. if (!Context.hasStores || !Context.hasLoads) @@ -1382,29 +1376,11 @@ void ScopDetection::printLocations(llvm::Function &F) { } } -void ScopDetection::emitMissedRemarksForValidRegions(const Function &F) { - for (const Region *R : ValidRegions) { - const Region *Parent = R->getParent(); - if (Parent && !Parent->isTopLevelRegion() && RejectLogs.count(Parent)) - emitRejectionRemarks(F, RejectLogs.at(Parent)); - } -} - -void ScopDetection::emitMissedRemarksForLeaves(const Function &F, - const Region *R) { - for (const std::unique_ptr &Child : *R) { - bool IsValid = DetectionContextMap.count(Child.get()); - if (IsValid) - continue; - - bool IsLeaf = Child->begin() == Child->end(); - if (!IsLeaf) - emitMissedRemarksForLeaves(F, Child.get()); - else { - if (RejectLogs.count(Child.get())) { - emitRejectionRemarks(F, RejectLogs.at(Child.get())); - } - } +void ScopDetection::emitMissedRemarks(const Function &F) { + for (auto &DIt : DetectionContextMap) { + auto &DC = DIt.getSecond(); + if (DC.Log.hasErrors()) + emitRejectionRemarks(DIt.getFirst(), DC.Log); } } @@ -1497,15 +1473,13 @@ bool ScopDetection::runOnFunction(llvm::Function &F) { findScops(*TopRegion); // Only makes sense when we tracked errors. - if (PollyTrackFailures) { - emitMissedRemarksForValidRegions(F); - emitMissedRemarksForLeaves(F, TopRegion); - } + if (PollyTrackFailures) + emitMissedRemarks(F); if (ReportLevel) printLocations(F); - assert(ValidRegions.size() == DetectionContextMap.size() && + assert(ValidRegions.size() <= DetectionContextMap.size() && "Cached more results than valid regions"); return false; } @@ -1519,7 +1493,7 @@ bool ScopDetection::isNonAffineSubRegion(const Region *SubR, const ScopDetection::DetectionContext * ScopDetection::getDetectionContext(const Region *R) const { - auto DCMIt = DetectionContextMap.find(R); + auto DCMIt = DetectionContextMap.find(getBBPairForRegion(R)); if (DCMIt == DetectionContextMap.end()) return nullptr; return &DCMIt->second; @@ -1546,6 +1520,11 @@ ScopDetection::getRequiredInvariantLoads(const Region *R) const { return &DC->RequiredILS; } +const RejectLog *ScopDetection::lookupRejectionLog(const Region *R) const { + const DetectionContext *DC = getDetectionContext(R); + return DC ? &DC->Log : nullptr; +} + void polly::ScopDetection::verifyRegion(const Region &R) const { assert(isMaxRegionInScop(R) && "Expect R is a valid region."); @@ -1579,7 +1558,6 @@ void ScopDetection::print(raw_ostream &OS, const Module *) const { } void ScopDetection::releaseMemory() { - RejectLogs.clear(); ValidRegions.clear(); DetectionContextMap.clear(); diff --git a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp index 25b121bc3aef..6b6b389895ce 100644 --- a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp +++ b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp @@ -36,6 +36,8 @@ #include +using namespace llvm; + #define BADSCOP_STAT(NAME, DESC) \ STATISTIC(Bad##NAME##ForScop, "Number of bad regions for Scop: " DESC) @@ -67,8 +69,21 @@ static bool operator<(const llvm::DebugLoc &LHS, const llvm::DebugLoc &RHS) { } namespace polly { -void getDebugLocations(const Region *R, DebugLoc &Begin, DebugLoc &End) { - for (const BasicBlock *BB : R->blocks()) +BBPair getBBPairForRegion(const Region *R) { + return std::make_pair(R->getEntry(), R->getExit()); +} + +void getDebugLocations(const BBPair &P, DebugLoc &Begin, DebugLoc &End) { + SmallPtrSet Seen; + SmallVector Todo; + Todo.push_back(P.first); + while (!Todo.empty()) { + auto *BB = Todo.pop_back_val(); + if (BB == P.second) + continue; + if (!Seen.insert(BB).second) + continue; + Todo.append(succ_begin(BB), succ_end(BB)); for (const Instruction &Inst : *BB) { DebugLoc DL = Inst.getDebugLoc(); if (!DL) @@ -77,15 +92,15 @@ void getDebugLocations(const Region *R, DebugLoc &Begin, DebugLoc &End) { Begin = Begin ? std::min(Begin, DL) : DL; End = End ? std::max(End, DL) : DL; } + } } -void emitRejectionRemarks(const llvm::Function &F, const RejectLog &Log) { +void emitRejectionRemarks(const BBPair &P, const RejectLog &Log) { + Function &F = *P.first->getParent(); LLVMContext &Ctx = F.getContext(); - const Region *R = Log.region(); DebugLoc Begin, End; - - getDebugLocations(R, Begin, End); + getDebugLocations(P, Begin, End); emitOptimizationRemarkMissed( Ctx, DEBUG_TYPE, F, Begin, diff --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp index 954b55dfb456..64b4fd1feaed 100644 --- a/polly/lib/Analysis/ScopInfo.cpp +++ b/polly/lib/Analysis/ScopInfo.cpp @@ -4909,7 +4909,7 @@ bool ScopInfo::runOnRegion(Region *R, RGPassManager &RGM) { auto &AC = getAnalysis().getAssumptionCache(*F); DebugLoc Beg, End; - getDebugLocations(R, Beg, End); + getDebugLocations(getBBPairForRegion(R), Beg, End); std::string Msg = "SCoP begins here."; emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F, Beg, Msg); diff --git a/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll b/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll index 1d4c8c334f66..e81bce5a1276 100644 --- a/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll +++ b/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll @@ -19,20 +19,20 @@ ; If we reject non-affine loops the non-affine loop bound will be reported: ; -; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop. +; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop. ; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: Failed to derive an affine function from the loop bounds. ; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here. ; If we allow non-affine loops the non-affine access will be reported: ; -; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop. +; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop. ; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: The array subscript of "A" is not affine ; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here. ; If we allow non-affine loops and non-affine accesses the region will be reported as not profitable: ; -; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop. -; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:3:5: No profitable polyhedral optimization found +; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop. +; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:1:12: No profitable polyhedral optimization found ; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here. target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"