-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[AArch64] Lower jump table cases threshold to 10 #143632
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
[AArch64] Lower jump table cases threshold to 10 #143632
Conversation
@llvm/pr-subscribers-backend-aarch64 Author: Guy David (guy-david) ChangesPrevious stabs at this setting (#71166) hypertuned it for SPEC2017, but Clang's own compilation can benefit from a slightly lower threshold, yielding a 0.3% improvement in compile time, while still not regressing SPEC. Most notable benefactors of this change are:
Test Suite:
Full diff: https://github.com/llvm/llvm-project/pull/143632.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64Subtarget.cpp b/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
index a28e6bad0dca0..68ed10570a52f 100644
--- a/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -73,7 +73,7 @@ static cl::opt<AArch64PAuth::AuthCheckMethod>
cl::values(AUTH_CHECK_METHOD_CL_VALUES_LR));
static cl::opt<unsigned> AArch64MinimumJumpTableEntries(
- "aarch64-min-jump-table-entries", cl::init(13), cl::Hidden,
+ "aarch64-min-jump-table-entries", cl::init(10), cl::Hidden,
cl::desc("Set minimum number of entries to use a jump table on AArch64"));
static cl::opt<unsigned> AArch64StreamingHazardSize(
|
Previous stabs at this setting hypertuned it for SPEC2017, but Clang's own compilation can benefit from a slightly lower threshold, yielding a 0.3% improvement in compile time, while still not regressing SPEC. Most notable benefactors of this change are: - llvm::Instruction::getNumSuccessors (11 cases) - llvm::Instruction::getSuccessor (11 cases)
925a6c7
to
54232b8
Compare
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.
LGTM! Seems reasonable. If we do see any other regressions due to the change it's always easy enough to revert.
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.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/24866 Here is the relevant piece of the build log for the reference
|
Previous stabs at this setting (#71166) hypertuned it for SPEC2017, but Clang's own compilation can benefit from a slightly lower threshold, yielding a 0.3% improvement in compile time, while still not regressing SPEC.
Most notable beneficiaries of this change are:
llvm::Instruction::getNumSuccessors
(11 cases)llvm::Instruction::getSuccessor
(11 cases)Test Suite with a bootstrapped build: