Skip to content

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

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

Jump to bottom
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 the Named Return Value Opt. (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 the Named Return Value Opt. (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
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
A-mir-opt Area: MIR optimizations A-mir-opt-nrvo Fixed by the Named Return Value Opt. (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