[LegacyPM] Port CGProfile pass

[evgeny777]

Summary

This allows generating CG profile using legacy pass manager

[tejohnson]

FYI what I heard at the dev meeting yesterday is that we are very close to being able to switch to the new PM by default, after which the legacy PM will be removed (I'm sure not immediately, but the goal is to get down to one PM soon). Before you do a lot of work backporting some of the new passes like this, is switching to the new PM an option for you?

[evgeny777]

Thanks for looking at it, Teresa!

Here is a llvm-dev post, suggesting to use new PM by default:
http://lists.llvm.org/pipermail/llvm-dev/2017-October/118280.html

As you may have noticed it was two years ago and new PM is still "brand new" :)
Also, AFAIK, codegen passes still use old PM, so I guess it will be around for quite a while.
Until new PM is adopted as default by LLVM community, switching to it is not an option for us, unfortunately.
And I need just this pass (it does pretty good job optimizing PGO instrumented images) and don't have plans to port anything else.

[tejohnson]

In D69331#1718885, @evgeny777 wrote:

Thanks for looking at it, Teresa!

Here is a llvm-dev post, suggesting to use new PM by default:
http://lists.llvm.org/pipermail/llvm-dev/2017-October/118280.html

As you may have noticed it was two years ago and new PM is still "brand new" :)
Also, AFAIK, codegen passes still use old PM, so I guess it will be around for quite a while.
Until new PM is adopted as default by LLVM community, switching to it is not an option for us, unfortunately.
And I need just this pass (it does pretty good job optimizing PGO instrumented images) and don't have plans to port anything else.

Ok, still suggest trying to move to new PM proactively to make sure it works for you, but it's fine with me to back port this. A few comments below.

include/llvm/LinkAllPasses.h

87 ↗

