[llvm] r320180 - Revert r320104: infinite loop profiling bug fix

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 8 11:38:07 PST 2017


Author: davidxl
Date: Fri Dec  8 11:38:07 2017
New Revision: 320180

URL: http://llvm.org/viewvc/llvm-project?rev=320180&view=rev
Log:
Revert r320104: infinite loop profiling bug fix

Causes unexpected memory issue with New PM this time.
The new PM invalidates BPI but not BFI, leaving the
reference to BPI from BFI invalid.

Abandon this patch.  There is a more general solution
which also handles runtime infinite loop (but not statically).


Removed:
    llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll
Modified:
    llvm/trunk/include/llvm/Analysis/BlockFrequencyInfo.h
    llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
    llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp
    llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h
    llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

Modified: llvm/trunk/include/llvm/Analysis/BlockFrequencyInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/BlockFrequencyInfo.h?rev=320180&r1=320179&r2=320180&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/BlockFrequencyInfo.h (original)
+++ llvm/trunk/include/llvm/Analysis/BlockFrequencyInfo.h Fri Dec  8 11:38:07 2017
@@ -56,7 +56,6 @@ public:
 
   const Function *getFunction() const;
   const BranchProbabilityInfo *getBPI() const;
-  const LoopInfo *getLoopInfo() const;
   void view() const;
 
   /// getblockFreq - Return block frequency. Return 0 if we don't have the

Modified: llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h?rev=320180&r1=320179&r2=320180&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h (original)
+++ llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h Fri Dec  8 11:38:07 2017
@@ -993,7 +993,6 @@ public:
   }
 
   const BranchProbabilityInfoT &getBPI() const { return *BPI; }
-  const LoopInfoT &getLoopInfo() const { return *LI; }
 
   /// \brief Print the frequencies for the current function.
   ///

Modified: llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp?rev=320180&r1=320179&r2=320180&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp (original)
+++ llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp Fri Dec  8 11:38:07 2017
@@ -264,10 +264,6 @@ const BranchProbabilityInfo *BlockFreque
   return BFI ? &BFI->getBPI() : nullptr;
 }
 
-const LoopInfo *BlockFrequencyInfo::getLoopInfo() const {
-  return BFI ? &BFI->getLoopInfo() : nullptr;
-}
-
 raw_ostream &BlockFrequencyInfo::
 printBlockFreq(raw_ostream &OS, const BlockFrequency Freq) const {
   return BFI ? BFI->printBlockFreq(OS, Freq) : OS;

Modified: llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h?rev=320180&r1=320179&r2=320180&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h Fri Dec  8 11:38:07 2017
@@ -20,7 +20,6 @@
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/CFG.h"
-#include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -137,21 +136,6 @@ public:
                      << " w = " << BBWeight << "\n");
       }
     }
-    // check if there is any infinite loop. If yes, add a fake edge from
-    // the header block to the fake node:
-    for (auto *L : *LI) {
-      SmallVector<BasicBlock *, 2> ExitingBlocks;
-      L->getExitingBlocks(ExitingBlocks);
-      if (!ExitingBlocks.empty())
-        continue;
-      auto *HB = L->getHeader();
-      if (!HB)
-        continue;
-      addEdge(HB, nullptr, UINT64_MAX);
-      DEBUG(dbgs() << "  Edge: from infinite loop header " << HB->getName()
-                   << " to exit"
-                   << " w = " << UINT64_MAX << "\n");
-    }
   }
 
   // Sort CFG edges based on its weight.
@@ -226,13 +210,13 @@ public:
     return *AllEdges.back();
   }
 
-  const BranchProbabilityInfo *BPI;
+  BranchProbabilityInfo *BPI;
   BlockFrequencyInfo *BFI;
-  const LoopInfo *LI;
 
 public:
