Skip to content

Commit

Permalink
Auto merge of #61005 - michaelwoerister:error-pgo-windows-unwind, r=z…
Browse files Browse the repository at this point in the history
…ackmdavis

Emit error when trying to use PGO in conjunction with unwinding on Windows.

This PR makes `rustc` emit an error when trying use PGO in conjunction with `-Cpanic=unwind` on Windows, isn't supported by LLVM yet. The error messages points to #61002, which documents this known limitation.
  • Loading branch information
bors committed May 30, 2019
2 parents 19e0ddb + a19a9e9 commit c28084a
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 8 deletions.
12 changes: 12 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,18 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
path.display()));
}
}

// PGO does not work reliably with panic=unwind on Windows. Let's make it
// an error to combine the two for now. It always runs into an assertions
// if LLVM is built with assertions, but without assertions it sometimes
// does not crash and will probably generate a corrupted binary.
if sess.opts.debugging_opts.pgo_gen.enabled() &&
sess.target.target.options.is_like_msvc &&
sess.panic_strategy() == PanicStrategy::Unwind {
sess.err("Profile-guided optimization does not yet work in conjunction \
with `-Cpanic=unwind` on Windows when targeting MSVC. \
See https://github.com/rust-lang/rust/issues/61002 for details.");
}
}

/// Hash value constructed out of all the `-C metadata` arguments passed to the
Expand Down
11 changes: 7 additions & 4 deletions src/test/codegen/pgo-instrumentation.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
// Test that `-Zpgo-gen` creates expected instrumentation artifacts in LLVM IR.
// Compiling with `-Cpanic=abort` because PGO+unwinding isn't supported on all platforms.

// needs-profiler-support
// compile-flags: -Z pgo-gen -Ccodegen-units=1
// compile-flags: -Z pgo-gen -Ccodegen-units=1 -Cpanic=abort

// CHECK: @__llvm_profile_raw_version =
// CHECK: @__profc_{{.*}}pgo_instrumentation{{.*}}some_function{{.*}} = private global
// CHECK: @__profd_{{.*}}pgo_instrumentation{{.*}}some_function{{.*}} = private global
// CHECK: @__profc_{{.*}}pgo_instrumentation{{.*}}main{{.*}} = private global
// CHECK: @__profd_{{.*}}pgo_instrumentation{{.*}}main{{.*}} = private global
// CHECK: @__profc_{{.*}}pgo_instrumentation{{.*}}some_other_function{{.*}} = private global
// CHECK: @__profd_{{.*}}pgo_instrumentation{{.*}}some_other_function{{.*}} = private global
// CHECK: @__llvm_profile_filename = {{.*}}"default_%m.profraw\00"{{.*}}

#![crate_type="lib"]

#[inline(never)]
fn some_function() {

}

fn main() {
pub fn some_other_function() {
some_function();
}
12 changes: 11 additions & 1 deletion src/test/run-make-fulldeps/pgo-gen-lto/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@

-include ../tools.mk

COMPILE_FLAGS=-Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)"

# LLVM doesn't yet support instrumenting binaries that use unwinding on MSVC:
# https://github.com/rust-lang/rust/issues/61002
#
# Things work fine with -Cpanic=abort though.
ifdef IS_MSVC
COMPILE_FLAGS+= -Cpanic=abort
endif

all:
$(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)" test.rs
$(RUSTC) $(COMPILE_FLAGS) test.rs
$(call RUN,test) || exit 1
[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
12 changes: 11 additions & 1 deletion src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,18 @@

-include ../tools.mk

COMPILE_FLAGS=-O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)"

# LLVM doesn't yet support instrumenting binaries that use unwinding on MSVC:
# https://github.com/rust-lang/rust/issues/61002
#
# Things work fine with -Cpanic=abort though.
ifdef IS_MSVC
COMPILE_FLAGS+= -Cpanic=abort
endif

all:
$(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)" --emit=llvm-ir test.rs
$(RUSTC) $(COMPILE_FLAGS) --emit=llvm-ir test.rs
# We expect symbols starting with "__llvm_profile_".
$(CGREP) "__llvm_profile_" < $(TMPDIR)/test.ll
# We do NOT expect the "__imp_" version of these symbols.
Expand Down
12 changes: 11 additions & 1 deletion src/test/run-make-fulldeps/pgo-gen/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@

-include ../tools.mk

COMPILE_FLAGS=-g -Z pgo-gen="$(TMPDIR)"

# LLVM doesn't yet support instrumenting binaries that use unwinding on MSVC:
# https://github.com/rust-lang/rust/issues/61002
#
# Things work fine with -Cpanic=abort though.
ifdef IS_MSVC
COMPILE_FLAGS+= -Cpanic=abort
endif

all:
$(RUSTC) -g -Z pgo-gen="$(TMPDIR)" test.rs
$(RUSTC) $(COMPILE_FLAGS) test.rs
$(call RUN,test) || exit 1
[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
2 changes: 1 addition & 1 deletion src/test/run-make-fulldeps/pgo-use/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
COMMON_FLAGS=-Copt-level=s -Ccodegen-units=1

# LLVM doesn't support instrumenting binaries that use SEH:
# https://bugs.llvm.org/show_bug.cgi?id=41279
# https://github.com/rust-lang/rust/issues/61002
#
# Things work fine with -Cpanic=abort though.
ifdef IS_MSVC
Expand Down

0 comments on commit c28084a

Please sign in to comment.