Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Disable nonzeroing move hint by default #27582

Merged
merged 3 commits into from
Aug 7, 2015
Merged

Disable nonzeroing move hint by default #27582

merged 3 commits into from
Aug 7, 2015

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Aug 7, 2015

Turn nonzeroing move hints back off by default.

Works around bugs injected by PR #26173.

  • (@pnkfelix is unavailable in the short-term (i.e. for the next week) to fix them.)
  • When the bugs are fixed, we will turn nonzeroing move hints back on by default.

Fix #27401

pnkfelix added 2 commits August 7, 2015 15:51
@pnkfelix
Turn nonzeroing move hints back off by default.
e9b28d2 
This is a temporary workaround for the bugs that have been found in
the implementation of PR rust-lang#26173.

 * pnkfelix is unavailable in the short-term (i.e. for the next week) to fix them.

 * When the bugs are fixed, we will turn this back on by default.

(If you want to play with the known-to-be-buggy optimization in the
meantime, you can opt-back in via the debugging option that this
commit is toggling.)
@pnkfelix
Regression test for dropflag reinit issue.
6170435 
Fix rust-lang#27401.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 7, 2015

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Aug 7, 2015

📌 Commit 6170435 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Aug 7, 2015

⌛ Testing commit 6170435 with merge e13c184...

@bors
Copy link
Contributor

bors commented Aug 7, 2015

💔 Test failed - auto-mac-64-opt

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 7, 2015

gah pretty!

I can understand pretty not likely the comment format I chose (I did some odd things to get //'s to line up), but looking at http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/6007/steps/test/logs/stdio I simply don't understand what the pretty is doing here. (In particular, how is it deciding to move the comment in line 34 in both the actual and expected output, while line 31 is moved backward?)

I'm just going to mark this as ignore-pretty with a link to this PR so someone can at least see my comment here.

@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 7, 2015

@bors r=nikomatsakis p=10 1a68b18

@bors
Copy link
Contributor

bors commented Aug 7, 2015

📌 Commit 1a68b18 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Aug 7, 2015

⌛ Testing commit 1a68b18 with merge 1b5d521...

bors added a commit that referenced this pull request Aug 7, 2015
@bors
Auto merge of #27582 - pnkfelix:disable-nonzeroing-move-hint-by-defau…
1b5d521 
…lt, r=nikomatsakis

Turn nonzeroing move hints back off by default.

Works around bugs injected by PR #26173.

 * (@pnkfelix is unavailable in the short-term (i.e. for the next week) to fix them.)

 * When the bugs are fixed, we will turn nonzeroing move hints back on by default.

Fix #27401
@pnkfelix pnkfelix mentioned this pull request Aug 7, 2015
Closed
@bors
Copy link
Contributor

bors commented Aug 7, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

@bors bors merged commit 1a68b18 into rust-lang:master Aug 7, 2015
@pnkfelix
Copy link
Member Author

pnkfelix commented Aug 8, 2015

@nikomatsakis BTW this needs to be back ported to the beta channel. I can do it in 9 days but probably better to delegate to someone else this week if possible

@pnkfelix pnkfelix mentioned this pull request Aug 8, 2015
Closed
@alexcrichton alexcrichton added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 10, 2015
@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 10, 2015
@nikomatsakis
Copy link
Contributor

Seems like a no-brainer to backport.

@brson brson mentioned this pull request Aug 10, 2015
Merged
@brson brson removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop flags on the stack not reinitialized in loops
6 participants
@pnkfelix @rust-highfive @nikomatsakis @bors @alexcrichton @brson