[CSSPGO] Unblock optimizations with pseudo probe instrumentation part 3.
authorHongtao Yu <hoy@fb.com>
Thu, 30 Sep 2021 16:13:41 +0000 (09:13 -0700)
committerHongtao Yu <hoy@fb.com>
Tue, 12 Oct 2021 16:44:12 +0000 (09:44 -0700)
This patch continues unblocking optimizations that are blocked by pseudo probe instrumentation.

Not exactly like DbgIntrinsics, PseudoProbe intrinsic has other attributes (such as mayread, maywrite, mayhaveSideEffect) that can block optimizations. The issues fixed are:
- Flipped default param of getFirstNonPHIOrDbg API to skip pseudo probes
- Unblocked CSE by avoiding pseudo probe from clobbering memory SSA
- Unblocked induction variable simpliciation
- Allow empty loop deletion by treating probe intrinsic isDroppable
- Some refactoring.

Reviewed By: wenlei

Differential Revision: https://reviews.llvm.org/D110847

17 files changed:
llvm/include/llvm/IR/BasicBlock.h
llvm/lib/Analysis/InlineCost.cpp
llvm/lib/Analysis/MemorySSA.cpp
llvm/lib/CodeGen/Analysis.cpp
llvm/lib/IR/User.cpp
llvm/lib/Transforms/IPO/GlobalDCE.cpp
llvm/lib/Transforms/IPO/GlobalOpt.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/lib/Transforms/Scalar/EarlyCSE.cpp
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
llvm/test/Transforms/SampleProfile/pseudo-probe-cse.ll [new file with mode: 0644]
llvm/test/Transforms/SampleProfile/pseudo-probe-loop-deletion.ll [new file with mode: 0644]

index 0af4ec4ef138f97ba9c80374f70affd2cff52542..184ddfc01c296eeb8e30cbaca38150df249ca419 100644 (file)
@@ -167,8 +167,8 @@ public:
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
   /// is true.
-  const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = false) const;
-  Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = false) {
+  const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
+  Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
     return const_cast<Instruction *>(
         static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg(
             SkipPseudoOp));
@@ -178,8 +178,8 @@ public:
   /// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
   /// operation if \c SkipPseudoOp is true.
   const Instruction *
-  getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = false) const;
-  Instruction *getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = false) {
+  getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
+  Instruction *getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
     return const_cast<Instruction *>(
         static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime(
             SkipPseudoOp));
@@ -200,14 +200,14 @@ public:
   /// SkipPseudoOp is true.
   iterator_range<filter_iterator<BasicBlock::const_iterator,
                                  std::function<bool(const Instruction &)>>>
-  instructionsWithoutDebug(bool SkipPseudoOp = false) const;
+  instructionsWithoutDebug(bool SkipPseudoOp = true) const;
 
   /// Return an iterator range over the instructions in the block, skipping any
   /// debug instructions. Skip and any pseudo operations as well if \c
   /// SkipPseudoOp is true.
   iterator_range<
       filter_iterator<BasicBlock::iterator, std::function<bool(Instruction &)>>>
-  instructionsWithoutDebug(bool SkipPseudoOp = false);
+  instructionsWithoutDebug(bool SkipPseudoOp = true);
 
   /// Return the size of the basic block ignoring debug instructions
   filter_iterator<BasicBlock::const_iterator,
index f0419d1e08b71b3d9ccaf81bdfeaf63a41020911..7ffe2aac689d0790327eea5d4c5e405e6ddb5926 100644 (file)
@@ -2393,11 +2393,8 @@ CallAnalyzer::analyzeBlock(BasicBlock *BB,
     // inlining due to debug symbols. Eventually, the number of unsimplified
     // instructions shouldn't factor into the cost computation, but until then,
     // hack around it here.
-    if (isa<DbgInfoIntrinsic>(I))
-      continue;
-
-    // Skip pseudo-probes.
-    if (isa<PseudoProbeInst>(I))
+    // Similarly, skip pseudo-probes.
+    if (I.isDebugOrPseudoInst())
       continue;
 
     // Skip ephemeral values.
index e005e2835ce50babbe81615f8e95a7a8a9e1c398..ac20e20f0c0d05932c67950d43f5b1fd8352aafd 100644 (file)
@@ -309,6 +309,7 @@ instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
     case Intrinsic::invariant_end:
     case Intrinsic::assume:
     case Intrinsic::experimental_noalias_scope_decl:
+    case Intrinsic::pseudoprobe:
       return {false, AliasResult(AliasResult::NoAlias)};
     case Intrinsic::dbg_addr:
     case Intrinsic::dbg_declare:
@@ -1782,6 +1783,7 @@ MemoryUseOrDef *MemorySSA::createNewAccess(Instruction *I,
       break;
     case Intrinsic::assume:
     case Intrinsic::experimental_noalias_scope_decl:
+    case Intrinsic::pseudoprobe:
       return nullptr;
     }
   }
index e5d576d879b536514f8bb373e8d3535fe7cee8cd..c21db82db7cc6fce3f0525ea074f851447c9514f 100644 (file)
@@ -524,10 +524,8 @@ bool llvm::isInTailCallPosition(const CallBase &Call, const TargetMachine &TM) {
     if (&*BBI == &Call)
       break;
     // Debug info intrinsics do not get in the way of tail call optimization.
-    if (isa<DbgInfoIntrinsic>(BBI))
-      continue;
     // Pseudo probe intrinsics do not block tail call optimization either.
-    if (isa<PseudoProbeInst>(BBI))
+    if (BBI->isDebugOrPseudoInst())
       continue;
     // A lifetime end, assume or noalias.decl intrinsic should not stop tail
     // call optimization.
index 8837151f2e183b484a4f96b006e281fc352988a4..68489075cd882ea81406b277d2770dfa0c069d25 100644 (file)
@@ -107,7 +107,7 @@ MutableArrayRef<uint8_t> User::getDescriptor() {
 }
 
 bool User::isDroppable() const {
-  return isa<AssumeInst>(this);
+  return isa<AssumeInst>(this) || isa<PseudoProbeInst>(this);
 }
 
 //===----------------------------------------------------------------------===//
index 9977fa7904a02859e270b69ad661092a1297d941..5e5d2086adc2e1a45db4f1d2e7ee8cb6485c69e5 100644 (file)
@@ -88,7 +88,7 @@ ModulePass *llvm::createGlobalDCEPass() {
 static bool isEmptyFunction(Function *F) {
   BasicBlock &Entry = F->getEntryBlock();
   for (auto &I : Entry) {
-    if (isa<DbgInfoIntrinsic>(I))
+    if (I.isDebugOrPseudoInst())
       continue;
     if (auto *RI = dyn_cast<ReturnInst>(&I))
       return !RI->getReturnValue();
index bd8f03296e809024e234707d23a5396b15a54d69..d40fc3a0a2922274e7db3e569960be932c5b875e 100644 (file)
@@ -2580,7 +2580,7 @@ static bool cxxDtorIsEmpty(const Function &Fn) {
     return false;
 
   for (auto &I : Fn.getEntryBlock()) {
-    if (isa<DbgInfoIntrinsic>(I))
+    if (I.isDebugOrPseudoInst())
       continue;
     if (isa<ReturnInst>(I))
       return true;
index e84137a1a8a652fa614e83120e0083e4158c7f34..9ff2cd9f03f1a06e78c5e63e36fbb014e9321a7e 100644 (file)
@@ -682,7 +682,7 @@ removeTriviallyEmptyRange(IntrinsicInst &EndI, InstCombinerImpl &IC,
   BasicBlock::reverse_iterator BI(EndI), BE(EndI.getParent()->rend());
   for (; BI != BE; ++BI) {
     if (auto *I = dyn_cast<IntrinsicInst>(&*BI)) {
-      if (isa<DbgInfoIntrinsic>(I) ||
+      if (I->isDebugOrPseudoInst() ||
           I->getIntrinsicID() == EndI.getIntrinsicID())
         continue;
       if (IsStart(*I)) {
index 213b3b86a9c034d60ce3015d1730165c28d6a989..79a8a065d02abba2bc00e8c7185749e5d77b5ac2 100644 (file)
@@ -1487,8 +1487,8 @@ bool InstCombinerImpl::mergeStoreIntoSuccessor(StoreInst &SI) {
   StoreInst *OtherStore = nullptr;
   if (OtherBr->isUnconditional()) {
     --BBI;
-    // Skip over debugging info.
-    while (isa<DbgInfoIntrinsic>(BBI) ||
+    // Skip over debugging info and pseudo probes.
+    while (BBI->isDebugOrPseudoInst() ||
            (isa<BitCastInst>(BBI) && BBI->getType()->isPointerTy())) {
       if (BBI==OtherBB->begin())
         return false;
index 83865e11973f8eeb40a360617a7d37dc228a02e1..d1d6be962edb785f01ce577980d2567db3f47512 100644 (file)
@@ -2925,7 +2925,7 @@ Instruction *InstCombinerImpl::visitUnconditionalBranchInst(BranchInst &BI) {
 
   auto GetLastSinkableStore = [](BasicBlock::iterator BBI) {
     auto IsNoopInstrForStoreMerging = [](BasicBlock::iterator BBI) {
-      return isa<DbgInfoIntrinsic>(BBI) ||
+      return BBI->isDebugOrPseudoInst() ||
              (isa<BitCastInst>(BBI) && BBI->getType()->isPointerTy());
     };
 
index 084ab8a1815c3ebed9d391d1c1bad0e8c2ad0c75..90f71f7729a7c8ae48895e098a2ae94eddb151cd 100644 (file)
@@ -1265,6 +1265,12 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
       continue;
     }
 
+    // Skip pseudoprobe intrinsics, for the same reason as assume intrinsics.
+    if (match(&Inst, m_Intrinsic<Intrinsic::pseudoprobe>())) {
+      LLVM_DEBUG(dbgs() << "EarlyCSE skipping pseudoprobe: " << Inst << '\n');
+      continue;
+    }
+
     // We can skip all invariant.start intrinsics since they only read memory,
     // and we can forward values across it. For invariant starts without
     // invariant ends, we can use the fact that the invariantness never ends to
index 0b5979a896c7be2b417a763f91138bf9a11d34ca..7fb024ea7fc770b7070d59d5b79101ce337a4b52 100644 (file)
@@ -1275,9 +1275,9 @@ bool IndVarSimplify::sinkUnusedInvariants(Loop *L) {
       // Skip debug info intrinsics.
       do {
         --I;
-      } while (isa<DbgInfoIntrinsic>(I) && I != Preheader->begin());
+      } while (I->isDebugOrPseudoInst() && I != Preheader->begin());
 
-      if (isa<DbgInfoIntrinsic>(I) && I == Preheader->begin())
+      if (I->isDebugOrPseudoInst() && I == Preheader->begin())
         Done = true;
     } else {
       Done = true;
index 7ea799a3f645313a86c9f263431e8f104983095a..048e691e33cf1b5e1ca2ec8b50ae20048ff601f4 100644 (file)
@@ -62,7 +62,7 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
     NewBB->getInstList().push_back(NewInst);
     VMap[&I] = NewInst; // Add instruction map to value.
 
-    hasCalls |= (isa<CallInst>(I) && !isa<DbgInfoIntrinsic>(I));
+    hasCalls |= (isa<CallInst>(I) && !I.isDebugOrPseudoInst());
     if (const AllocaInst *AI = dyn_cast<AllocaInst>(&I)) {
       if (!AI->isStaticAlloca()) {
         hasDynamicAllocas = true;
@@ -410,7 +410,7 @@ void PruningFunctionCloner::CloneBlock(
       NewInst->setName(II->getName() + NameSuffix);
     VMap[&*II] = NewInst; // Add instruction map to value.
     NewBB->getInstList().push_back(NewInst);
-    hasCalls |= (isa<CallInst>(II) && !isa<DbgInfoIntrinsic>(II));
+    hasCalls |= (isa<CallInst>(II) && !II->isDebugOrPseudoInst());
 
     if (CodeInfo) {
       CodeInfo->OrigVMap[&*II] = NewInst;
index 3add561c66d5efd99934b964cc9659efb7131e0d..920df4e9d742132e15f9eabc00c647b119d40778 100644 (file)
@@ -2587,7 +2587,7 @@ static bool BlockIsSimpleEnoughToThreadThrough(BasicBlock *BB) {
 
   // Walk the loop in reverse so that we can identify ephemeral values properly
   // (values only feeding assumes).
-  for (Instruction &I : reverse(BB->instructionsWithoutDebug())) {
+  for (Instruction &I : reverse(BB->instructionsWithoutDebug(false))) {
     // Can't fold blocks that contain noduplicate or convergent calls.
     if (CallInst *CI = dyn_cast<CallInst>(&I))
       if (CI->cannotDuplicate() || CI->isConvergent())
@@ -2891,8 +2891,7 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
   // instructions.
   for (BasicBlock *IfBlock : IfBlocks)
     for (BasicBlock::iterator I = IfBlock->begin(); !I->isTerminator(); ++I)
-      if (!AggressiveInsts.count(&*I) && !isa<DbgInfoIntrinsic>(I) &&
-          !isa<PseudoProbeInst>(I)) {
+      if (!AggressiveInsts.count(&*I) && !I->isDebugOrPseudoInst()) {
         // This is not an aggressive instruction that we can promote.
         // Because of this, we won't be able to get rid of the control flow, so
         // the xform is not worth it.
@@ -3416,7 +3415,7 @@ static bool mergeConditionalStoreToAddress(
     InstructionCost Cost = 0;
     InstructionCost Budget =
         PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic;
-    for (auto &I : BB->instructionsWithoutDebug()) {
+    for (auto &I : BB->instructionsWithoutDebug(false)) {
       // Consider terminator instruction to be free.
       if (I.isTerminator())
         continue;
@@ -3739,7 +3738,7 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
   // fold the conditions into logical ops and one cond br.
 
   // Ignore dbg intrinsics.
-  if (&*BB->instructionsWithoutDebug().begin() != BI)
+  if (&*BB->instructionsWithoutDebug(false).begin() != BI)
     return false;
 
   int PBIOp, BIOp;
@@ -5182,7 +5181,7 @@ GetCaseResults(SwitchInst *SI, ConstantInt *CaseVal, BasicBlock *CaseDest,
   // which we can constant-propagate the CaseVal, continue to its successor.
   SmallDenseMap<Value *, Constant *> ConstantPool;
   ConstantPool.insert(std::make_pair(SI->getCondition(), CaseVal));
-  for (Instruction &I :CaseDest->instructionsWithoutDebug()) {
+  for (Instruction &I : CaseDest->instructionsWithoutDebug(false)) {
     if (I.isTerminator()) {
       // If the terminator is a simple branch, continue to the next block.
       if (I.getNumSuccessors() != 1 || I.isExceptionalTerminator())
@@ -6197,7 +6196,7 @@ bool SimplifyCFGOpt::simplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) {
 
     // If the block only contains the switch, see if we can fold the block
     // away into any preds.
-    if (SI == &*BB->instructionsWithoutDebug().begin())
+    if (SI == &*BB->instructionsWithoutDebug(false).begin())
       if (FoldValueComparisonIntoPredecessors(SI, Builder))
         return requestResimplify();
   }
index cc634c56282adf864883cca463129434c9fc2d83..ad3a92a41671528f249b59a879d31314c3ad903c 100644 (file)
@@ -1085,7 +1085,7 @@ bool VectorCombine::run() {
       continue;
     // Use early increment range so that we can erase instructions in loop.
     for (Instruction &I : make_early_inc_range(BB)) {
-      if (isa<DbgInfoIntrinsic>(I))
+      if (I.isDebugOrPseudoInst())
         continue;
       FoldInst(I);
     }
diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-cse.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-cse.ll
new file mode 100644 (file)
index 0000000..7296a87
--- /dev/null
@@ -0,0 +1,28 @@
+; RUN: opt < %s -S -early-cse-memssa | FileCheck %s
+
+define i16 @f1() readonly {
+  ret i16 0
+}
+
+declare void @f2()
+
+; Check that EarlyCSE correctly handles pseudo probes that don't have
+; a MemoryAccess. 
+
+define void @f3() {
+; CHECK-LABEL: @f3(
+; CHECK-NEXT:    [[CALL1:%.*]] = call i16 @f1()
+; CHECK-NEXT:    call void @llvm.pseudoprobe
+; CHECK-NEXT:    ret void
+;
+  %call1 = call i16 @f1()
+  call void @llvm.pseudoprobe(i64 6878943695821059507, i64 9, i32 0, i64 -1)
+  %call2 = call i16 @f1()
+  ret void
+}
+
+
+; Function Attrs: inaccessiblememonly nounwind willreturn
+declare void @llvm.pseudoprobe(i64, i64, i32, i64) #0
+
+attributes #0 = { inaccessiblememonly nounwind willreturn }
\ No newline at end of file
diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-loop-deletion.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-loop-deletion.ll
new file mode 100644 (file)
index 0000000..cd4b1fd
--- /dev/null
@@ -0,0 +1,35 @@
+; RUN: opt %s -passes=loop-deletion -S | FileCheck %s --check-prefixes=CHECK
+
+%class.Loc.95 = type { %class.Domain.96 }
+%class.Domain.96 = type { %class.DomainBase.97 }
+%class.DomainBase.97 = type { [3 x %struct.WrapNoInit] }
+%struct.WrapNoInit = type { %class.Loc }
+%class.Loc = type { %class.Domain.67 }
+%class.Domain.67 = type { %class.DomainBase.68 }
+%class.DomainBase.68 = type { i32 }
+
+define dso_local void @foo(%class.Loc.95* %0) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    br label [[foo:%.*]]
+; CHECK:       foo.exit:
+; CHECK-NEXT:    ret void
+;
+  br label %2
+
+2:                                                ; preds = %4, %1
+  %.0.i.i = phi %class.Loc.95* [ undef, %1 ], [ %5, %4 ]
+  %3 = icmp ne %class.Loc.95* %.0.i.i, %0
+  br i1 %3, label %4, label %foo.exit
+
+4:                                                ; preds = %2
+  call void @llvm.pseudoprobe(i64 6878943695821059507, i64 9, i32 0, i64 -1)
+  %5 = getelementptr inbounds %class.Loc.95, %class.Loc.95* %.0.i.i, i32 1
+  br label %2
+
+foo.exit:            ; preds = %2
+  ret void
+}
+
+declare void @llvm.pseudoprobe(i64, i64, i32, i64)  #1
+
+attributes #1 = { willreturn readnone norecurse nofree }