Pass AssumptionCacheTracker from SampleProfileLoader to Inliner

[danielcdh]

Summary

Inliner needs ACT when calling InlineFunction. Instead of nullptr, we need to pass it in from SampleProfileLoader

[davidxl]

lib/Transforms/IPO/SampleProfile.cpp

1282

add a FIXME comment for new pm here.

[danielcdh]

add FIXME

[danielcdh]

fix typo

[davidxl]

Is it possible to extract a test demonstrating the problem?

[danielcdh]

Add unittest

[danielcdh]

ping...

Thanks,
Dehao

[davidxl]

Is the testing just testing opt not crashing?

Please cleanup the test case: shorten the names, and minimizes the number of debug info needed.

test/Transforms/SampleProfile/inline-act.ll

4

empt --> empty

[danielcdh]

trim debug info

[vsk]

It looks like the key parts of this test case are: a load/store to a global, an inline candidate with a switch in it, and debug information.

Are there any elements there that are unnecessary, or that could be trimmed down further? I'm asking because I'm not very familiar with the SampleProf code, and am having a hard time working out exactly how inline-act.ll works. It would help to have SampleProfileLoader dump some debug output that can be FileChecked.

[danielcdh]

This test is just testing if SampleLoader will crash under certain conditions. You are right, the crash will only happen when 3 of these conditions satisfy. It's more like a robustness test, so FileCheck may not be an option here.

[eraman]

In D21205#458903, @danielcdh wrote:

This test is just testing if SampleLoader will crash under certain conditions. You are right, the crash will only happen when 3 of these conditions satisfy. It's more like a robustness test, so FileCheck may not be an option here.

IT looks like this use of IFI is at the InlineFunction method where a phi node is inserted when the callee has multiple exits and the result of the call is used. I think the test case can be simplified without needing to have any debug information. Alternatively, there are other places where IFI is used without any guards - for example when there is a pointer parameter with a specified alignment.

[davidxl]

lgtm

Easwaran, can you help investigate the inliner to see if there is anything general to be done there -- either tighten the interface if ACT is always required or something else.

