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.

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

Unnecessary drop flags #41110

Closed
dotdash opened this issue Apr 6, 2017 · 1 comment
Closed

Unnecessary drop flags #41110

dotdash opened this issue Apr 6, 2017 · 1 comment
Assignees
Labels
P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dotdash
Copy link
Contributor

dotdash commented Apr 6, 2017

In the following example, there's an unnecessary drop flag (and an associated cleanup path) for the result of the S.id() call. This is only present in beta and nightly, stable does not have this drop flag.

#![crate_type="rlib"]

struct S;
impl Drop for S {
    fn drop(&mut self) {
    }
}

impl S {
    fn id(self) -> Self { self }
    fn other(self, s: Self) {}
}

pub fn test() {
    let x = S.other(S.id());
}

IR in stable:

bb2:                                              ; preds = %start
  store i8 0, i8* %_5, !dbg !47
  invoke void @_ZN8rust_out1S5other17he982095737f244abE()
          to label %bb3 unwind label %cleanup, !dbg !47

IR in beta/nightly

bb2:                                              ; preds = %start
  store i8 1, i8* %_6, !dbg !47
  store i8 0, i8* %_5, !dbg !47
  store i8 0, i8* %_6, !dbg !47
  invoke void @_ZN8rust_out1S5other17hbd876dece79ff804E()
          to label %bb4 unwind label %cleanup1, !dbg !47

LLVM is able to remove the unnecessary flag and the drop call, but cannot remove the extraneous landing pads.

@dotdash dotdash added I-nominated regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Apr 6, 2017
@arielb1 arielb1 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 6, 2017
@pnkfelix
Copy link
Member

pnkfelix commented Apr 6, 2017

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Apr 6, 2017
arielb1 added a commit to arielb1/rust that referenced this issue Apr 8, 2017
This avoids creating drop flags in many unnecessary situations.

Fixes rust-lang#41110.
bors added a commit that referenced this issue Apr 8, 2017
borrowck::mir::dataflow: ignore unwind edges of empty drops

This avoids creating drop flags in many unnecessary situations.

Fixes #41110.

r? @nagisa

beta-nominating because regression. However, that is merely a small perf regression and codegen changes are always risky, so we might let this slide for 1.17.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants