-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[InstCombine] Use more inline elements in a SmallVector #100942
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
[InstCombine] Use more inline elements in a SmallVector #100942
Conversation
The 4 inline elements only cover 58% of cases encountered here during the compilation of X86ISelLowering.cpp.ll, a .ll version of X86ISelLowering.cpp. The 8 inline elements cover 96% and save 0.27% of heap allocations.
|
@llvm/pr-subscribers-llvm-transforms Author: Kazu Hirata (kazutakahirata) ChangesThe 4 inline elements only cover 58% of cases encountered here during The 8 inline elements cover 96% and save 0.27% of heap allocations. Full diff: https://github.com/llvm/llvm-project/pull/100942.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index f6c4b6e180937..e61480d7542d7 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1500,7 +1500,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// Don't try to simplify calls without uses. It will not do anything useful,
// but will result in the following folds being skipped.
if (!CI.use_empty()) {
- SmallVector<Value *, 4> Args;
+ SmallVector<Value *, 8> Args;
Args.reserve(CI.arg_size());
for (Value *Op : CI.args())
Args.push_back(Op);
|
nikic
left a comment
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/154/builds/2038 Here is the relevant piece of the build log for the reference: |
| // but will result in the following folds being skipped. | ||
| if (!CI.use_empty()) { | ||
| SmallVector<Value *, 4> Args; | ||
| SmallVector<Value *, 8> Args; |
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.
Can this be rewritten as
SmallVector<Value *, 8> Args(CI.arg_begin(), CI.arg_end());
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.
Yes, thank you for bringing this up. It can actually be even shorter:
SmallVector<Value *, 8> Args(CI.args());
I'll post a follow-up PR.
The 4 inline elements only cover 58% of cases encountered here during
the compilation of X86ISelLowering.cpp.ll, a .ll version of
X86ISelLowering.cpp.
The 8 inline elements cover 96% and save 0.27% of heap allocations.