[llvm] r346492 - Revert r346483: [CallSiteSplitting] Only record conditions up to the IDom(call site).

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 9 05:28:58 PST 2018


Author: fhahn
Date: Fri Nov  9 05:28:58 2018
New Revision: 346492

URL: http://llvm.org/viewvc/llvm-project?rev=346492&view=rev
Log:
Revert r346483: [CallSiteSplitting] Only record conditions up to the IDom(call site).

This cause a failure with EXPENSIVE_CHECKS

Modified:
    llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp
    llvm/trunk/test/Other/new-pm-lto-defaults.ll
    llvm/trunk/test/Other/opt-O3-pipeline.ll
    llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll

Modified: llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp?rev=346492&r1=346491&r2=346492&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp Fri Nov  9 05:28:58 2018
@@ -149,14 +149,14 @@ static void recordCondition(CallSite CS,
 
 /// Record ICmp conditions relevant to any argument in CS following Pred's
 /// single predecessors. If there are conflicting conditions along a path, like
-/// x == 1 and x == 0, the first condition will be used. We stop once we reach
-/// an edge to StopAt.
+/// x == 1 and x == 0, the first condition will be used.
 static void recordConditions(CallSite CS, BasicBlock *Pred,
-                             ConditionsTy &Conditions, BasicBlock *StopAt) {
+                             ConditionsTy &Conditions) {
+  recordCondition(CS, Pred, CS.getInstruction()->getParent(), Conditions);
   BasicBlock *From = Pred;
   BasicBlock *To = Pred;
   SmallPtrSet<BasicBlock *, 4> Visited;
-  while (To != StopAt && !Visited.count(From->getSinglePredecessor()) &&
+  while (!Visited.count(From->getSinglePredecessor()) &&
          (From = From->getSinglePredecessor())) {
     recordCondition(CS, From, To, Conditions);
     Visited.insert(From);
@@ -302,7 +302,7 @@ static void copyMustTailReturn(BasicBloc
 static void splitCallSite(
     CallSite CS,
     const SmallVectorImpl<std::pair<BasicBlock *, ConditionsTy>> &Preds,
-    DominatorTree &DT) {
+    DominatorTree *DT) {
   Instruction *Instr = CS.getInstruction();
   BasicBlock *TailBB = Instr->getParent();
   bool IsMustTailCall = CS.isMustTailCall();
@@ -327,7 +327,7 @@ static void splitCallSite(
     BasicBlock *PredBB = Preds[i].first;
     BasicBlock *SplitBlock = DuplicateInstructionsInSplitBetween(
         TailBB, PredBB, &*std::next(Instr->getIterator()), ValueToValueMaps[i],
-        &DT);
+        DT);
     assert(SplitBlock && "Unexpected new basic block split.");
 
     Instruction *NewCI =
@@ -438,7 +438,7 @@ static bool isPredicatedOnPHI(CallSite C
   return false;
 }
 
-static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree &DT) {
+static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree *DT) {
   if (!isPredicatedOnPHI(CS))
     return false;
 
@@ -449,25 +449,15 @@ static bool tryToSplitOnPHIPredicatedArg
   return true;
 }
 
-static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree &DT) {
+static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree *DT) {
   auto Preds = getTwoPredecessors(CS.getInstruction()->getParent());
   if (Preds[0] == Preds[1])
     return false;
 
-  // We can stop recording conditions once we reached the immediate dominator
-  // for the block containing the call site. Conditions in predecessors of the
-  // that node will be the same for all paths to the call site and splitting
-  // is not beneficial.
-  auto *CSDTNode = DT.getNode(CS.getInstruction()->getParent());
-  BasicBlock *StopAt = CSDTNode ? CSDTNode->getIDom()->getBlock() : nullptr;
-
   SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2> PredsCS;
   for (auto *Pred : make_range(Preds.rbegin(), Preds.rend())) {
     ConditionsTy Conditions;
-    // Record condition on edge BB(CS) <- Pred
-    recordCondition(CS, Pred, CS.getInstruction()->getParent(), Conditions);
-    // Record conditions followng Pred's single predecessors.
-    recordConditions(CS, Pred, Conditions, StopAt);
+    recordConditions(CS, Pred, Conditions);
     PredsCS.push_back({Pred, Conditions});
   }
 
@@ -476,24 +466,12 @@ static bool tryToSplitOnPredicatedArgume
       }))
     return false;
 
-  // Record common conditions starting from StopAt. Those conditions hold for
-  // all paths to CS. Adding them gives the inliner a better chance at inlining
-  // CS.
-  ConditionsTy CommonConditions;
-  if (StopAt)
-    recordConditions(CS, StopAt, CommonConditions, nullptr);
-  if (!CommonConditions.empty())
-    for (auto &Pred : PredsCS) {
-      Pred.second.insert(Pred.second.end(), CommonConditions.begin(),
-                         CommonConditions.end());
-    }
-
   splitCallSite(CS, PredsCS, DT);
   return true;
 }
 
 static bool tryToSplitCallSite(CallSite CS, TargetTransformInfo &TTI,
-                               DominatorTree &DT) {
+                               DominatorTree *DT) {
   if (!CS.arg_size() || !canSplitCallSite(CS, TTI))
     return false;
   return tryToSplitOnPredicatedArgument(CS, DT) ||
@@ -501,7 +479,7 @@ static bool tryToSplitCallSite(CallSite
 }
 
 static bool doCallSiteSplitting(Function &F, TargetLibraryInfo &TLI,
-                                TargetTransformInfo &TTI, DominatorTree &DT) {
+                                TargetTransformInfo &TTI, DominatorTree *DT) {
   bool Changed = false;
   for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE;) {
     BasicBlock &BB = *BI++;
@@ -546,7 +524,6 @@ struct CallSiteSplittingLegacyPass : pub
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<TargetLibraryInfoWrapperPass>();
     AU.addRequired<TargetTransformInfoWrapperPass>();
-    AU.addRequired<DominatorTreeWrapperPass>();
     AU.addPreserved<DominatorTreeWrapperPass>();
     FunctionPass::getAnalysisUsage(AU);
   }
@@ -557,8 +534,9 @@ struct CallSiteSplittingLegacyPass : pub
 
     auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
     auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
-    auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-    return doCallSiteSplitting(F, TLI, TTI, DT);
+    auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
+    return doCallSiteSplitting(F, TLI, TTI,
+                               DTWP ? &DTWP->getDomTree() : nullptr);
   }
 };
 } // namespace