-  CFGMST(Function &Func, BlockFrequencyInfo *BFI_ = nullptr)
-      : F(Func), BPI(BFI_->getBPI()), BFI(BFI_), LI(BFI_->getLoopInfo()) {
+  CFGMST(Function &Func, BranchProbabilityInfo *BPI_ = nullptr,
+         BlockFrequencyInfo *BFI_ = nullptr)
+      : F(Func), BPI(BPI_), BFI(BFI_) {
     buildEdges();
     sortEdgesByWeight();
     computeMinimumSpanningTree();

Modified: llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp?rev=320180&r1=320179&r2=320180&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp Fri Dec  8 11:38:07 2017
@@ -425,6 +425,7 @@ char PGOInstrumentationGenLegacyPass::ID
 INITIALIZE_PASS_BEGIN(PGOInstrumentationGenLegacyPass, "pgo-instr-gen",
                       "PGO instrumentation.", false, false)
 INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)
 INITIALIZE_PASS_END(PGOInstrumentationGenLegacyPass, "pgo-instr-gen",
                     "PGO instrumentation.", false, false)
 
@@ -437,6 +438,7 @@ char PGOInstrumentationUseLegacyPass::ID
 INITIALIZE_PASS_BEGIN(PGOInstrumentationUseLegacyPass, "pgo-instr-use",
                       "Read PGO instrumentation profile.", false, false)
 INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)
 INITIALIZE_PASS_END(PGOInstrumentationUseLegacyPass, "pgo-instr-use",
                     "Read PGO instrumentation profile.", false, false)
 
@@ -527,9 +529,10 @@ public:
   FuncPGOInstrumentation(
       Function &Func,
       std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
-      bool CreateGlobalVar = false, BlockFrequencyInfo *BFI = nullptr)
+      bool CreateGlobalVar = false, BranchProbabilityInfo *BPI = nullptr,
+      BlockFrequencyInfo *BFI = nullptr)
       : F(Func), ComdatMembers(ComdatMembers), ValueSites(IPVK_Last + 1),
