PGO][InstrProf] Do not promote count if the exit blocks contains ret instruction

[xur]

Summary

Skip profile count promotion if any of the ExitBlocks contains a ret
instruction. This is to prevent dumping of incomplete profile -- if the
the loop is a long running loop and dump is called in the middle
of the loop, the result profile is incomplete.

ExitBlocks containing a ret instruction is an indication of a long running
loop -- early exit to error handling code.

[davidxl]

lgtm

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

292

Add a //FIXME here to add other heuristics to detect long running loop (runtime infinite), e.g. exit condition etc.

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

cl::desc("When the option is false, if the target block is in a loop, "
              "the promotion will be disallowed unless the promoted counter "
              " update can be further/iteratively promoted into an acyclic "
              " region."));
 
 cl::opt<bool> IterativeCounterPromotion(
     cl::ZeroOrMore, "iterative-counter-promotion", cl::init(true),
     cl::desc("Allow counter promotion across the whole loop nest."));
 
+cl::opt<bool> SkipRetExitBlock(
+    cl::ZeroOrMore, "skip-ret-exit-block", cl::init(true),
+    cl::desc("Suppress counter promotion if exit blocks contain ret."));
+
 class InstrProfilingLegacyPass : public ModulePass {
   InstrProfiling InstrProf;
 
 public:
   static char ID;
 
   InstrProfilingLegacyPass() : ModulePass(ID) {}
   InstrProfilingLegacyPass(const InstrProfOptions &Options, bool IsCS = false)
for (BasicBlock *ExitBlock : LoopExitBlocks) {
       }
     }
   }
 
   bool run(int64_t *NumPromoted) {
     // Skip 'infinite' loops:
     if (ExitBlocks.size() == 0)
       return false;
+
+    // Skip if any of the ExitBlocks contains a ret instruction.
+    // This is to prevent dumping of incomplete profile -- if the
+    // the loop is a long running loop and dump is called in the middle
+    // of the loop, the result profile is incomplete.
+    if (SkipRetExitBlock) {
+      for (auto BB : ExitBlocks)
+        if (dyn_cast<ReturnInst>(BB->getTerminator()) != nullptr)
+          return false;
+    }
+
     unsigned MaxProm = getMaxNumOfPromotionsInLoop(&L);
     if (MaxProm == 0)
       return false;
 
     unsigned Promoted = 0;
     for (auto &Cand : LoopToCandidates[&L]) {
 
       SmallVector<PHINode *, 4> NewPHIs;

llvm/test/Transforms/PGOProfile/counter_promo.ll

-; RUN: opt < %s -pgo-instr-gen -instrprof -do-counter-promotion=true -S | FileCheck --check-prefix=PROMO --check-prefix=NONATOMIC_PROMO %s
-; RUN: opt < %s --passes=pgo-instr-gen,instrprof -do-counter-promotion=true -S | FileCheck --check-prefix=PROMO --check-prefix=NONATOMIC_PROMO %s 
-; RUN: opt < %s -pgo-instr-gen -instrprof -do-counter-promotion=true -atomic-counter-update-promoted -S | FileCheck --check-prefix=PROMO --check-prefix=ATOMIC_PROMO %s
-; RUN: opt < %s --passes=pgo-instr-gen,instrprof -do-counter-promotion=true -atomic-counter-update-promoted -S | FileCheck --check-prefix=PROMO --check-prefix=ATOMIC_PROMO %s 
+; RUN: opt < %s -pgo-instr-gen -instrprof -do-counter-promotion=true -skip-ret-exit-block=0 -S | FileCheck --check-prefix=PROMO --check-prefix=NONATOMIC_PROMO %s
+; RUN: opt < %s --passes=pgo-instr-gen,instrprof -do-counter-promotion=true -skip-ret-exit-block=0 -S | FileCheck --check-prefix=PROMO --check-prefix=NONATOMIC_PROMO %s
+; RUN: opt < %s -pgo-instr-gen -instrprof -do-counter-promotion=true -atomic-counter-update-promoted -skip-ret-exit-block=0 -S | FileCheck --check-prefix=PROMO --check-prefix=ATOMIC_PROMO %s
+; RUN: opt < %s --passes=pgo-instr-gen,instrprof -do-counter-promotion=true -atomic-counter-update-promoted -skip-ret-exit-block=0 -S | FileCheck --check-prefix=PROMO --check-prefix=ATOMIC_PROMO %s
 
 define void @foo(i32 %n, i32 %N) {
 ; PROMO-LABEL: @foo
 ; PROMO: {{.*}} = load {{.*}} @__profc_foo{{.*}} 3)
-; PROMO-NEXT: add 
+; PROMO-NEXT: add
 ; PROMO-NEXT: store {{.*}}@__profc_foo{{.*}}3)
 bb:
   %tmp = add nsw i32 %n, 1
   %tmp1 = add nsw i32 %n, -1
   br label %bb2
 
 bb2:                                              ; preds = %bb9, %bb
 ; PROMO: phi {{.*}}

llvm/test/Transforms/PGOProfile/not_promote_ret_exit.ll

+; RUN: opt < %s -instrprof -S -o - -do-counter-promotion=1  -skip-ret-exit-block=1 | FileCheck %s --check-prefixes=CHECK,SKIP
+; RUN: opt < %s -instrprof -S -o - -do-counter-promotion=1  -skip-ret-exit-block=0 | FileCheck %s --check-prefixes=CHECK,NOTSKIP
+
+$__llvm_profile_raw_version = comdat any
+
+@bar = dso_local local_unnamed_addr global i32 0, align 4
+@__llvm_profile_raw_version = constant i64 72057594037927941, comdat
+@__profn_foo = private constant [3 x i8] c"foo"
+
+define dso_local void @foo(i32 %n)  {
+entry:
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 29212902728, i32 2, i32 1)
+  br label %for.cond
+
+for.cond:
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %if.end ]
+  %cmp = icmp slt i32 %i.0, %n
+  %0 = load i32, i32* @bar, align 4
+  %tobool.not = icmp eq i32 %0, 0
+  %or.cond = and i1 %cmp, %tobool.not
+  br i1 %or.cond, label %if.end, label %cleanup
+
+if.end:
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 29212902728, i32 2, i32 0)
+  call void (...) @bar2()
+  %inc = add nuw nsw i32 %i.0, 1
+  br label %for.cond
+
+cleanup:
+; CHECK: cleanup:
+; SKIP-NOT:  %pgocount.promoted
+; NOTSKIP:  %pgocount.promoted
+  ret void
+}
+
+declare dso_local void @bar2(...)
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)