[polly] r269323 - Cleanup rejection log handling [NFC]

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Thu May 12 11:50:01 PDT 2016


Author: jdoerfert
Date: Thu May 12 13:50:01 2016
New Revision: 269323

URL: http://llvm.org/viewvc/llvm-project?rev=269323&view=rev
Log:
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.

Modified:
    polly/trunk/include/polly/ScopDetection.h
    polly/trunk/include/polly/ScopDetectionDiagnostic.h
    polly/trunk/lib/Analysis/ScopDetection.cpp
    polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp
    polly/trunk/lib/Analysis/ScopInfo.cpp
    polly/trunk/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll

Modified: polly/trunk/include/polly/ScopDetection.h
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopDetection.h?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/include/polly/ScopDetection.h (original)
+++ polly/trunk/include/polly/ScopDetection.h Thu May 12 13:50:01 2016
@@ -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<const Region *, DetectionContext>;
+  /// @brief Map to remember detection contexts for all regions.
+  using DetectionContextMapTy = DenseMap<BBPair, DetectionContext>;
   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<const Region *, RejectLog>::iterator reject_iterator;
-  typedef std::map<const Region *, RejectLog>::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.
-  ///
-  /// @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.
+  /// @brief Emit rejection remarks for all rejected regions.
   ///
   /// @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.

Modified: polly/trunk/include/polly/ScopDetectionDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopDetectionDiagnostic.h?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/include/polly/ScopDetectionDiagnostic.h (original)
+++ polly/trunk/include/polly/ScopDetectionDiagnostic.h Thu May 12 13:50:01 2016
@@ -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<BasicBlock *, BasicBlock *>;
+
+/// @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<const Region *, RejectLog> Logs;
-
-public:
-  typedef std::map<const Region *, RejectLog>::iterator iterator;
-  typedef std::map<const Region *, RejectLog>::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<iterator, bool>
-  insert(const std::pair<const Region *, RejectLog> &New) {
-    return Logs.insert(New);
-  }
-
-  std::map<const Region *, RejectLog>::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.
 ///

Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
+++ polly/trunk/lib/Analysis/ScopDetection.cpp Thu May 12 13:50:01 2016
@@ -262,10 +262,10 @@ bool ScopDetection::isMaxRegionInScop(co
     return false;
 
   if (Verify) {
-    DetectionContextMap.erase(&R);
-    const auto &It = DetectionContextMap.insert(
-        std::make_pair(&R, DetectionContext(const_cast<Region &>(R), *AA,
-                                            false /*verifying*/)));
+    DetectionContextMap.erase(getBBPairForRegion(&R));
+    const auto &It = DetectionContextMap.insert(std::make_pair(
+        getBBPairForRegion(&R),
+        DetectionContext(const_cast<Region &>(R), *AA, false /*verifying*/)));
     DetectionContext &Context = It.first->second;
     return isValidRegion(Context);
   }
@@ -274,19 +274,16 @@ bool ScopDetection::isMaxRegionInScop(co
 }
 
 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(Regi
 
   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::removeCachedResu
 
 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(D
   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:
   }
 }
 
-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<Region> &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::
   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 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
   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 &O
 }
 
 void ScopDetection::releaseMemory() {
-  RejectLogs.clear();
   ValidRegions.clear();
   DetectionContextMap.clear();
 

Modified: polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp (original)
+++ polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp Thu May 12 13:50:01 2016
@@ -36,6 +36,8 @@
 
 #include <string>
 
+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::DebugL
 }
 
 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<BasicBlock *, 32> Seen;
+  SmallVector<BasicBlock *, 32> 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,
       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,

Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
+++ polly/trunk/lib/Analysis/ScopInfo.cpp Thu May 12 13:50:01 2016
@@ -4909,7 +4909,7 @@ bool ScopInfo::runOnRegion(Region *R, RG
   auto &AC = getAnalysis<AssumptionCacheTracker>().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);
 

Modified: polly/trunk/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll?rev=269323&r1=269322&r2=269323&view=diff
==============================================================================
--- polly/trunk/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll (original)
+++ polly/trunk/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll Thu May 12 13:50:01 2016
@@ -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"




More information about the llvm-commits mailing list