|
| 1 | +From 19992a8c7f2df2000ea7fd4a284ec7b407400fb0 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Wei Mi <wmi@google.com> |
| 3 | +Date: Sun, 29 Mar 2020 17:14:12 -0400 |
| 4 | +Subject: [PATCH] [DAGCombine] Limit the number of times for the same store and |
| 5 | + root nodes to bail out in store merging dependence check. |
| 6 | + |
| 7 | +We run into a case where dependence check in store merging bail out many times |
| 8 | +for the same store and root nodes in a huge basicblock. That increases compile |
| 9 | +time by almost 100x. The patch add a map to track how many times the bailing |
| 10 | +out happen for the same store and root, and if it is over a limit, stop |
| 11 | +considering the store with the same root as a merging candidate. |
| 12 | + |
| 13 | +Differential Revision: https://reviews.llvm.org/D65174 |
| 14 | +--- |
| 15 | + llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 45 +++++++++++++++++-- |
| 16 | + 1 file changed, 42 insertions(+), 3 deletions(-) |
| 17 | + |
| 18 | +diff --git llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp |
| 19 | +index 6af01423ca1..9c7e37d6945 100644 |
| 20 | +--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp |
| 21 | ++++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp |
| 22 | +@@ -112,6 +112,11 @@ static cl::opt<bool> |
| 23 | + MaySplitLoadIndex("combiner-split-load-index", cl::Hidden, cl::init(true), |
| 24 | + cl::desc("DAG combiner may split indexing from loads")); |
| 25 | + |
| 26 | ++static cl::opt<unsigned> StoreMergeDependenceLimit( |
| 27 | ++ "combiner-store-merge-dependence-limit", cl::Hidden, cl::init(10), |
| 28 | ++ cl::desc("Limit the number of times for the same StoreNode and RootNode " |
| 29 | ++ "to bail out in store merging dependence check")); |
| 30 | ++ |
| 31 | + namespace { |
| 32 | + |
| 33 | + class DAGCombiner { |
| 34 | +@@ -145,6 +150,14 @@ namespace { |
| 35 | + /// which have not yet been combined to the worklist. |
| 36 | + SmallPtrSet<SDNode *, 32> CombinedNodes; |
| 37 | + |
| 38 | ++ /// Map from candidate StoreNode to the pair of RootNode and count. |
| 39 | ++ /// The count is used to track how many times we have seen the StoreNode |
| 40 | ++ /// with the same RootNode bail out in dependence check. If we have seen |
| 41 | ++ /// the bail out for the same pair many times over a limit, we won't |
| 42 | ++ /// consider the StoreNode with the same RootNode as store merging |
| 43 | ++ /// candidate again. |
| 44 | ++ DenseMap<SDNode *, std::pair<SDNode *, unsigned>> StoreRootCountMap; |
| 45 | ++ |
| 46 | + // AA - Used for DAG load/store alias analysis. |
| 47 | + AliasAnalysis *AA; |
| 48 | + |
| 49 | +@@ -190,6 +203,7 @@ namespace { |
| 50 | + /// Remove all instances of N from the worklist. |
| 51 | + void removeFromWorklist(SDNode *N) { |
| 52 | + CombinedNodes.erase(N); |
| 53 | ++ StoreRootCountMap.erase(N); |
| 54 | + |
| 55 | + auto It = WorklistMap.find(N); |
| 56 | + if (It == WorklistMap.end()) |
| 57 | +@@ -14423,6 +14437,18 @@ void DAGCombiner::getStoreMergeCandidates( |
| 58 | + return (BasePtr.equalBaseIndex(Ptr, DAG, Offset)); |
| 59 | + }; |
| 60 | + |
| 61 | ++ // Check if the pair of StoreNode and the RootNode already bail out many |
| 62 | ++ // times which is over the limit in dependence check. |
| 63 | ++ auto OverLimitInDependenceCheck = [&](SDNode *StoreNode, |
| 64 | ++ SDNode *RootNode) -> bool { |
| 65 | ++ auto RootCount = StoreRootCountMap.find(StoreNode); |
| 66 | ++ if (RootCount != StoreRootCountMap.end() && |
| 67 | ++ RootCount->second.first == RootNode && |
| 68 | ++ RootCount->second.second > StoreMergeDependenceLimit) |
| 69 | ++ return true; |
| 70 | ++ return false; |
| 71 | ++ }; |
| 72 | ++ |
| 73 | + // We looking for a root node which is an ancestor to all mergable |
| 74 | + // stores. We search up through a load, to our root and then down |
| 75 | + // through all children. For instance we will find Store{1,2,3} if |
| 76 | +@@ -14450,7 +14476,8 @@ void DAGCombiner::getStoreMergeCandidates( |
| 77 | + if (StoreSDNode *OtherST = dyn_cast<StoreSDNode>(*I2)) { |
| 78 | + BaseIndexOffset Ptr; |
| 79 | + int64_t PtrDiff; |
| 80 | +- if (CandidateMatch(OtherST, Ptr, PtrDiff)) |
| 81 | ++ if (CandidateMatch(OtherST, Ptr, PtrDiff) && |
| 82 | ++ !OverLimitInDependenceCheck(OtherST, RootNode)) |
| 83 | + StoreNodes.push_back(MemOpLink(OtherST, PtrDiff)); |
| 84 | + } |
| 85 | + } else |
| 86 | +@@ -14459,7 +14486,8 @@ void DAGCombiner::getStoreMergeCandidates( |
| 87 | + if (StoreSDNode *OtherST = dyn_cast<StoreSDNode>(*I)) { |
| 88 | + BaseIndexOffset Ptr; |
| 89 | + int64_t PtrDiff; |
| 90 | +- if (CandidateMatch(OtherST, Ptr, PtrDiff)) |
| 91 | ++ if (CandidateMatch(OtherST, Ptr, PtrDiff) && |
| 92 | ++ !OverLimitInDependenceCheck(OtherST, RootNode)) |
| 93 | + StoreNodes.push_back(MemOpLink(OtherST, PtrDiff)); |
| 94 | + } |
| 95 | + } |
| 96 | +@@ -14517,8 +14545,19 @@ bool DAGCombiner::checkMergeStoreCandidatesForDependencies( |
| 97 | + // Search through DAG. We can stop early if we find a store node. |
| 98 | + for (unsigned i = 0; i < NumStores; ++i) |
| 99 | + if (SDNode::hasPredecessorHelper(StoreNodes[i].MemNode, Visited, Worklist, |
| 100 | +- Max)) |
| 101 | ++ Max)) { |
| 102 | ++ // If the searching bail out, record the StoreNode and RootNode in the |
| 103 | ++ // StoreRootCountMap. If we have seen the pair many times over a limit, |
| 104 | ++ // we won't add the StoreNode into StoreNodes set again. |
| 105 | ++ if (Visited.size() >= Max) { |
| 106 | ++ auto &RootCount = StoreRootCountMap[StoreNodes[i].MemNode]; |
| 107 | ++ if (RootCount.first == RootNode) |
| 108 | ++ RootCount.second++; |
| 109 | ++ else |
| 110 | ++ RootCount = {RootNode, 1}; |
| 111 | ++ } |
| 112 | + return false; |
| 113 | ++ } |
| 114 | + return true; |
| 115 | + } |
| 116 | + |
| 117 | +-- |
| 118 | +2.25.2 |
| 119 | + |
0 commit comments