-        SIVisitor(Func), MIVisitor(Func), MST(F, BFI) {
+        SIVisitor(Func), MIVisitor(Func), MST(F, BPI, BFI) {
     // This should be done before CFG hash computation.
     SIVisitor.countSelects(Func);
     MIVisitor.countMemIntrinsics(Func);
@@ -711,9 +714,10 @@ BasicBlock *FuncPGOInstrumentation<Edge,
 // Visit all edge and instrument the edges not in MST, and do value profiling.
 // Critical edges will be split.
 static void instrumentOneFunc(
-    Function &F, Module *M, BlockFrequencyInfo *BFI,
+    Function &F, Module *M, BranchProbabilityInfo *BPI, BlockFrequencyInfo *BFI,
     std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
-  FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, ComdatMembers, true, BFI);
+  FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, ComdatMembers, true, BPI,
+                                                   BFI);
   unsigned NumCounters = FuncInfo.getNumCounters();
 
   uint32_t I = 0;
@@ -839,9 +843,11 @@ class PGOUseFunc {
 public:
   PGOUseFunc(Function &Func, Module *Modu,
              std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
+             BranchProbabilityInfo *BPI = nullptr,
              BlockFrequencyInfo *BFIin = nullptr)
       : F(Func), M(Modu), BFI(BFIin),
-        FuncInfo(Func, ComdatMembers, false, BFIin), FreqAttr(FFA_Normal) {}
+        FuncInfo(Func, ComdatMembers, false, BPI, BFIin),
+        FreqAttr(FFA_Normal) {}
 
   // Read counts for the instrumented BB from profile.
   bool readCounters(IndexedInstrProfReader *PGOReader);
@@ -1372,7 +1378,8 @@ static void collectComdatMembers(
 }
 
 static bool InstrumentAllFunctions(
-    Module &M, function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
+    Module &M, function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
+    function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
   createIRLevelProfileFlagVariable(M);
   std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
   collectComdatMembers(M, ComdatMembers);
@@ -1380,8 +1387,9 @@ static bool InstrumentAllFunctions(
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
+    auto *BPI = LookupBPI(F);
     auto *BFI = LookupBFI(F);
-    instrumentOneFunc(F, &M, BFI, ComdatMembers);
+    instrumentOneFunc(F, &M, BPI, BFI, ComdatMembers);
   }
   return true;
 }
@@ -1390,21 +1398,27 @@ bool PGOInstrumentationGenLegacyPass::ru
   if (skipModule(M))
     return false;
 
+  auto LookupBPI = [this](Function &F) {
+    return &this->getAnalysis<BranchProbabilityInfoWrapperPass>(F).getBPI();
+  };
   auto LookupBFI = [this](Function &F) {
     return &this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
   };
-  return InstrumentAllFunctions(M, LookupBFI);
+  return InstrumentAllFunctions(M, LookupBPI, LookupBFI);
 }
 
 PreservedAnalyses PGOInstrumentationGen::run(Module &M,
                                              ModuleAnalysisManager &AM) {
   auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+  auto LookupBPI = [&FAM](Function &F) {
+    return &FAM.getResult<BranchProbabilityAnalysis>(F);
+  };
 
   auto LookupBFI = [&FAM](Function &F) {
     return &FAM.getResult<BlockFrequencyAnalysis>(F);
   };
 
-  if (!InstrumentAllFunctions(M, LookupBFI))
+  if (!InstrumentAllFunctions(M, LookupBPI, LookupBFI))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
@@ -1412,6 +1426,7 @@ PreservedAnalyses PGOInstrumentationGen:
 
 static bool annotateAllFunctions(
     Module &M, StringRef ProfileFileName,
+    function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
     function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
   DEBUG(dbgs() << "Read in profile counters: ");
   auto &Ctx = M.getContext();
@@ -1446,8 +1461,9 @@ static bool annotateAllFunctions(
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
+    auto *BPI = LookupBPI(F);
     auto *BFI = LookupBFI(F);
-    PGOUseFunc Func(F, &M, ComdatMembers, BFI);
+    PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI);
     if (!Func.readCounters(PGOReader.get()))
       continue;
     Func.populateCounters();
@@ -1515,12 +1531,15 @@ PreservedAnalyses PGOInstrumentationUse:
                                              ModuleAnalysisManager &AM) {
 
   auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+  auto LookupBPI = [&FAM](Function &F) {
+    return &FAM.getResult<BranchProbabilityAnalysis>(F);
+  };
 
   auto LookupBFI = [&FAM](Function &F) {
     return &FAM.getResult<BlockFrequencyAnalysis>(F);
   };
 
-  if (!annotateAllFunctions(M, ProfileFileName, LookupBFI))
+  if (!annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
@@ -1530,11 +1549,14 @@ bool PGOInstrumentationUseLegacyPass::ru
   if (skipModule(M))
     return false;
 
+  auto LookupBPI = [this](Function &F) {
+    return &this->getAnalysis<BranchProbabilityInfoWrapperPass>(F).getBPI();
+  };
   auto LookupBFI = [this](Function &F) {
     return &this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
   };
 
-  return annotateAllFunctions(M, ProfileFileName, LookupBFI);
+  return annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI);
 }
 
 static std::string getSimpleNodeName(const BasicBlock *Node) {

Removed: llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll?rev=320179&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll (removed)
@@ -1,18 +0,0 @@
-; RUN: opt < %s -pgo-instr-gen -S -o -   | FileCheck %s
-
-define void @foo() {
-entry:
-  br label %while.body
-; CHECK: llvm.instrprof.increment
-
-  while.body:                                       ; preds = %entry, %while.body
-; CHECK: llvm.instrprof.increment
-    call void (...) @bar() #2
-    br label %while.body
-}
-
-declare void @bar(...)
-
-
-attributes #0 = { nounwind }
-




More information about the llvm-commits mailing list