(On Diff #226107)

Formatting off

lib/Transforms/IPO/PassManagerBuilder.cpp

781 ↗

(On Diff #226107)

I notice that in the new PM this is added after the OptimizerLast EPs are added. Not sure if that is intentional - should the new and old PMs be consistent here?

lib/Transforms/Instrumentation/CGProfile.cpp

30 ↗

(On Diff #226107)

Nit: Suggest moving this declaration to before Symtab which puts it in the same relative order as before and should avoid a spurious diff here.

test/Other/opt-O2-pipeline.ll

290 ↗

(On Diff #226107)

Will this unnamed pass issue be fixed by D69315?

[Bigcheese]

This was originally not added to the legacy pass manager because it leaks memory under it due to fundamental issues with the old pass manager. See https://reviews.llvm.org/D48105#1140299 .

[evgeny777]

This was originally not added to the legacy pass manager because it leaks memory under it due to fundamental issues with the old pass manager. See https://reviews.llvm.org/D48105#1140299 .

@Bigcheese @chandlerc Can you please elaborate? I've run llvm, clang and lld test suites with lsan and everything seems good. What's the problem exactly?

[evgeny777]

test/Other/opt-O2-pipeline.ll

290 ↗

(On Diff #226107)

Yes

[evgeny777]

Addressed comments.

I've run test cases of llvm/clang built with ASAN and LSAN without problems.
If problem details can't be recalled at the moment, I suggest landing this patch and reverting it if problem arises

[evgeny777]

Rebased

[zequanwu]

Hi, I just noticed now that here was a patch porting CGProfile Pass to legacy PM. I created a similar one and landed it https://reviews.llvm.org/D83013.

llvm/include/llvm/InitializePasses.h

 void initializeVirtRegRewriterPass(PassRegistry&);
 void initializeWarnMissedTransformationsLegacyPass(PassRegistry &);
 void initializeWasmEHPreparePass(PassRegistry&);
 void initializeWholeProgramDevirtPass(PassRegistry&);
 void initializeWinEHPreparePass(PassRegistry&);
 void initializeWriteBitcodePassPass(PassRegistry&);
 void initializeWriteThinLTOBitcodePass(PassRegistry&);
 void initializeXRayInstrumentationPass(PassRegistry&);
-
+void initializeCGProfileLegacyPassPass(PassRegistry &);
 } // end namespace llvm
 
 #endif // LLVM_INITIALIZEPASSES_H

llvm/include/llvm/LinkAllPasses.h

ForcePassLinking() {
       (void) llvm::createScopedNoAliasAAWrapperPass();
       (void) llvm::createBoundsCheckingLegacyPass();
       (void) llvm::createBreakCriticalEdgesPass();
       (void) llvm::createCallGraphDOTPrinterPass();
       (void) llvm::createCallGraphViewerPass();
       (void) llvm::createCFGSimplificationPass();
       (void) llvm::createCFLAndersAAWrapperPass();
       (void) llvm::createCFLSteensAAWrapperPass();
+      (void)llvm::createCGProfilePass();
       (void) llvm::createStructurizeCFGPass();
       (void) llvm::createLibCallsShrinkWrapPass();
       (void) llvm::createCalledValuePropagationPass();
       (void) llvm::createConstantMergePass();
       (void) llvm::createConstantPropagationPass();
       (void) llvm::createControlHeightReductionLegacyPass();
       (void) llvm::createCostModelAnalysisPass();
       (void) llvm::createDeadArgEliminationPass();

llvm/include/llvm/Transforms/IPO.h

 /// createGVExtractionPass - If deleteFn is true, this pass deletes
 /// the specified global values. Otherwise, it deletes as much of the module as
 /// possible, except for the global values specified.
 ///
 ModulePass *createGVExtractionPass(std::vector<GlobalValue*>& GVs, bool
                                    deleteFn = false);
 
 //===----------------------------------------------------------------------===//
+/// createCGProfilePass - This function returns a new pass that generates
+/// call graph profile metadata.
+///
+ModulePass *createCGProfilePass();
+
+//===----------------------------------------------------------------------===//
 /// This pass performs iterative function importing from other modules.
 Pass *createFunctionImportPass();
 
 //===----------------------------------------------------------------------===//
 /// createFunctionInliningPass - Return a new pass object that uses a heuristic
 /// to inline direct function calls to small functions.
 ///
 /// The Threshold can be passed directly, or asked to be computed from the

llvm/include/llvm/Transforms/Instrumentation/CGProfile.h

 
 #include "llvm/ADT/MapVector.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 class CGProfilePass : public PassInfoMixin<CGProfilePass> {
 public:
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
-
-private:
-  void addModuleFlags(
-      Module &M,
-      MapVector<std::pair<Function *, Function *>, uint64_t> &Counts) const;
 };
+
+ModulePass *createCGProfilePass();
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_CGPROFILE_H

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp

void PassManagerBuilder::populateModulePassManager(
   // passes to avoid re-sinking, but before SimplifyCFG because it can allow
   // flattening of blocks.
   MPM.add(createDivRemPairsPass());
 
   // LoopSink (and other loop passes since the last simplifyCFG) might have
   // resulted in single-entry-single-exit or empty blocks. Clean up the CFG.
   MPM.add(createCFGSimplificationPass());
 
+  // Create CG profile metadata if appropriate
+  MPM.add(createCGProfilePass());
+
   addExtensionsToPM(EP_OptimizerLast, MPM);
 
   if (PrepareForLTO) {
     MPM.add(createCanonicalizeAliasesPass());
     // Rename anon globals to be able to handle them in the summary
     MPM.add(createNameAnonGlobalPass());
   }
 }

llvm/lib/Transforms/Instrumentation/CGProfile.cpp

 #include "llvm/IR/PassManager.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Transforms/Instrumentation.h"
 
 #include <array>
 
 using namespace llvm;
 
-PreservedAnalyses CGProfilePass::run(Module &M, ModuleAnalysisManager &MAM) {
-  MapVector<std::pair<Function *, Function *>, uint64_t> Counts;
-  FunctionAnalysisManager &FAM =
-      MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+static void generateCGProfileMetadata(
+    Module &M, function_ref<TargetTransformInfo &(Function &)> GetTTI,
+    function_ref<BlockFrequencyInfo &(Function &)> GetBFI) {
   InstrProfSymtab Symtab;
+  MapVector<std::pair<Function *, Function *>, uint64_t> Counts;
   auto UpdateCounts = [&](TargetTransformInfo &TTI, Function *F,
                           Function *CalledF, uint64_t NewCount) {
     if (!CalledF || !TTI.isLoweredToCall(CalledF))
       return;
     uint64_t &Count = Counts[std::make_pair(F, CalledF)];
     Count = SaturatingAdd(Count, NewCount);
   };
   // Ignore error here.  Indirect calls are ignored if this fails.
   (void)(bool)Symtab.create(M);
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
-    auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
+    auto &BFI = GetBFI(F);
     if (BFI.getEntryFreq() == 0)
       continue;
-    TargetTransformInfo &TTI = FAM.getResult<TargetIRAnalysis>(F);
+    TargetTransformInfo &TTI = GetTTI(F);
     for (auto &BB : F) {
       Optional<uint64_t> BBCount = BFI.getBlockProfileCount(&BB);
       if (!BBCount)
         continue;
       for (auto &I : BB) {
         CallSite CS(&I);
         if (!CS)
           continue;
for (auto &BB : F) {
           }
           continue;
         }
         UpdateCounts(TTI, &F, CS.getCalledFunction(), *BBCount);
       }
     }
   }
 
-  addModuleFlags(M, Counts);
-
-  return PreservedAnalyses::all();
-}
-
-void CGProfilePass::addModuleFlags(
-    Module &M,
-    MapVector<std::pair<Function *, Function *>, uint64_t> &Counts) const {
   if (Counts.empty())
     return;
 
   LLVMContext &Context = M.getContext();
   MDBuilder MDB(Context);
   std::vector<Metadata *> Nodes;
 
   for (auto E : Counts) {
     Metadata *Vals[] = {ValueAsMetadata::get(E.first.first),
                         ValueAsMetadata::get(E.first.second),
                         MDB.createConstant(ConstantInt::get(
                             Type::getInt64Ty(Context), E.second))};
     Nodes.push_back(MDNode::get(Context, Vals));
   }
 
   M.addModuleFlag(Module::Append, "CG Profile", MDNode::get(Context, Nodes));
 }
+
+PreservedAnalyses CGProfilePass::run(Module &M, ModuleAnalysisManager &MAM) {
+  FunctionAnalysisManager &FAM =
+      MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+
+  auto GetTTI = [&FAM](Function &F) -> TargetTransformInfo & {
+    return FAM.getResult<TargetIRAnalysis>(F);
+  };
+
+  auto GetBFI = [&FAM](Function &F) -> BlockFrequencyInfo & {
+    return FAM.getResult<BlockFrequencyAnalysis>(F);
+  };
+  generateCGProfileMetadata(M, GetTTI, GetBFI);
+  return PreservedAnalyses::all();
+}
+
+namespace {
+
+struct CGProfileLegacyPass : public ModulePass {
+  static char ID; // Pass identification, replacement for typeid
+
+  CGProfileLegacyPass() : ModulePass(ID) {
+    initializeCGProfileLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
+
+  bool runOnModule(Module &M) override {
+    if (skipModule(M))
+      return false;
+
+    auto GetTTI = [this](Function &F) -> TargetTransformInfo & {
+      return this->getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+    };
+
+    auto GetBFI = [this](Function &F) -> BlockFrequencyInfo & {
+      return this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
+    };
+
+    generateCGProfileMetadata(M, GetTTI, GetBFI);
+    return false;
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<TargetTransformInfoWrapperPass>();
+    AU.addRequired<BlockFrequencyInfoWrapperPass>();
+  }
+};
+
+} // end anonymous namespace
+
+char CGProfileLegacyPass::ID = 0;
+
+INITIALIZE_PASS_BEGIN(CGProfileLegacyPass, "cgprofile",
+                      "CG profile metadata generator", false, false)
+INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
+INITIALIZE_PASS_END(CGProfileLegacyPass, "cgprofile",
+                    "CG profile metadata generator", false, false)
+
+ModulePass *llvm::createCGProfilePass() { return new CGProfileLegacyPass(); }

llvm/test/Other/opt-O2-pipeline.ll

 ; CHECK-NEXT:       Loop Pass Manager
 ; CHECK-NEXT:         Loop Sink
 ; CHECK-NEXT:       Lazy Branch Probability Analysis
 ; CHECK-NEXT:       Lazy Block Frequency Analysis
 ; CHECK-NEXT:       Optimization Remark Emitter
 ; CHECK-NEXT:       Remove redundant instructions
 ; CHECK-NEXT:       Hoist/decompose integer division and remainder
 ; CHECK-NEXT:       Simplify the CFG
+; CHECK-NEXT:     CG profile metadata generator
+; CHECK-NEXT:      FunctionPass Manager
+; CHECK-NEXT:        Dominator Tree Construction
+; CHECK-NEXT:        Natural Loop Information
+; CHECK-NEXT:        Branch Probability Analysis
+; CHECK-NEXT:        Block Frequency Analysis
+; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Module Verifier
 ; CHECK-NEXT:     Bitcode Writer
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT:  FunctionPass Manager
 ; CHECK-NEXT:     Dominator Tree Construction
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT: Target Library Information
 ; CHECK-NEXT:   FunctionPass Manager

llvm/test/Other/opt-O3-pipeline.ll

 ; CHECK-NEXT:       Loop Pass Manager
 ; CHECK-NEXT:         Loop Sink
 ; CHECK-NEXT:       Lazy Branch Probability Analysis
 ; CHECK-NEXT:       Lazy Block Frequency Analysis
 ; CHECK-NEXT:       Optimization Remark Emitter
 ; CHECK-NEXT:       Remove redundant instructions
 ; CHECK-NEXT:       Hoist/decompose integer division and remainder
 ; CHECK-NEXT:       Simplify the CFG
+; CHECK-NEXT:     CG profile metadata generator
+; CHECK-NEXT:      FunctionPass Manager
+; CHECK-NEXT:        Dominator Tree Construction
+; CHECK-NEXT:        Natural Loop Information
+; CHECK-NEXT:        Branch Probability Analysis
+; CHECK-NEXT:        Block Frequency Analysis
+; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Module Verifier
 ; CHECK-NEXT:     Bitcode Writer
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT:  FunctionPass Manager
 ; CHECK-NEXT:     Dominator Tree Construction
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT: Target Library Information
 ; CHECK-NEXT:   FunctionPass Manager

llvm/test/Other/opt-Os-pipeline.ll

 ; CHECK-NEXT:       Loop Pass Manager
 ; CHECK-NEXT:         Loop Sink
 ; CHECK-NEXT:       Lazy Branch Probability Analysis
 ; CHECK-NEXT:       Lazy Block Frequency Analysis
 ; CHECK-NEXT:       Optimization Remark Emitter
 ; CHECK-NEXT:       Remove redundant instructions
 ; CHECK-NEXT:       Hoist/decompose integer division and remainder
 ; CHECK-NEXT:       Simplify the CFG
+; CHECK-NEXT:     CG profile metadata generator
+; CHECK-NEXT:      FunctionPass Manager
+; CHECK-NEXT:        Dominator Tree Construction
+; CHECK-NEXT:        Natural Loop Information
+; CHECK-NEXT:        Branch Probability Analysis
+; CHECK-NEXT:        Block Frequency Analysis
+; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Module Verifier
 ; CHECK-NEXT:     Bitcode Writer
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT:  FunctionPass Manager
 ; CHECK-NEXT:     Dominator Tree Construction
 ; CHECK-NEXT: Pass Arguments:
 ; CHECK-NEXT: Target Library Information
 ; CHECK-NEXT:   FunctionPass Manager

llvm/test/Other/pass-pipelines.ll

 ; the runtime unrolling though.
 ; CHECK-O2: Loop Pass Manager
 ; CHECK-O2-NEXT: Loop Invariant Code Motion
 ; SPLIT: Hot Cold Splitting
 ; CHECK-O2: FunctionPass Manager
 ; CHECK-O2: Loop Pass Manager
 ; CHECK-O2-NEXT: Loop Sink
 ; CHECK-O2: Simplify the CFG
-; CHECK-O2-NOT: Manager
 ;
 ; FIXME: There really shouldn't be another pass manager, especially one that
 ; just builds the domtree. It doesn't even run the verifier.
 ; CHECK-O2: Pass Arguments:
 ; CHECK-O2: FunctionPass Manager
 ; CHECK-O2-NEXT: Dominator Tree Construction
 
 define void @foo() {
   ret void
 }