lib/Transforms/IPO/SampleProfile.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/SampleProfile.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/PostDominators.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstIterator.h"

 /// \brief Sample profile pass.
 ///
 /// This pass reads profile data from the file specified by
 /// -sample-profile-file and annotates every affected function with the
 /// profile information found in that file.
 class SampleProfileLoader {
 public:
   SampleProfileLoader(StringRef Name = SampleProfileFile)
-      : DT(nullptr), PDT(nullptr), LI(nullptr), Reader(), Samples(nullptr),
-        Filename(Name), ProfileIsValid(false), TotalCollectedSamples(0) {}
+      : DT(nullptr), PDT(nullptr), LI(nullptr), ACT(nullptr), Reader(),
+        Samples(nullptr), Filename(Name), ProfileIsValid(false),
+        TotalCollectedSamples(0) {}
 
   bool doInitialization(Module &M);
   bool runOnModule(Module &M);
+  void setACT(AssumptionCacheTracker *A) { ACT = A; }
 
   void dump() { Reader->dump(); }
 
 protected:
   bool runOnFunction(Function &F);
   unsigned getFunctionLoc(Function &F);
   bool emitAnnotations(Function &F);
   ErrorOr<uint64_t> getInstWeight(const Instruction &I) const;
protected:
   /// the same number of times.
   EquivalenceClassMap EquivalenceClass;
 
   /// \brief Dominance, post-dominance and loop information.
   std::unique_ptr<DominatorTree> DT;
   std::unique_ptr<DominatorTreeBase<BasicBlock>> PDT;
   std::unique_ptr<LoopInfo> LI;
 
+  AssumptionCacheTracker *ACT;
+
   /// \brief Predecessors for each basic block in the CFG.
   BlockEdgeMap Predecessors;
 
   /// \brief Successors for each basic block in the CFG.
   BlockEdgeMap Successors;
 
   /// \brief Profile reader object.
   std::unique_ptr<SampleProfileReader> Reader;
public:
   void dump() { SampleLoader.dump(); }
 
   bool doInitialization(Module &M) override {
     return SampleLoader.doInitialization(M);
   }
   const char *getPassName() const override { return "Sample profile pass"; }
   bool runOnModule(Module &M) override;
 
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<AssumptionCacheTracker>();
+  }
 private:
   SampleProfileLoader SampleLoader;
 };
 
 class SampleCoverageTracker {
 public:
   SampleCoverageTracker() : SampleCoverage(), TotalUsedSamples(0) {}
 
while (true) {
     for (auto &BB : F) {
       for (auto &I : BB.getInstList()) {
         CallInst *CI = dyn_cast<CallInst>(&I);
         if (CI && callsiteIsHot(Samples, findCalleeFunctionSamples(*CI)))
           CIS.push_back(CI);
       }
     }
     for (auto CI : CIS) {
-      InlineFunctionInfo IFI;
+      InlineFunctionInfo IFI(nullptr, ACT);
       Function *CalledFunction = CI->getCalledFunction();
       DebugLoc DLoc = CI->getDebugLoc();
       uint64_t NumSamples = findCalleeFunctionSamples(*CI)->getTotalSamples();
       if (InlineFunction(CI, IFI)) {
         LocalChanged = true;
         emitOptimizationRemark(Ctx, DEBUG_TYPE, F, DLoc,
                                Twine("inlined hot callee '") +
                                    CalledFunction->getName() + "' with " +
if (Coverage < SampleProfileSampleCoverage) {
               Twine(Coverage) + "%) were applied",
           DS_Warning));
     }
   }
   return Changed;
 }
 
 char SampleProfileLoaderLegacyPass::ID = 0;
-INITIALIZE_PASS(SampleProfileLoaderLegacyPass, "sample-profile",
+INITIALIZE_PASS_BEGIN(SampleProfileLoaderLegacyPass, "sample-profile",
+                "Sample Profile loader", false, false)
+INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
+INITIALIZE_PASS_END(SampleProfileLoaderLegacyPass, "sample-profile",
                 "Sample Profile loader", false, false)
 
 bool SampleProfileLoader::doInitialization(Module &M) {
   auto &Ctx = M.getContext();
   auto ReaderOrErr = SampleProfileReader::create(Filename, Ctx);
   if (std::error_code EC = ReaderOrErr.getError()) {
     std::string Msg = "Could not open profile: " + EC.message();
     Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg));
for (auto &F : M)
     if (!F.isDeclaration()) {
       clearFunctionData();
       retval |= runOnFunction(F);
     }
   return retval;
 }
 
 bool SampleProfileLoaderLegacyPass::runOnModule(Module &M) {
+  // FIXME: pass in AssumptionCache correctly for the new pass manager. 
+  SampleLoader.setACT(&getAnalysis<AssumptionCacheTracker>());
   return SampleLoader.runOnModule(M);
 }
 
 bool SampleProfileLoader::runOnFunction(Function &F) {
   F.setEntryCount(0);
   Samples = Reader->getSamplesFor(F);
   if (!Samples->empty())
     return emitAnnotations(F);

test/Transforms/SampleProfile/Inputs/inline-act.prof

+_Z3bari:100:0
+ 1: _Z3fooi:100
+  2: 100

test/Transforms/SampleProfile/inline-act.ll

+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline-act.prof
+
+; Sample profile should have non-empty ACT passed to inliner
+
+; int t;
+; bool foo(int value) {
+;   switch(value) {
+;     case 0:
+;     case 1:
+;     case 3:
+;       return true;
+;     default:
+;       return false;
+;   }
+; }
+; void bar(int i) {
+;   if (foo(i))
+;     t *= 2;
+; }
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@t = global i32 0, align 4
+
+; Function Attrs: nounwind uwtable
+define zeroext i1 @_Z3fooi(i32) #0 {
+  %switch.tableidx = sub i32 %0, 0
+  %2 = icmp ult i32 %switch.tableidx, 4
+  br i1 %2, label %switch.lookup, label %3
+
+switch.lookup:                                    ; preds = %1
+  %switch.cast = trunc i32 %switch.tableidx to i4
+  %switch.shiftamt = mul i4 %switch.cast, 1
+  %switch.downshift = lshr i4 -5, %switch.shiftamt
+  %switch.masked = trunc i4 %switch.downshift to i1
+  ret i1 %switch.masked
+
+; <label>:3:                                      ; preds = %1
+  ret i1 false
+}
+
+; Function Attrs: nounwind uwtable
+define void @_Z3bari(i32) #0 !dbg !9 {
+  %2 = call zeroext i1 @_Z3fooi(i32 %0), !dbg !10
+  br i1 %2, label %3, label %6, !dbg !10
+
+; <label>:3:                                      ; preds = %1
+  %4 = load i32, i32* @t, align 4
+  %5 = shl nsw i32 %4, 1
+  store i32 %5, i32* @t, align 4
+  br label %6
+
+; <label>:6:                                      ; preds = %3, %1
+  ret void
+}
+
+attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+!llvm.ident = !{!4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.9.0 (trunk 272227) (llvm/trunk 272226)", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, enums: !2)
+!1 = !DIFile(filename: "test.cc", directory: "./")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{!"clang version 3.9.0 (trunk 272227) (llvm/trunk 272226)"}
+!6 = !DISubroutineType(types: !2)
+!9 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 14, type: !6, isLocal: false, isDefinition: true, scopeLine: 14, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!10 = !DILocation(line: 15, column: 7, scope: !9)
+!11 = !DILocation(line: 16, column: 7, scope: !9)