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

[MIR] improve operand lifetimes #40133

Merged
merged 4 commits into from
Mar 3, 2017
Merged

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Feb 27, 2017

r? @eddyb

@arielb1
Copy link
Contributor Author

arielb1 commented Feb 27, 2017

cc @nagisa

@eddyb
Copy link
Member

eddyb commented Feb 27, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 27, 2017

📌 Commit 7f8529d has been approved by eddyb

@eddyb
Copy link
Member

eddyb commented Feb 28, 2017

@bors r-

/checkout/src/test/codegen/lifetime_start_end.rs:39:11: error: expected string not found in input
// CHECK: [[E_b:%[0-9]+]] = bitcast %"core::option::Option<i32>"** %b to i8*
          ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/lifetime_start_end.ll:37:2: note: scanning from here
 %9 = bitcast i32* %c to i8*
 ^

@arielb1
Copy link
Contributor Author

arielb1 commented Feb 28, 2017

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Feb 28, 2017

📌 Commit e4458de has been approved by eddyb

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 2, 2017
@frewsxcv
Copy link
Member

frewsxcv commented Mar 2, 2017

Could this failure be related? #40211 (comment)

@frewsxcv
Copy link
Member

frewsxcv commented Mar 2, 2017

Yes, Travis in this PR also has the fail

@bors r-

This reduces the number of dynamic drops in libstd from 1141 to 899.
However, without this change, the next patch would have created much
more dynamic drops.

A basic merge unswitching hack reduced the number of dynamic drops to
644, with no effect on stack usage. I should be writing a more dedicated
drop unswitching pass.

No performance measurements.
@arielb1 arielb1 force-pushed the operand-lifetimes branch from e4458de to 31faaf2 Compare March 3, 2017 00:33
@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

📌 Commit 31faaf2 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

⌛ Testing commit 31faaf2 with merge d6e5b10...

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

💔 Test failed - status-appveyor

arielb1 added 3 commits March 3, 2017 13:54
In MIR construction, operands need to live exactly until they are used,
which is during the (sub)expression that made the call to `as_operand`.

Before this PR, operands lived until the end of the temporary scope,
which was sometimes unnecessarily longer and sometimes too short.

Fixes rust-lang#38669.
@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

Rebase damage.

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Reduce std_unicode’s public API #40189

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

📌 Commit 31faaf2 has been approved by eddyb

@arielb1 arielb1 force-pushed the operand-lifetimes branch from 31faaf2 to f99f1f8 Compare March 3, 2017 11:54
@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

@bors r=eddyb f99f1f8

@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

@bors r-

@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

📌 Commit f99f1f8 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

⌛ Testing commit f99f1f8 with merge b2a424f...

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

💔 Test failed - status-appveyor

@arielb1
Copy link
Contributor Author

arielb1 commented Mar 3, 2017

C:/projects/rust/mingw32/bin/ar.exe: unable to rename '../../../libLLVMSparcDesc.a'; reason: Permission denied

Looks spurious

@bors retry

@bors
Copy link
Collaborator

bors commented Mar 3, 2017

⌛ Testing commit f99f1f8 with merge f0b5145...

bors added a commit that referenced this pull request Mar 3, 2017
@bors
Copy link
Collaborator

bors commented Mar 3, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing f0b5145 to master...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants