Do not emit alloca for ZST locals with multiple assignments #86166
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit e8b2d46ac74d3e072631d607a868935393b47e66 with merge 004afd4df22776fea09704d480c8e7b8b1c10a1e... |
|
☀️ Try build successful - checks-actions |
|
Queued 004afd4df22776fea09704d480c8e7b8b1c10a1e with parent c4b5406, future comparison URL. |
|
r? @nagisa |
|
Finished benchmarking try commit (004afd4df22776fea09704d480c8e7b8b1c10a1e): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
When rebuilding the standard library with `-Zbuild-std` this reduces the number of locals that require an allocation from 62315 to 61767.
e8b2d46 to
40c9aae
Compare
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit 40c9aae with merge 6f83711458d087a929d9bedb62c37342e448eb97... |
|
☀️ Try build successful - checks-actions |
|
Queued 6f83711458d087a929d9bedb62c37342e448eb97 with parent 0279cb1, future comparison URL. |
|
Finished benchmarking try commit (6f83711458d087a929d9bedb62c37342e448eb97): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
This seems pretty low value to me. We already don't emit any load/store operations on the ZST allocas (AFAIR), and so these allocas are trivially removable by LLVM. The perf data seems similarly mixed. But I do find the new code nicer to read, so @bors r+ |
|
📌 Commit 40c9aae has been approved by |
|
☀️ Test successful - checks-actions |
This extends 35566bf to additionally stop emitting unnecessary allocas for zero sized locals that are assigned multiple times.
When rebuilding the standard library with
-Zbuild-stdthis reduces the number of locals that require an allocation from 62315 to 61767.