-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[SLP] Increase UsesLimit to 64 #99467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Since commit 82b800e addressed the issue llvm#99327 , we see some performance regression (13%) on some verilator generated C++ code. This is because the UsesLimit is set to 8, which is too small for the verilator generated code. I have analyzed the need for the UsesLimit from [1] and found that the UsesLimit should be at least 64 to cover most of these cases. Thus, This patch increases the UsesLimit to 64. Link: llvm#99327 (comment) [1] Signed-off-by: Yangyu Chen <[email protected]>
@llvm/pr-subscribers-llvm-transforms Author: Yangyu Chen (cyyself) ChangesSince commit 82b800e addressed the issue #99327 , we see some performance regression (13%) on some verilator generated C++ code. This is because the UsesLimit is set to 8, which is too small for the verilator generated code. I have analyzed the need for the UsesLimit from [1] and found that the UsesLimit should be at least 64 to cover most of these cases. Thus, This patch increases the UsesLimit to 64. Link: #99327 (comment) [1] Full diff: https://github.com/llvm/llvm-project/pull/99467.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index d88c6307b994b..6f0878d5384be 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -209,7 +209,7 @@ static const unsigned AliasedCheckLimit = 10;
// Limit of the number of uses for potentially transformed instructions/values,
// used in checks to avoid compile-time explode.
-static constexpr int UsesLimit = 8;
+static constexpr int UsesLimit = 64;
// Another limit for the alias checks: The maximum distance between load/store
// instructions where alias checks are done.
|
I've created a branch (perf/slp_usecase_limit) to determine the effect on compile time - it should appear on https://llvm-compile-time-tracker.com/?config=Overview&stat=instructions%3Au&remote=RKSimon in the next hour |
|
Yeah. Surprisingly it also improves the performance of clang stage2-build. |
Most probably, it has lots of uses, which must be replaced by the new extractelement instructions. And if the number is >8 but <64, better to walk over actual uses, rather than all uses (including those, potentially deleted/dropped a bit later) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
Since commit 82b800e addressed the issue #99327 , we see some performance regression (13%) on some verilator generated C++ code. This is because the UsesLimit is set to 8, which is too small for the verilator generated code. I have analyzed the need for the UsesLimit from [1] and found that the UsesLimit should be at least 64 to cover most of these cases. Thus, This patch increases the UsesLimit to 64. Link: #99327 (comment) [1] Signed-off-by: Yangyu Chen <[email protected]>
Since commit 82b800e addressed the issue #99327 , we see some performance regression (13%) on some verilator generated C++ code. This is because the UsesLimit is set to 8, which is too small for the verilator generated code. I have analyzed the need for the UsesLimit from [1] and found that the UsesLimit should be at least 64 to cover most of these cases. Thus, This patch increases the UsesLimit to 64.
Link: #99327 (comment) [1]