@@ -568,7 +546,6 @@ INITIALIZE_PASS_BEGIN(CallSiteSplittingL
                       "Call-site splitting", false, false)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_END(CallSiteSplittingLegacyPass, "callsite-splitting",
                     "Call-site splitting", false, false)
 FunctionPass *llvm::createCallSiteSplittingPass() {
@@ -579,7 +556,7 @@ PreservedAnalyses CallSiteSplittingPass:
                                              FunctionAnalysisManager &AM) {
   auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
   auto &TTI = AM.getResult<TargetIRAnalysis>(F);
-  auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
+  auto *DT = AM.getCachedResult<DominatorTreeAnalysis>(F);
 
   if (!doCallSiteSplitting(F, TLI, TTI, DT))
     return PreservedAnalyses::all();

Modified: llvm/trunk/test/Other/new-pm-lto-defaults.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/new-pm-lto-defaults.ll?rev=346492&r1=346491&r2=346492&view=diff
==============================================================================
--- llvm/trunk/test/Other/new-pm-lto-defaults.ll (original)
+++ llvm/trunk/test/Other/new-pm-lto-defaults.ll Fri Nov  9 05:28:58 2018
@@ -38,13 +38,13 @@
 ; CHECK-O2-NEXT: Running pass: CallSiteSplittingPass on foo
 ; CHECK-O2-NEXT: Running analysis: TargetLibraryAnalysis on foo
 ; CHECK-O2-NEXT: Running analysis: TargetIRAnalysis on foo
-; CHECK-O2-NEXT: Running analysis: DominatorTreeAnalysis on foo
 ; CHECK-O2-NEXT: Finished llvm::Function pass manager run.
 ; CHECK-O2-NEXT: PGOIndirectCallPromotion
 ; CHECK-O2-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-O2-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
 ; CHECK-O2-NEXT: Running pass: IPSCCPPass
-; CHECK-O2-NEXT: Running analysis: AssumptionAnalysis on foo
+; CHECK-O2-DAG: Running analysis: AssumptionAnalysis on foo
+; CHECK-O2-DAG: Running analysis: DominatorTreeAnalysis on foo
 ; CHECK-O2-NEXT: Running pass: CalledValuePropagationPass
 ; CHECK-O-NEXT: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}PostOrderFunctionAttrsPass>
 ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}SCC

Modified: llvm/trunk/test/Other/opt-O3-pipeline.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-O3-pipeline.ll?rev=346492&r1=346491&r2=346492&view=diff
==============================================================================
--- llvm/trunk/test/Other/opt-O3-pipeline.ll (original)
+++ llvm/trunk/test/Other/opt-O3-pipeline.ll Fri Nov  9 05:28:58 2018
@@ -28,7 +28,6 @@
 ; CHECK-NEXT:     Force set function attributes
 ; CHECK-NEXT:     Infer set function attributes
 ; CHECK-NEXT:     FunctionPass Manager
-; CHECK-NEXT:       Dominator Tree Construction
 ; CHECK-NEXT:       Call-site splitting
 ; CHECK-NEXT:     Interprocedural Sparse Conditional Constant Propagation
 ; CHECK-NEXT:       Unnamed pass: implement Pass::getPassName()

Modified: llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll?rev=346492&r1=346491&r2=346492&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll (original)
+++ llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll Fri Nov  9 05:28:58 2018
@@ -123,14 +123,14 @@ End:
 ;CHECK-LABEL: Header2.split:
 ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 10)
 ;CHECK-LABEL: TBB.split:
-;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p)
+;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p)
 ;CHECK-LABEL: Tail
 ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ]
 ;CHECK: ret i32 %[[MERGED]]
 define i32 @test_ne_eq_ne(i32* %a, i32 %v, i32 %p) {
 Header:
   %tobool1 = icmp ne i32* %a, null
-  br i1 %tobool1, label %Header2, label %TBB
+  br i1 %tobool1, label %Header2, label %End
 
 Header2:
   %tobool2 = icmp eq i32 %p, 10
@@ -178,14 +178,14 @@ End:
 ;CHECK-LABEL: Header2.split:
 ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p)
 ;CHECK-LABEL: TBB.split:
-;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p)
+;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p)
 ;CHECK-LABEL: Tail
 ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ]
 ;CHECK: ret i32 %[[MERGED]]
 define i32 @test_ne_ne_ne_constrain_same_pointer_arg(i32* %a, i32 %v, i32 %p, i32* %a2, i32* %a3) {
 Header:
   %tobool1 = icmp ne i32* %a, null
-  br i1 %tobool1, label %Header2, label %TBB
+  br i1 %tobool1, label %Header2, label %End
 
 Header2:
   %tobool2 = icmp ne i32* %a, %a2
@@ -235,14 +235,14 @@ End:
 ;CHECK-LABEL: Header2.split:
 ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 10)
 ;CHECK-LABEL: TBB.split:
-;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 1, i32 %p)
+;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 1, i32 %p)
 ;CHECK-LABEL: Tail
 ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ]
 ;CHECK: ret i32 %[[MERGED]]
 define i32 @test_eq_eq_eq_untaken(i32* %a, i32 %v, i32 %p) {
 Header:
   %tobool1 = icmp eq i32* %a, null
-  br i1 %tobool1, label %TBB, label %Header2
+  br i1 %tobool1, label %End, label %Header2
 
 Header2:
   %tobool2 = icmp eq i32 %p, 10
@@ -290,14 +290,14 @@ End:
 ;CHECK-LABEL: Header2.split:
 ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* null, i32 %v, i32 10)
 ;CHECK-LABEL: TBB.split:
-;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p)
+;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* null, i32 %v, i32 %p)
 ;CHECK-LABEL: Tail
 ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ]
 ;CHECK: ret i32 %[[MERGED]]
 define i32 @test_ne_eq_ne_untaken(i32* %a, i32 %v, i32 %p) {
 Header:
   %tobool1 = icmp ne i32* %a, null
-  br i1 %tobool1, label %TBB, label %Header2
+  br i1 %tobool1, label %End, label %Header2
 
 Header2:
   %tobool2 = icmp eq i32 %p, 10
@@ -486,31 +486,6 @@ Tail:
   ret i32 %r
 
 End:
-  ret i32 %v
-}
-
-;CHECK-LABEL: @test_cond_no_effect
-;CHECK-NOT: Header.split:
-;CHECK-NOT: TBB.split:
-;CHECK-LABEL: Tail:
-;CHECK: %r = call i32 @callee(i32* %a, i32 %v, i32 0)
-;CHECK: ret i32 %r
-define i32 @test_cond_no_effect(i32* %a, i32 %v) {
-Entry:
-  %tobool1 = icmp eq i32* %a, null
-  br i1 %tobool1, label %Header, label %End
-
-Header:
-  br i1 undef, label %Tail, label %TBB
-
-TBB:
-  br i1 undef, label %Tail, label %End
-
-Tail:
-  %r = call i32 @callee(i32* %a, i32 %v, i32 0)
-  ret i32 %r
-
-End:
   ret i32 %v
 }
 




More information about the llvm-commits mailing list