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

Empty landing pads aren't optimized away on x86_64-pc-windows-gnu #43151

Closed
alexcrichton opened this issue Jul 10, 2017 · 7 comments
Closed

Empty landing pads aren't optimized away on x86_64-pc-windows-gnu #43151

alexcrichton opened this issue Jul 10, 2017 · 7 comments
Labels
A-codegen Area: Code generation C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

It looks like empty landing pads are not optimized away due to our usage of rust_eh_unwind_resume instead of the standard resumption function. For example:

// bar.rs
#![crate_type = "rlib"]
pub fn bar() {}
// foo.rs
#![crate_type = "rlib"]
extern crate bar;

struct A;

impl Drop for A {
    fn drop(&mut self) {
    }
}

pub fn foo() {
    let _a = A;
    bar::bar();
}
$ rustc bar.rs --target x86_64-pc-windows-gnu
$ rustc foo.rs --target x86_64-pc-windows-gnu -L . -O --emit llvm-ir
$ cat foo.ll

yields this IR, notably:

; foo::foo
; Function Attrs: uwtable
define void @_ZN3foo3foo17h6cb4f4601b3a0374E() unnamed_addr #1 personality i32 (%"panic_unwind::windows::EXCEPTION_RECORD"*, i8*, %"panic_unwind::windows::CONTEXT"*, %"panic_unwind::windows::DISPATCHER_CONTEXT"*)* @rust_eh_personality {
start:
; invoke bar::bar
  invoke void @_ZN3bar3bar17h21868f3a4452d05bE()
          to label %bb2 unwind label %cleanup

bb2:                                              ; preds = %start
  ret void

cleanup:                                          ; preds = %start
  %0 = landingpad { i8*, i32 }
          cleanup
  %.fca.0.extract = extractvalue { i8*, i32 } %0, 0
  tail call void @rust_eh_unwind_resume(i8* %.fca.0.extract)
  unreachable
}

On other targets such as x86_64-unknown-linux-gnu this is optimized away.

@alexcrichton alexcrichton added A-codegen Area: Code generation O-windows-gnu Toolchain: GNU, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2017
@alexcrichton
Copy link
Member Author

cc @vadimcn

@alexcrichton
Copy link
Member Author

@vadimcn I wonder if we should codegen resume instructions and then only after optimizations have all finished we translate them all back to funciton calls to rust_eh_unwind_resume? In theory that'll allow optimization passes to optimize as usual without having to modify LLVM?

@vadimcn
Copy link
Contributor

vadimcn commented Jul 10, 2017

@alexcrichton: Do you know why those landing pads are empty? Is it because LLVM has optimized everything away, or just because we've created them empty? If the latter, maybe we can avoid creating empty landing pads in the first place?

@alexcrichton
Copy link
Member Author

@vadimcn I think it's due to #43150, but I figure we'd want to perform this optimization here regardless. I think lots of times landing pads can optimize away to nothing after other passes like constant opragation.

@vadimcn
Copy link
Contributor

vadimcn commented Jul 10, 2017

@alexcrichton, yes, I suppose we could do what you suggest.

Also, looks like it might be possible to override the libcall name: targetMachine->getSubtargetImpl(Fn)->getTargetLowering()->setLibcallName(RTLIB::UNWIND_RESUME, "rust_eh_unwind_resume")?

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 28, 2017
@mati865
Copy link
Contributor

mati865 commented Apr 3, 2020

@alexcrichton is it still relevant?
Nightly IR:

; ModuleID = 'foo.3a1fbbbh-cgu.0'
source_filename = "foo.3a1fbbbh-cgu.0"
target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-gnu"

%A = type {}
%"unwind::libunwind::EXCEPTION_RECORD" = type { [0 x i8] }
%"unwind::libunwind::CONTEXT" = type { [0 x i8] }
%"unwind::libunwind::DISPATCHER_CONTEXT" = type { [0 x i8] }

; <foo::A as core::ops::drop::Drop>::drop
; Function Attrs: norecurse nounwind readnone uwtable
define void @"_ZN48_$LT$foo..A$u20$as$u20$core..ops..drop..Drop$GT$4drop17h095156138f26c8b3E"(%A* nocapture nonnull align 1 %self) unnamed_addr #0 {
start:
  ret void
}

; foo::foo
; Function Attrs: uwtable
define void @_ZN3foo3foo17he8a114b84f989db4E() unnamed_addr #1 personality i32 (%"unwind::libunwind::EXCEPTION_RECORD"*, i8*, %"unwind::libunwind::CONTEXT"*, %"unwind::libunwind::DISPATCHER_CONTEXT"*)* @rust_eh_personality {
start:
; call bar::bar
  tail call void @_ZN3bar3bar17h6d6497eda9d60abfE()
  ret void
}

; Function Attrs: nounwind uwtable
declare i32 @rust_eh_personality(%"unwind::libunwind::EXCEPTION_RECORD"*, i8*, %"unwind::libunwind::CONTEXT"*, %"unwind::libunwind::DISPATCHER_CONTEXT"*) unnamed_addr #2

; bar::bar
; Function Attrs: uwtable
declare void @_ZN3bar3bar17h6d6497eda9d60abfE() unnamed_addr #1

attributes #0 = { norecurse nounwind readnone uwtable "target-cpu"="x86-64" }
attributes #1 = { uwtable "target-cpu"="x86-64" }
attributes #2 = { nounwind uwtable "target-cpu"="x86-64" }

!llvm.module.flags = !{!0}

!0 = !{i32 7, !"PIC Level", i32 2}

@alexcrichton
Copy link
Member Author

Looks like the need for rust_eh_unwind_resume was removed in be055d9, so yes, this is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows 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