Skip to content

Commit

Permalink
Merge pull request #35262 from JuliaLang/jb/storemergeperf
Browse files Browse the repository at this point in the history
add LLVM patch D65174 to fix a compiler performance regression
  • Loading branch information
vchuravy authored Mar 28, 2020
2 parents 712731e + 7807224 commit b637cb7
Show file tree
Hide file tree
Showing 108 changed files with 182 additions and 56 deletions.
2 changes: 1 addition & 1 deletion deps/Versions.make
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
LLVM_VER = 9.0.1
LLVM_BB_REL = 4
LLVM_BB_REL = 1
PCRE_VER = 10.31
PCRE_BB_REL = 0
DSFMT_VER = 2.2.3
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4c9a2a5ea1937a7b5412884f1f794b6a
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
66ca30822c287bfb6f3632f434fcda07f1c4e38f5184f48c6ca18abcaa6ed9b4aee60b89a9a6605410c44080a53a40a46a9a3eac715822b4a3b99357164d1ccc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fe89e5378dfe76a43f7a88931f80a828
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
01d7b136791d069f93ac0e568526f20ad1e2fe2844924c34c76779776a233195910df7b8c9eff3d64a6bc659cff3f142787d15fe7da60d72b8f82aa123aab947
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a5aa690a5a064b4a363587f2bbe80d8b
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
af0f212396fef61dbd8f5214ec9e4f89abfc9128abee3c596ff9d3337c6eee0b8b39e03ba10e063f7e51a21e97cb9d25c84e08daae48c0a233cbf3e706d1b538
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4512e0bd5a04455a2a8ad64bf85b632f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a02339b62f2b0e3e19eed152d91d7ad8a96903f58e4cd8a9d6f7668fb747276788ba1513b5ab09031b711c0667c0a18ec3be88ef751f78d82ce9d35d627805a2
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
35341a6f648e070202efff8e317377a1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
5a3333448571452b484c4bb5dcf67d2df19ff29f1336d633f800ae8875c0263ed0666acf989007e228021dbc99b7f72e7387be23de7bb2c5f62cbbcbc1b94138
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
b3d25df5735d82ececdfa14647aa7d47
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
83f1b25b39917c821325ae32f274a4e5dafcc3a8327b36c6a9fde2f42d51497edd44acc173e71cca5a4e7e0cbeff3333c6aa12f862074f4958aaab7fa3c212e3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0a4f795cde203f7d43688c0f883cb5c4
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
876a4e958df5177e06e7265f3fed55976b2537646c9f368c1f944c2433d0180f42bc44bcdb70357233a3560d4c0877e6cc5d8576edd859ea86ab9ee92f07aa84
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
77cc0b3a28c76ac81598e9a7e6b0db6f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
590582c8190aff3b6d2a260923dea93f0c8e5e6b3ad254d4774bb8556bee20ddedba2a57b10af24fd839b1894f22c2aaa82a4719acd3376a306646abec0078e8
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
23741cbddba0a4cfb00b608447a06199
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
94765137fcf14a62b74d12785103d746a47e10114f3198ca64ff48d04516070b28882749284fa85b93eb65d4c786a59fdc5ff194da43f73e01afae2310f60d09
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
5c83d79d516f49e028c64618d91f40fa
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
46088d0baa06e85bd269e5050c0f27ec1c23c81a597d2cbe7c91a8ff24229deec150324ef39fdb664f4183bef422b249b240995be4fc92b2679e09bbaf4cdc42
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
14a59dfe908fa6d5f54038f9693e1666
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
daddcf3ccfeeb7f9c01d806bb7fc6347917e85e4dfbc0a1a2a4471c48518d66af6f892f8a9aec980414d09b28d17d45a14c13561a5a6b1fbde8bd1525fc07efb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
899312263148ef5b9e4ed3c8d0c94c9c
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f70cf145f12a513a86caf41c9495fcc4a015247bb1c68ebdbafccfaadded1ac965b0e9f3a266b152a8fb0dc25f2ddb5180919b0e857c01077f04b112a664dd56
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
56fa1fc935870aa4911ce733d5124f9c
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
7b9c4699bb1ea9d0f0c306423fac4d95649038985a074c1647bccae1439e21821a40cffc0d2d2717a95c52ac209aab6028b92e6f37e0d0fc4e6aae580ba5bf0f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0b3a87b622b0988322194e3ee58ff868
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ff0f0d939433f56cf594740219de1293657b8ea09041962013eab2caf05e8ec8d4506921f7b1448469f7a98bd524e5e428227abf2ddbb40546746dcd504c0455
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
08e034ad8fdae1651c8e435d11d85a25
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
205c32db264a64a8393e2befb6601832f2fc3661667ddc5a80e241eb4e469cb8170d3ee0172eabe624562d13463d3ae9a38110d1d54cef35f4c511c9cc5d1a47
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c2c8607a5be4ba87715991bd89dd6f3f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
14c297e7531cf5bbb9ca6cd6791160b3ebd940a3fbf791c6ff32a492e0da4209c18735c84fc1f550aceb672553768abb2466b4c7e0932aaaad5f910ce3b2d459
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
6dd4597ecdc66af540743352c1eadfcd
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
23041943dd2202d47966211277b4323aa5e574c68496ad378085253cee52abe92fc66422c915c63cfa174a7f89953df3274822b631462ba6cc8b3597fced59ec
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2cb3a97b3d4c439a876c44862ecda60b
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8107a713544be08b86dbaebd26b51054c58a27434c15ef60c14c39fc22adee803b610cd6e74c400a913ad5a4b3e2e6d6cc41f7c735111c291ca7ca9ed597f625
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
b20eb7f7655404618335ba28a60d054d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
b67014a59fd047c10d852bfcbbe690cd69703c01bd2284486a38f93685a5862a699c244c9f19909170bbe48445b6f76a3781025dc3a06e61b36abf0b24e6ff10
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
880d47de9c0f2808a8c35172ab8bd280
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0d8fce015f7955d28823e502026cff315e99b285d0d8f9e1c9ff5cf6a3fc1d38e4a02cfda4624a42a2b9006b36e1016ca2fec9d3d282866cdb6154dbef0870e7
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
837bb364fa43a407ebcff8856a7b028f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0c1b03dcdaf2a5663d9ebfe27091c4ef8720f14f0099bc17dc7fb66b32bb623c0eddf54c59f14fbdbec34859860b363abaee2595a66306a574c7e2b38a116ac7
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8a9b53ae0b638f60def5e566b6d72f8d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
093230989979d880e4de75bed92a999346e91ca7d7c5d44023c00eb71befb38b2d132982f1ec865796ee2c920359f8122978bec6312a4bc9e143c2a47ddeba91
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c23e7867fd1e6c9d208cb88dfafbb89d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d81c8bb0c101922aa0bdffb6c312e2403e2c3311ade2906a5cbdcd3daaf6c78b2595b5402a6d92deb0c3b5a2009517b16e5bb9ef3d1ac38b1bb1010be4e78eca
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fef715afdf80aba021b3a0460a945d84
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
70e3fc0890f9cf4dde1b06ea8fe9962afe14de0291f3ce6b200a34536fb87562ee4d7513e1f827cff6c2df1ec9347e0c36b76194af16731b08d35bf381c9bfa4
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8510570603eb363fa4b0316b7c664528
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9d7004c28cfbdf523db99d228059295cb0c566d2885712229f2ab1109e615b01e622e0203b3d919771e344ea4c23a1a46dd87f8746895bfcdc1d8703f2f74845
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f3b4eb03e7228b7121ccc988265f3953
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fb523b1ecbd216e4f601b70f81d2505dc2c2ec5626af22ec5c5c5aaeb77041be1c4077e4c117e09fab7fca5edcc117b08fb2b017cf076fffa0392fc4c2ac3dbc
8 changes: 5 additions & 3 deletions deps/llvm.mk
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ $(eval $(call LLVM_PATCH,llvm-8.0-D59389-refactor-wmma)) # remove for 9.0
$(eval $(call LLVM_PATCH,llvm-8.0-D59393-mma-ptx63-fix)) # remove for 9.0
$(eval $(call LLVM_PATCH,llvm-8.0-D66657-codegen-degenerate)) # remove for 10.0
$(eval $(call LLVM_PATCH,llvm-8.0-D71495-vectorize-freduce)) # remove for 10.0
# $(eval $(call LLVM_PATCH,llvm-8.0-D65174-limit-merge-stores)) # remove for 10.0
endif # LLVM_VER 8.0

ifeq ($(LLVM_VER_SHORT),9.0)
Expand All @@ -413,6 +414,7 @@ $(eval $(call LLVM_PATCH,llvm7-revert-D44485))
$(eval $(call LLVM_PATCH,llvm-8.0-D66657-codegen-degenerate)) # remove for 10.0
$(eval $(call LLVM_PATCH,llvm-8.0-D71495-vectorize-freduce)) # remove for 10.0
$(eval $(call LLVM_PATCH,llvm-D75072-SCEV-add-type))
$(eval $(call LLVM_PATCH,llvm-9.0-D65174-limit-merge-stores)) # remove for 10.0
endif # LLVM_VER 9.0


Expand Down Expand Up @@ -494,11 +496,11 @@ update-llvm:
git pull --ff-only
endif
else # USE_BINARYBUILDER_LLVM
LLVM_BB_URL_BASE := https://github.com/JuliaBinaryWrappers/LLVM_jll.jl/releases/download/LLVM-v$(LLVM_VER)+$(LLVM_BB_REL)
LLVM_BB_URL_BASE := https://github.com/JuliaBinaryWrappers/LLVM_full_jll.jl/releases/download/LLVM_full-v$(LLVM_VER)+$(LLVM_BB_REL)
ifneq ($(BINARYBUILDER_LLVM_ASSERTS), 1)
LLVM_BB_NAME := LLVM.v$(LLVM_VER)
LLVM_BB_NAME := LLVM_full.v$(LLVM_VER)
else
LLVM_BB_NAME := LLVM.asserts.v$(LLVM_VER)
LLVM_BB_NAME := LLVM_full.asserts.v$(LLVM_VER)
endif

