Rollup merge of #60038 - michaelwoerister:pgo-updates-2, r=alexcrichton

Centril · web-flow · commit bb892be98ede · 2019-04-25T03:05:22.000+02:00
Add codegen test for PGO instrumentation. This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`. The second commit updates a test case to the new commandline option syntax introduced in #59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
@@ -1268,11 +1268,11 @@ impl Step for Compiletest {
         builder.add_rust_test_threads(&mut cmd);
 
         if builder.config.sanitizers {
-            cmd.env("SANITIZER_SUPPORT", "1");
+            cmd.env("RUSTC_SANITIZER_SUPPORT", "1");
         }
 
         if builder.config.profiler {
-            cmd.env("PROFILER_SUPPORT", "1");
+            cmd.env("RUSTC_PROFILER_SUPPORT", "1");
         }
 
         cmd.env("RUST_TEST_TMPDIR", builder.out.join("tmp"));
diff --git a/src/test/codegen/pgo-instrumentation.rs b/src/test/codegen/pgo-instrumentation.rs
@@ -0,0 +1,20 @@
+// Test that `-Zpgo-gen` creates expected instrumentation artifacts in LLVM IR.
+
+// needs-profiler-support
+// compile-flags: -Z pgo-gen -Ccodegen-units=1
+
+// 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: @__llvm_profile_filename = {{.*}}"default_%m.profraw\00"{{.*}}
+
+#[inline(never)]
+fn some_function() {
+
+}
+
+fn main() {
+    some_function();
+}
diff --git a/src/test/run-make-fulldeps/pgo-gen-lto/Makefile b/src/test/run-make-fulldeps/pgo-gen-lto/Makefile
@@ -1,8 +1,8 @@
+# needs-profiler-support
+
 -include ../tools.mk
 
 all:
-ifeq ($(PROFILER_SUPPORT),1)
 	$(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)" test.rs
 	$(call RUN,test) || exit 1
 	[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
-endif
diff --git a/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile b/src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile
@@ -1,11 +1,11 @@
+# needs-profiler-support
+
 -include ../tools.mk
 
 all:
-ifeq ($(PROFILER_SUPPORT),1)
-	$(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)/test.profraw" --emit=llvm-ir test.rs
+	$(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)" --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.
 	$(CGREP) -v "__imp___llvm_profile_" < $(TMPDIR)/test.ll # 64 bit
 	$(CGREP) -v "__imp____llvm_profile_" < $(TMPDIR)/test.ll # 32 bit
-endif
diff --git a/src/test/run-make-fulldeps/pgo-gen/Makefile b/src/test/run-make-fulldeps/pgo-gen/Makefile
@@ -1,8 +1,8 @@
+# needs-profiler-support
+
 -include ../tools.mk
 
 all:
-ifeq ($(PROFILER_SUPPORT),1)
 	$(RUSTC) -g -Z pgo-gen="$(TMPDIR)" test.rs
 	$(call RUN,test) || exit 1
 	[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
-endif
diff --git a/src/test/run-make-fulldeps/profile/Makefile b/src/test/run-make-fulldeps/profile/Makefile
@@ -1,9 +1,9 @@
+# needs-profiler-support
+
 -include ../tools.mk
 
 all:
-ifeq ($(PROFILER_SUPPORT),1)
 	$(RUSTC) -g -Z profile test.rs
 	$(call RUN,test) || exit 1
 	[ -e "$(TMPDIR)/test.gcno" ] || (echo "No .gcno file"; exit 1)
 	[ -e "$(TMPDIR)/test.gcda" ] || (echo "No .gcda file"; exit 1)
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-address/Makefile b/src/test/run-make-fulldeps/sanitizer-address/Makefile
@@ -1,15 +1,15 @@
+# needs-sanitizer-support
+
 -include ../tools.mk
 
 LOG := $(TMPDIR)/log.txt
 
 # NOTE the address sanitizer only supports x86_64 linux and macOS
 
 ifeq ($(TARGET),x86_64-apple-darwin)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
 EXTRA_RUSTFLAG=-C rpath
 else
 ifeq ($(TARGET),x86_64-unknown-linux-gnu)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
 
 # Apparently there are very specific Linux kernels, notably the one that's
 # currently on Travis CI, which contain a buggy commit that triggers failures in
@@ -23,7 +23,5 @@ endif
 endif
 
 all:
-ifeq ($(ASAN_SUPPORT),1)
 	$(RUSTC) -g -Z sanitizer=address -Z print-link-args $(EXTRA_RUSTFLAG) overflow.rs | $(CGREP) librustc_asan
 	$(TMPDIR)/overflow 2>&1 | $(CGREP) stack-buffer-overflow
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-cdylib-link/Makefile b/src/test/run-make-fulldeps/sanitizer-cdylib-link/Makefile
@@ -1,3 +1,7 @@
+# needs-sanitizer-support
+# only-x86_64
+# only-linux
+
 -include ../tools.mk
 
 LOG := $(TMPDIR)/log.txt
@@ -7,16 +11,10 @@ LOG := $(TMPDIR)/log.txt
 # are compiled with address sanitizer, and we assert that a fault in the cdylib
 # is correctly detected.
 
-ifeq ($(TARGET),x86_64-unknown-linux-gnu)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
-
 # See comment in sanitizer-address/Makefile for why this is here
 EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
-endif
 
 all:
-ifeq ($(ASAN_SUPPORT),1)
 	$(RUSTC) -g -Z sanitizer=address --crate-type cdylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
 	$(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) $(EXTRA_RUSTFLAG) -llibrary program.rs
 	LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-dylib-link/Makefile b/src/test/run-make-fulldeps/sanitizer-dylib-link/Makefile
@@ -1,3 +1,7 @@
+# needs-sanitizer-support
+# only-x86_64
+# only-linux
+
 -include ../tools.mk
 
 LOG := $(TMPDIR)/log.txt
@@ -7,16 +11,10 @@ LOG := $(TMPDIR)/log.txt
 # are compiled with address sanitizer, and we assert that a fault in the dylib
 # is correctly detected.
 
-ifeq ($(TARGET),x86_64-unknown-linux-gnu)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
-
 # See comment in sanitizer-address/Makefile for why this is here
 EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
-endif
 
 all:
-ifeq ($(ASAN_SUPPORT),1)
 	$(RUSTC) -g -Z sanitizer=address --crate-type dylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
 	$(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) $(EXTRA_RUSTFLAG) -llibrary program.rs
 	LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-invalid-cratetype/Makefile b/src/test/run-make-fulldeps/sanitizer-invalid-cratetype/Makefile
@@ -1,18 +1,16 @@
+# needs-sanitizer-support
+
 -include ../tools.mk
 
 # NOTE the address sanitizer only supports x86_64 linux and macOS
 
 ifeq ($(TARGET),x86_64-apple-darwin)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
 EXTRA_RUSTFLAG=-C rpath
 else
 ifeq ($(TARGET),x86_64-unknown-linux-gnu)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
 EXTRA_RUSTFLAG=
 endif
 endif
 
 all:
-ifeq ($(ASAN_SUPPORT),1)
 	$(RUSTC) -Z sanitizer=address --crate-type proc-macro --target $(TARGET) hello.rs 2>&1 | $(CGREP) '-Z sanitizer'
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-leak/Makefile b/src/test/run-make-fulldeps/sanitizer-leak/Makefile
@@ -1,12 +1,11 @@
 -include ../tools.mk
 
+# needs-sanitizer-support
 # only-linux
 # only-x86_64
 # ignore-test
 # FIXME(#46126) ThinLTO for libstd broke this test
 
 all:
-ifdef SANITIZER_SUPPORT
 	$(RUSTC) -C opt-level=1 -g -Z sanitizer=leak -Z print-link-args leak.rs | $(CGREP) librustc_lsan
 	$(TMPDIR)/leak 2>&1 | $(CGREP) 'detected memory leaks'
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-memory/Makefile b/src/test/run-make-fulldeps/sanitizer-memory/Makefile
@@ -1,10 +1,9 @@
 -include ../tools.mk
 
+# needs-sanitizer-support
 # only-linux
 # only-x86_64
 
 all:
-ifdef SANITIZER_SUPPORT
 	$(RUSTC) -g -Z sanitizer=memory -Z print-link-args uninit.rs | $(CGREP) librustc_msan
 	$(TMPDIR)/uninit 2>&1 | $(CGREP) use-of-uninitialized-value
-endif
diff --git a/src/test/run-make-fulldeps/sanitizer-staticlib-link/Makefile b/src/test/run-make-fulldeps/sanitizer-staticlib-link/Makefile
@@ -1,18 +1,15 @@
+# needs-sanitizer-support
+# only-x86_64
+# only-linux
+
 -include ../tools.mk
 
 # This test builds a staticlib, then an executable that links to it.
-# The staticlib and executable both  are compiled with address sanitizer, 
+# The staticlib and executable both  are compiled with address sanitizer,
 # and we assert that a fault in the staticlib is correctly detected.
 
-ifeq ($(TARGET),x86_64-unknown-linux-gnu)
-ASAN_SUPPORT=$(SANITIZER_SUPPORT)
-EXTRA_RUSTFLAG=
-endif
-
 all:
-ifeq ($(ASAN_SUPPORT),1)
 	$(RUSTC) -g -Z sanitizer=address --crate-type staticlib --target $(TARGET) library.rs
 	$(CC) program.c $(call STATICLIB,library) $(call OUT_EXE,program) $(EXTRACFLAGS) $(EXTRACXXFLAGS)
 	LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
-endif
 
diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs
@@ -88,6 +88,9 @@ impl EarlyProps {
             }
         }
 
+        let rustc_has_profiler_support = env::var_os("RUSTC_PROFILER_SUPPORT").is_some();
+        let rustc_has_sanitizer_support = env::var_os("RUSTC_SANITIZER_SUPPORT").is_some();
+
         iter_header(testfile, None, &mut |ln| {
             // we should check if any only-<platform> exists and if it exists
             // and does not matches the current platform, skip the test
@@ -116,6 +119,16 @@ impl EarlyProps {
                    config.parse_needs_matching_clang(ln) {
                     props.ignore = Ignore::Ignore;
                 }
+
+                if !rustc_has_profiler_support &&
+                   config.parse_needs_profiler_support(ln) {
+                    props.ignore = Ignore::Ignore;
+                }
+
+                if !rustc_has_sanitizer_support &&
+                   config.parse_needs_sanitizer_support(ln) {
+                    props.ignore = Ignore::Ignore;
+                }
             }
 
             if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoBoth) &&
@@ -748,6 +761,14 @@ impl Config {
         self.parse_name_directive(line, "needs-matching-clang")
     }
 
+    fn parse_needs_profiler_support(&self, line: &str) -> bool {
+        self.parse_name_directive(line, "needs-profiler-support")
+    }
+
+    fn parse_needs_sanitizer_support(&self, line: &str) -> bool {
+        self.parse_name_directive(line, "needs-sanitizer-support")
+    }
+
     /// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
     /// or `normalize-stderr-32bit`.
     fn parse_cfg_name_directive(&self, line: &str, prefix: &str) -> ParsedNameDirective {

Original file line number	Diff line number	Diff line change
`@@ -1268,11 +1268,11 @@ impl Step for Compiletest {`
`1268`	`1268`	`builder.add_rust_test_threads(&mut cmd);`
`1269`	`1269`
`1270`	`1270`	`if builder.config.sanitizers {`
`1271`		`- cmd.env("SANITIZER_SUPPORT", "1");`
	`1271`	`+ cmd.env("RUSTC_SANITIZER_SUPPORT", "1");`
`1272`	`1272`	`}`
`1273`	`1273`
`1274`	`1274`	`if builder.config.profiler {`
`1275`		`- cmd.env("PROFILER_SUPPORT", "1");`
	`1275`	`+ cmd.env("RUSTC_PROFILER_SUPPORT", "1");`
`1276`	`1276`	`}`
`1277`	`1277`
`1278`	`1278`	`cmd.env("RUST_TEST_TMPDIR", builder.out.join("tmp"));`