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

Do NRVO when type of returned local does not match return type #72428

Closed
ecstatic-morse opened this issue May 21, 2020 · 1 comment · Fixed by #72451
Closed

Do NRVO when type of returned local does not match return type #72428

ecstatic-morse opened this issue May 21, 2020 · 1 comment · Fixed by #72451
Labels
A-mir-opt Area: MIR optimizations A-mir-opt-nrvo Fixed by NRVO C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ecstatic-morse
Copy link
Contributor

#72205 will fail to fire for code such as the following:

fn foo(x: &mut i32) -> &i32 { x }

This is because, by the time we get to the RenameReturnPlace pass, the reborrow has been optimized away and we have MIR like:

_0: &i32 = _1: &mut i32

To merge these locals, we need to pick a type for the unified LocalDecl. However, I wasn't sure how or if codegen makes use of the types of locals, so for now #72205 just doesn't run if the locals have different type. We should come up with a solution here.
@ecstatic-morse ecstatic-morse added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-mir-opt Area: MIR optimizations A-mir-opt-nrvo Fixed by NRVO labels May 21, 2020
@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented May 21, 2020 •
edited
Loading

Worth noting that an initial perf run of #72205 was a bit faster than the one for the version that was merged. This issue is one plausible explanation, although there was also an LLVM upgrade in between.

@jonas-schievink jonas-schievink added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label May 21, 2020
@ecstatic-morse ecstatic-morse mentioned this issue May 22, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 25, 2020
@Dylan-DPC
Rollup merge of rust-lang#72451 - ecstatic-morse:nrvo-type-mismatch, …
123d918 
…r=matthewjasper

Perform MIR NRVO even if types don't match

This is the most straightforward way to resolve rust-lang#72428, but it could cause problems in codegen since the type of `_0` may no longer match the return type of the body.
@bors bors closed this as completed in 036688f May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-mir-opt Area: MIR optimizations A-mir-opt-nrvo Fixed by NRVO C-enhancement Category: An issue proposing an enhancement or a PR with one. 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 a pull request may close this issue.

Perform MIR NRVO even if types don't match ecstatic-morse/rust
2 participants
@jonas-schievink @ecstatic-morse