$(eval $(call bb-install,llvm,LLVM,false,true))
Expand Down
116 changes: 116 additions & 0 deletions deps/patches/llvm-9.0-D65174-limit-merge-stores.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
commit f49c107f06c6a98d11a09d758f08554c78b9b933
Author: Wei Mi <wmi@google.com>
Date: Wed Jul 31 19:59:24 2019 +0000

[DAGCombine] Limit the number of times for the same store and root nodes
to bail out in store merging dependence check.

We run into a case where dependence check in store merging bail out many times
for the same store and root nodes in a huge basicblock. That increases compile
time by almost 100x. The patch add a map to track how many times the bailing
out happen for the same store and root, and if it is over a limit, stop
considering the store with the same root as a merging candidate.

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

llvm-svn: 367472

diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index bf62aa86509..2e5ba82af22 100644
--- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -120,6 +120,11 @@ static cl::opt<unsigned> TokenFactorInlineLimit(
"combiner-tokenfactor-inline-limit", cl::Hidden, cl::init(2048),
cl::desc("Limit the number of operands to inline for Token Factors"));

+static cl::opt<unsigned> StoreMergeDependenceLimit(
+ "combiner-store-merge-dependence-limit", cl::Hidden, cl::init(10),
+ cl::desc("Limit the number of times for the same StoreNode and RootNode "
+ "to bail out in store merging dependence check"));
+
namespace {

class DAGCombiner {
@@ -157,6 +162,14 @@ namespace {
/// which have not yet been combined to the worklist.
SmallPtrSet<SDNode *, 32> CombinedNodes;

+ /// Map from candidate StoreNode to the pair of RootNode and count.
+ /// The count is used to track how many times we have seen the StoreNode
+ /// with the same RootNode bail out in dependence check. If we have seen
+ /// the bail out for the same pair many times over a limit, we won't
+ /// consider the StoreNode with the same RootNode as store merging
+ /// candidate again.
+ DenseMap<SDNode *, std::pair<SDNode *, unsigned>> StoreRootCountMap;
+
// AA - Used for DAG load/store alias analysis.
AliasAnalysis *AA;

@@ -241,6 +254,7 @@ namespace {
void removeFromWorklist(SDNode *N) {
CombinedNodes.erase(N);
PruningList.remove(N);
+ StoreRootCountMap.erase(N);

auto It = WorklistMap.find(N);
if (It == WorklistMap.end())
@@ -15423,6 +15437,18 @@ void DAGCombiner::getStoreMergeCandidates(
return (BasePtr.equalBaseIndex(Ptr, DAG, Offset));
};

+ // Check if the pair of StoreNode and the RootNode already bail out many
+ // times which is over the limit in dependence check.
+ auto OverLimitInDependenceCheck = [&](SDNode *StoreNode,
+ SDNode *RootNode) -> bool {
+ auto RootCount = StoreRootCountMap.find(StoreNode);
+ if (RootCount != StoreRootCountMap.end() &&
+ RootCount->second.first == RootNode &&
+ RootCount->second.second > StoreMergeDependenceLimit)
+ return true;
+ return false;
+ };
+
// We looking for a root node which is an ancestor to all mergable
// stores. We search up through a load, to our root and then down
// through all children. For instance we will find Store{1,2,3} if
@@ -15452,7 +15478,8 @@ void DAGCombiner::getStoreMergeCandidates(
if (StoreSDNode *OtherST = dyn_cast<StoreSDNode>(*I2)) {
BaseIndexOffset Ptr;
int64_t PtrDiff;
- if (CandidateMatch(OtherST, Ptr, PtrDiff))
+ if (CandidateMatch(OtherST, Ptr, PtrDiff) &&
+ !OverLimitInDependenceCheck(OtherST, RootNode))
StoreNodes.push_back(MemOpLink(OtherST, PtrDiff));
}
} else
@@ -15462,7 +15489,8 @@ void DAGCombiner::getStoreMergeCandidates(
if (StoreSDNode *OtherST = dyn_cast<StoreSDNode>(*I)) {
BaseIndexOffset Ptr;
int64_t PtrDiff;
- if (CandidateMatch(OtherST, Ptr, PtrDiff))
+ if (CandidateMatch(OtherST, Ptr, PtrDiff) &&
+ !OverLimitInDependenceCheck(OtherST, RootNode))
StoreNodes.push_back(MemOpLink(OtherST, PtrDiff));
}
}
@@ -15520,8 +15548,19 @@ bool DAGCombiner::checkMergeStoreCandidatesForDependencies(
// Search through DAG. We can stop early if we find a store node.
for (unsigned i = 0; i < NumStores; ++i)
if (SDNode::hasPredecessorHelper(StoreNodes[i].MemNode, Visited, Worklist,
- Max))
+ Max)) {
+ // If the searching bail out, record the StoreNode and RootNode in the
+ // StoreRootCountMap. If we have seen the pair many times over a limit,
+ // we won't add the StoreNode into StoreNodes set again.
+ if (Visited.size() >= Max) {
+ auto &RootCount = StoreRootCountMap[StoreNodes[i].MemNode];
+ if (RootCount.first == RootNode)
+ RootCount.second++;
+ else
+ RootCount = {RootNode, 1};
+ }
return false;
+ }
return true;
}

8 changes: 8 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7385,6 +7385,14 @@ extern "C" void jl_init_llvm(void)
#endif
cl::ParseEnvironmentOptions("Julia", "JULIA_LLVM_ARGS");

// if the patch adding this option has been applied, lower its limit to provide
// better DAGCombiner performance.
auto &clOptions = cl::getRegisteredOptions();
if (clOptions.find("combiner-store-merge-dependence-limit") != clOptions.end()) {
const char *const argv_smdl[] = {"", "-combiner-store-merge-dependence-limit=4"};
cl::ParseCommandLineOptions(sizeof(argv_smdl)/sizeof(argv_smdl[0]), argv_smdl);
}

jl_page_size = jl_getpagesize();
imaging_mode = jl_generating_output() && !jl_options.incremental;
jl_default_cgparams.module_setup = jl_nothing;
Expand Down

2 comments on commit b637cb7

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.