-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Set opt-level = 3 the third time. #52212
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
@bors try |
Set opt-level = 3 the third time. This PR reverts #51165 (set -O2 to fix #50867), which reverted #50329 (set -O3), which was second attempt of #48204 (set -O3, closed due to Windows segfault that is fixed now), which reverted #42123 (set -O2 to fix spurious Windows segfaults), which reverted #41967 (set -O3). Last time we've found that setting -O3 regressed the wall time of NLL (#50329 (comment)), so we may need another perf run to confirm. I'd like to check this *after* the LLVM 7 upgrade #51966 has been merged, so marking this as <kbd>S-blocked</kbd> for now.
☀️ Test successful - status-travis |
@rust-timer build 25ff830 |
Success: Queued 25ff830 with parent ae5b629, comparison URL. |
The direct comparison is still not ready, an indirect comparison https://perf.rust-lang.org/compare.html?start=989fa053895a27fd40896335224b619843b7e58a&end=25ff830981232dd0cd40311f391e045fe8a4d1d0 is green everywhere. For the NLL dashboard:
The ratio seems to be not regressing. Also I believe this is now ready for review. Edit: It seems the NLL comparison has been moved elsewhere, checking again. |
@bors: r+ 🥇 |
📌 Commit e12b1b6 has been approved by |
⌛ Testing commit e12b1b6 with merge ece70aaac56d450315409b6267e7af11df272249... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
rustc segfault when linking |
The segfault is spurious 😢 |
Blocked by #52333. I'm not confident to proceed without any tools to determine where the segfault is happening, in case we want to trace the spurious failure in the future (if it happens again). |
@bors r=alexcrichton |
📌 Commit 6c635b7 has been approved by |
Set opt-level = 3 the third time. This PR reverts #51165 (set -O2 for fixing #50867), which reverted #50329 (set -O3), which was second attempt of #48204 (set -O3, closed due to Windows segfault that is fixed now), which reverted #42123 (set -O2 to fix spurious Windows segfaults), which reverted #41967 (set -O3). Since we have found the root cause of #50867, this optimization could be tried again. Last time we've found that setting -O3 regressed the wall time of NLL (#50329 (comment)), so we may need another perf run to confirm. I'd like to check this *after* the LLVM 7 upgrade #51966 has been merged, so marking this as <kbd>S-blocked</kbd> for now.
☀️ Test successful - status-appveyor, status-travis |
Unfortunately there's strong enough evident that the segfault is real, so we're going to revert this PR until the cause (#52378) is understood. Maybe we could try again when LLVM 7 is finalized. |
This PR reverts #51165 (set -O2 for fixing #50867),
which reverted #50329 (set -O3),
which was second attempt of #48204 (set -O3, closed due to Windows segfault that is fixed now),
which reverted #42123 (set -O2 to fix spurious Windows segfaults),
which reverted #41967 (set -O3).
Since we have found the root cause of #50867, this optimization could be tried again.
Last time we've found that setting -O3 regressed the wall time of NLL (#50329 (comment)), so we may need another perf run to confirm. I'd like to check this after the LLVM 7 upgrade #51966 has been merged, so marking this as S-blocked for now.