[llvm] r319794 - [PGO] detect infinite loop and form MST properly

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 5 13:55:42 PST 2017


Reverted.

There is some unexpected interaction from onTheFlyFP.

David

On Tue, Dec 5, 2017 at 12:21 PM, Vlad Tsyrklevich <vlad at tsyrklevich.net>
wrote:

> Hello, this change seems to be causing some LeakSanitizer test failures,
> e.g. http://lab.llvm.org:8011/builders/sanitizer-x86_64-
> linux-fast/builds/10962/steps/check-llvm%20asan/logs/stdio
>
> On Tue, Dec 5, 2017 at 9:20 AM Xinliang David Li via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: davidxl
>> Date: Tue Dec  5 09:19:41 2017
>> New Revision: 319794
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=319794&view=rev
>> Log:
>> [PGO] detect  infinite loop and form MST properly
>>
>> Differential Revision: http://reviews.llvm.org/D40702
>>
>> Added:
>>     llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll
>> Modified:
>>     llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h
>>     llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Transforms/Instrumentation/CFGMST.h?rev=319794&r1=319793&
>> r2=319794&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h (original)
>> +++ llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h Tue Dec  5
>> 09:19:41 2017
>> @@ -20,6 +20,7 @@
>>  #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"
>> @@ -136,6 +137,21 @@ 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.
>> @@ -212,11 +228,13 @@ public:
>>
>>    BranchProbabilityInfo *BPI;
>>    BlockFrequencyInfo *BFI;
>> +  LoopInfo *LI;
>>
>>  public:
>>    CFGMST(Function &Func, BranchProbabilityInfo *BPI_ = nullptr,
>> -         BlockFrequencyInfo *BFI_ = nullptr)
>> -      : F(Func), BPI(BPI_), BFI(BFI_) {
>> +         BlockFrequencyInfo *BFI_ = nullptr,
>> +         LoopInfo *LI_ = nullptr)
>> +      : F(Func), BPI(BPI_), BFI(BFI_), LI(LI_) {
>>      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=
>> 319794&r1=319793&r2=319794&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp Tue
>> Dec  5 09:19:41 2017
>> @@ -390,6 +390,7 @@ private:
>>
>>    void getAnalysisUsage(AnalysisUsage &AU) const override {
>>      AU.addRequired<BlockFrequencyInfoWrapperPass>();
>> +    AU.addRequired<LoopInfoWrapperPass>();
>>    }
>>  };
>>
>> @@ -415,6 +416,7 @@ private:
>>
>>    void getAnalysisUsage(AnalysisUsage &AU) const override {
>>      AU.addRequired<BlockFrequencyInfoWrapperPass>();
>> +    AU.addRequired<LoopInfoWrapperPass>();
>>    }
>>  };
>>
>> @@ -426,6 +428,7 @@ INITIALIZE_PASS_BEGIN(PGOInstrumentation
>>                        "PGO instrumentation.", false, false)
>>  INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
>>  INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)
>> +INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
>>  INITIALIZE_PASS_END(PGOInstrumentationGenLegacyPass, "pgo-instr-gen",
>>                      "PGO instrumentation.", false, false)
>>
>> @@ -439,6 +442,7 @@ INITIALIZE_PASS_BEGIN(PGOInstrumentation
>>                        "Read PGO instrumentation profile.", false, false)
>>  INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
>>  INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)
>> +INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
>>  INITIALIZE_PASS_END(PGOInstrumentationUseLegacyPass, "pgo-instr-use",
>>                      "Read PGO instrumentation profile.", false, false)
>>
>> @@ -530,9 +534,9 @@ public:
>>        Function &Func,
>>        std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
>>        bool CreateGlobalVar = false, BranchProbabilityInfo *BPI = nullptr,
>> -      BlockFrequencyInfo *BFI = nullptr)
>> +      BlockFrequencyInfo *BFI = nullptr, LoopInfo *LI = nullptr)
>>        : F(Func), ComdatMembers(ComdatMembers), ValueSites(IPVK_Last + 1),
>> -        SIVisitor(Func), MIVisitor(Func), MST(F, BPI, BFI) {
>> +        SIVisitor(Func), MIVisitor(Func), MST(F, BPI, BFI, LI) {
>>      // This should be done before CFG hash computation.
>>      SIVisitor.countSelects(Func);
>>      MIVisitor.countMemIntrinsics(Func);
>> @@ -715,9 +719,10 @@ BasicBlock *FuncPGOInstrumentation<Edge,
>>  // Critical edges will be split.
>>  static void instrumentOneFunc(
>>      Function &F, Module *M, BranchProbabilityInfo *BPI,
>> BlockFrequencyInfo *BFI,
>> +    LoopInfo *LI,
>>      std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
>>    FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, ComdatMembers,
>> true, BPI,
>> -                                                   BFI);
>> +                                                   BFI, LI);
>>    unsigned NumCounters = FuncInfo.getNumCounters();
>>
>>    uint32_t I = 0;
>> @@ -844,9 +849,10 @@ public:
>>    PGOUseFunc(Function &Func, Module *Modu,
>>               std::unordered_multimap<Comdat *, GlobalValue *>
>> &ComdatMembers,
>>               BranchProbabilityInfo *BPI = nullptr,
>> -             BlockFrequencyInfo *BFIin = nullptr)
>> +             BlockFrequencyInfo *BFIin = nullptr,
>> +             LoopInfo *LI = nullptr)
>>        : F(Func), M(Modu), BFI(BFIin),
>> -        FuncInfo(Func, ComdatMembers, false, BPI, BFIin),
>> +        FuncInfo(Func, ComdatMembers, false, BPI, BFIin, LI),
>>          FreqAttr(FFA_Normal) {}
>>
>>    // Read counts for the instrumented BB from profile.
>> @@ -1379,7 +1385,8 @@ static void collectComdatMembers(
>>
>>  static bool InstrumentAllFunctions(
>>      Module &M, function_ref<BranchProbabilityInfo *(Function &)>
>> LookupBPI,
>> -    function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
>> +    function_ref<BlockFrequencyInfo *(Function &)> LookupBFI,
>> +    function_ref<LoopInfo *(Function &)> LookupLI) {
>>    createIRLevelProfileFlagVariable(M);
>>    std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
>>    collectComdatMembers(M, ComdatMembers);
>> @@ -1389,7 +1396,8 @@ static bool InstrumentAllFunctions(
>>        continue;
>>      auto *BPI = LookupBPI(F);
>>      auto *BFI = LookupBFI(F);
>> -    instrumentOneFunc(F, &M, BPI, BFI, ComdatMembers);
>> +    auto *LI = LookupLI(F);
>> +    instrumentOneFunc(F, &M, BPI, BFI, LI, ComdatMembers);
>>    }
>>    return true;
>>  }
>> @@ -1404,7 +1412,10 @@ bool PGOInstrumentationGenLegacyPass::ru
>>    auto LookupBFI = [this](Function &F) {
>>      return &this->getAnalysis<BlockFrequencyInfoWrapperPass>
>> (F).getBFI();
>>    };
>> -  return InstrumentAllFunctions(M, LookupBPI, LookupBFI);
>> +  auto LookupLI = [this](Function &F) {
>> +    return &this->getAnalysis<LoopInfoWrapperPass>(F).getLoopInfo();
>> +  };
>> +  return InstrumentAllFunctions(M, LookupBPI, LookupBFI, LookupLI);
>>  }
>>
>>  PreservedAnalyses PGOInstrumentationGen::run(Module &M,
>> @@ -1418,7 +1429,11 @@ PreservedAnalyses PGOInstrumentationGen:
>>      return &FAM.getResult<BlockFrequencyAnalysis>(F);
>>    };
>>
>> -  if (!InstrumentAllFunctions(M, LookupBPI, LookupBFI))
>> +  auto LookupLI = [&FAM](Function &F) {
>> +    return &FAM.getResult<LoopAnalysis>(F);
>> +  };
>> +
>> +  if (!InstrumentAllFunctions(M, LookupBPI, LookupBFI, LookupLI))
>>      return PreservedAnalyses::all();
>>
>>    return PreservedAnalyses::none();
>> @@ -1427,7 +1442,8 @@ PreservedAnalyses PGOInstrumentationGen:
>>  static bool annotateAllFunctions(
>>      Module &M, StringRef ProfileFileName,
>>      function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
>> -    function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
>> +    function_ref<BlockFrequencyInfo *(Function &)> LookupBFI,
>> +    function_ref<LoopInfo *(Function &)> LookupLI) {
>>    DEBUG(dbgs() << "Read in profile counters: ");
>>    auto &Ctx = M.getContext();
>>    // Read the counter array from file.
>> @@ -1463,7 +1479,8 @@ static bool annotateAllFunctions(
>>        continue;
>>      auto *BPI = LookupBPI(F);
>>      auto *BFI = LookupBFI(F);
>> -    PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI);
>> +    auto *LI = LookupLI(F);
>> +    PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI, LI);
>>      if (!Func.readCounters(PGOReader.get()))
>>        continue;
>>      Func.populateCounters();
>> @@ -1539,7 +1556,11 @@ PreservedAnalyses PGOInstrumentationUse:
>>      return &FAM.getResult<BlockFrequencyAnalysis>(F);
>>    };
>>
>> -  if (!annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI))
>> +  auto LookupLI = [&FAM](Function &F) {
>> +    return &FAM.getResult<LoopAnalysis>(F);
>> +  };
>> +
>> +  if (!annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI,
>> LookupLI))
>>      return PreservedAnalyses::all();
>>
>>    return PreservedAnalyses::none();
>> @@ -1555,8 +1576,11 @@ bool PGOInstrumentationUseLegacyPass::ru
>>    auto LookupBFI = [this](Function &F) {
>>      return &this->getAnalysis<BlockFrequencyInfoWrapperPass>
>> (F).getBFI();
>>    };
>> +  auto LookupLI = [this](Function &F) {
>> +    return &this->getAnalysis<LoopInfoWrapperPass>(F).getLoopInfo();
>> +  };
>>
>> -  return annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI);
>> +  return annotateAllFunctions(M, ProfileFileName, LookupBPI, LookupBFI,
>> LookupLI);
>>  }
>>
>>  static std::string getSimpleNodeName(const BasicBlock *Node) {
>>
>> Added: 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=319794&view=auto
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll (added)
>> +++ llvm/trunk/test/Transforms/PGOProfile/infinite_loop_gen.ll Tue Dec
>> 5 09:19:41 2017
>> @@ -0,0 +1,18 @@
>> +; 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 }
>> +
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171205/86dce7a5/attachment-0001.html>


More information about the llvm-commits mailing list