Skip to content

Commit 55dde6a

Browse files
committed
Auto merge of #125165 - Oneirical:pgo-branch-weights, r=<try>
Migrate `run-make/pgo-branch-weights` to `rmake` Part of #121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). This is a scary one and I expect things to break. Set as draft, because this isn't ready. - [x] There is this comment here, which suggests the test is excluded from the testing process due to a platform specific issue? I can't see anything here that would cause this test to not run... > // FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works // properly. Since we only have GCC on the CI ignore the test for now." EDIT: This is specific to Windows-gnu. - [x] The Makefile has this line: ``` ifneq (,$(findstring x86,$(TARGET))) COMMON_FLAGS=-Clink-args=-fuse-ld=gold ``` I honestly can't tell whether this is checking if the target IS x86, or IS NOT. EDIT: It's checking if it IS x86. - [x] I don't know why the Makefile was trying to pass an argument directly in the Makefile instead of setting that "aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc" input as a variable in the Rust program directly. I changed that, let me know if that was wrong. - [x] Trying to rewrite `cat "$(TMPDIR)/interesting.ll" | "$(LLVM_FILECHECK)" filecheck-patterns.txt` resulted in some butchery. For starters, in `tools.mk`, LLVM_FILECHECK corrects its own backslashes on Windows distributions, but there is no further mention of it, so I assume this is a preset environment variable... but is it really? Then, the command itself uses a Standard Input and a passed input file as an argument simultaneously, according to the [documentation](https://llvm.org/docs/CommandGuide/FileCheck.html#synopsis). try-job: aarch64-gnu
2 parents 3ea5e23 + 0bb84af commit 55dde6a

File tree

7 files changed

+211
-42
lines changed

7 files changed

+211
-42
lines changed

src/tools/run-make-support/src/lib.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ mod command;
99
pub mod diff;
1010
mod drop_bomb;
1111
pub mod fs_wrapper;
12-
pub mod llvm_readobj;
12+
pub mod llvm;
1313
pub mod run;
1414
pub mod rustc;
1515
pub mod rustdoc;
@@ -29,8 +29,10 @@ pub use wasmparser;
2929
pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc};
3030
pub use clang::{clang, Clang};
3131
pub use diff::{diff, Diff};
32-
pub use llvm_readobj::{llvm_readobj, LlvmReadobj};
33-
pub use run::{cmd, run, run_fail};
32+
pub use llvm::{
33+
llvm_filecheck, llvm_profdata, llvm_readobj, LlvmFilecheck, LlvmProfdata, LlvmReadobj,
34+
};
35+
pub use run::{cmd, run, run_fail, run_with_args};
3436
pub use rustc::{aux_build, rustc, Rustc};
3537
pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};
3638

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
use std::path::{Path, PathBuf};
2+
3+
use crate::{env_var, Command};
4+
5+
/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
6+
/// at `$LLVM_BIN_DIR/llvm-readobj`.
7+
pub fn llvm_readobj() -> LlvmReadobj {
8+
LlvmReadobj::new()
9+
}
10+
11+
/// Construct a new `llvm-profdata` invocation. This assumes that `llvm-profdata` is available
12+
/// at `$LLVM_BIN_DIR/llvm-profdata`.
13+
pub fn llvm_profdata() -> LlvmProfdata {
14+
LlvmProfdata::new()
15+
}
16+
17+
/// Construct a new `llvm-filecheck` invocation. This assumes that `llvm-filecheck` is available
18+
/// at `$LLVM_FILECHECK`.
19+
pub fn llvm_filecheck() -> LlvmFilecheck {
20+
LlvmFilecheck::new()
21+
}
22+
23+
/// A `llvm-readobj` invocation builder.
24+
#[derive(Debug)]
25+
pub struct LlvmReadobj {
26+
cmd: Command,
27+
}
28+
29+
/// A `llvm-profdata` invocation builder.
30+
#[derive(Debug)]
31+
pub struct LlvmProfdata {
32+
cmd: Command,
33+
}
34+
35+
/// A `llvm-filecheck` invocation builder.
36+
#[derive(Debug)]
37+
pub struct LlvmFilecheck {
38+
cmd: Command,
39+
}
40+
41+
crate::impl_common_helpers!(LlvmReadobj);
42+
crate::impl_common_helpers!(LlvmProfdata);
43+
crate::impl_common_helpers!(LlvmFilecheck);
44+
45+
/// Generate the path to the bin directory of LLVM.
46+
pub fn llvm_bin_dir() -> PathBuf {
47+
let llvm_bin_dir = env_var("LLVM_BIN_DIR");
48+
PathBuf::from(llvm_bin_dir)
49+
}
50+
51+
impl LlvmReadobj {
52+
/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
53+
/// at `$LLVM_BIN_DIR/llvm-readobj`.
54+
pub fn new() -> Self {
55+
let llvm_readobj = llvm_bin_dir().join("llvm-readobj");
56+
let cmd = Command::new(llvm_readobj);
57+
Self { cmd }
58+
}
59+
60+
/// Provide an input file.
61+
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
62+
self.cmd.arg(path.as_ref());
63+
self
64+
}
65+
66+
/// Pass `--file-header` to display file headers.
67+
pub fn file_header(&mut self) -> &mut Self {
68+
self.cmd.arg("--file-header");
69+
self
70+
}
71+
}
72+
73+
impl LlvmProfdata {
74+
/// Construct a new `llvm-profdata` invocation. This assumes that `llvm-profdata` is available
75+
/// at `$LLVM_BIN_DIR/llvm-profdata`.
76+
pub fn new() -> Self {
77+
let llvm_profdata = llvm_bin_dir().join("llvm-profdata");
78+
let cmd = Command::new(llvm_profdata);
79+
Self { cmd }
80+
}
81+
82+
/// Provide an input file.
83+
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
84+
self.cmd.arg(path.as_ref());
85+
self
86+
}
87+
88+
/// Specify the output file path.
89+
pub fn output<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
90+
self.cmd.arg("-o");
91+
self.cmd.arg(path.as_ref());
92+
self
93+
}
94+
95+
/// Take several profile data files generated by PGO instrumentation and merge them
96+
/// together into a single indexed profile data file.
97+
pub fn merge(&mut self) -> &mut Self {
98+
self.cmd.arg("merge");
99+
self
100+
}
101+
}
102+
103+
impl LlvmFilecheck {
104+
/// Construct a new `llvm-filecheck` invocation. This assumes that `llvm-filecheck` is available
105+
/// at `$LLVM_FILECHECK`.
106+
pub fn new() -> Self {
107+
let llvm_filecheck = env_var("LLVM_FILECHECK");
108+
let cmd = Command::new(llvm_filecheck);
109+
Self { cmd }
110+
}
111+
112+
/// Pipe a read file into standard input containing patterns that will be matched against the .patterns(path) call.
113+
pub fn stdin<I: AsRef<[u8]>>(&mut self, input: I) -> &mut Self {
114+
self.cmd.set_stdin(input.as_ref().to_vec().into_boxed_slice());
115+
self
116+
}
117+
118+
/// Provide the patterns that need to be matched.
119+
pub fn patterns<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
120+
self.cmd.arg(path.as_ref());
121+
self
122+
}
123+
}

src/tools/run-make-support/src/run.rs

+20-3
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@ use crate::{cwd, env_var, is_windows, set_host_rpath};
99
use super::handle_failed_output;
1010

1111
#[track_caller]
12-
fn run_common(name: &str) -> Command {
12+
fn run_common(name: &str, args: Option<&[&str]>) -> Command {
1313
let mut bin_path = PathBuf::new();
1414
bin_path.push(cwd());
1515
bin_path.push(name);
1616
let ld_lib_path_envvar = env_var("LD_LIB_PATH_ENVVAR");
1717
let mut cmd = Command::new(bin_path);
18+
if let Some(args) = args {
19+
for arg in args {
20+
cmd.arg(arg);
21+
}
22+
}
1823
cmd.env(&ld_lib_path_envvar, {
1924
let mut paths = vec![];
2025
paths.push(cwd());
@@ -43,7 +48,19 @@ fn run_common(name: &str) -> Command {
4348
#[track_caller]
4449
pub fn run(name: &str) -> CompletedProcess {
4550
let caller = panic::Location::caller();
46-
let mut cmd = run_common(name);
51+
let mut cmd = run_common(name, None);
52+
let output = cmd.run();
53+
if !output.status().success() {
54+
handle_failed_output(&cmd, output, caller.line());
55+
}
56+
output
57+
}
58+
59+
/// Run a built binary with one or more argument(s) and make sure it succeeds.
60+
#[track_caller]
61+
pub fn run_with_args(name: &str, args: &[&str]) -> CompletedProcess {
62+
let caller = panic::Location::caller();
63+
let mut cmd = run_common(name, Some(args));
4764
let output = cmd.run();
4865
if !output.status().success() {
4966
handle_failed_output(&cmd, output, caller.line());
@@ -55,7 +72,7 @@ pub fn run(name: &str) -> CompletedProcess {
5572
#[track_caller]
5673
pub fn run_fail(name: &str) -> CompletedProcess {
5774
let caller = panic::Location::caller();
58-
let mut cmd = run_common(name);
75+
let mut cmd = run_common(name, None);
5976
let output = cmd.run_fail();
6077
if output.status().success() {
6178
handle_failed_output(&cmd, output, caller.line());

src/tools/run-make-support/src/rustc.rs

+18
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,24 @@ impl Rustc {
146146
self
147147
}
148148

149+
/// Specify directory path used for profile generation
150+
pub fn profile_generate<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
151+
let mut arg = OsString::new();
152+
arg.push("-Cprofile-generate=");
153+
arg.push(path.as_ref());
154+
self.cmd.arg(&arg);
155+
self
156+
}
157+
158+
/// Specify directory path used for profile usage
159+
pub fn profile_use<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
160+
let mut arg = OsString::new();
161+
arg.push("-Cprofile-use=");
162+
arg.push(path.as_ref());
163+
self.cmd.arg(&arg);
164+
self
165+
}
166+
149167
/// Specify error format to use
150168
pub fn error_format(&mut self, format: &str) -> &mut Self {
151169
self.cmd.arg(format!("--error-format={format}"));

src/tools/tidy/src/allowed_run_make_makefiles.txt

-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ run-make/pass-linker-flags/Makefile
168168
run-make/pass-non-c-like-enum-to-c/Makefile
169169
run-make/pdb-alt-path/Makefile
170170
run-make/pdb-buildinfo-cl-cmd/Makefile
171-
run-make/pgo-branch-weights/Makefile
172171
run-make/pgo-gen-lto/Makefile
173172
run-make/pgo-gen-no-imp-symbols/Makefile
174173
run-make/pgo-gen/Makefile

tests/run-make/pgo-branch-weights/Makefile

-35
This file was deleted.
+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// This test generates an instrumented binary - a program which
2+
// will keep track of how many times it calls each function, a useful
3+
// feature for optimization. Then, an argument (aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc)
4+
// is passed into the instrumented binary, which should react with a number of function calls
5+
// fully known in advance. (For example, the letter 'a' results in calling f1())
6+
7+
// If the test passes, the expected function call count was added to the use-phase LLVM-IR.
8+
// See https://github.com/rust-lang/rust/pull/66631
9+
10+
//@ needs-profiler-support
11+
//@ ignore-cross-compile
12+
13+
// (This test has problems generating profdata on mingw. This could use further investigation.)
14+
//@ ignore-windows-gnu
15+
16+
use run_make_support::{fs_wrapper, llvm_filecheck, llvm_profdata, run_with_args, rustc};
17+
use std::fs;
18+
use std::path::Path;
19+
20+
fn main() {
21+
let path_prof_data_dir = Path::new("prof_data_dir");
22+
let path_merged_profdata = path_prof_data_dir.join("merged.profdata");
23+
rustc().input("opaque.rs").run();
24+
fs_wrapper::create_dir_all(&path_prof_data_dir);
25+
rustc()
26+
.input("interesting.rs")
27+
.profile_generate(&path_prof_data_dir)
28+
.opt()
29+
.codegen_units(1)
30+
.run();
31+
rustc().input("main.rs").profile_generate(&path_prof_data_dir).opt().run();
32+
run_with_args("main", &["aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc"]);
33+
llvm_profdata().merge().output(&path_merged_profdata).input(path_prof_data_dir).run();
34+
rustc()
35+
.input("interesting.rs")
36+
.profile_use(path_merged_profdata)
37+
.opt()
38+
.codegen_units(1)
39+
.emit("llvm-ir")
40+
.run();
41+
llvm_filecheck()
42+
.patterns("filecheck-patterns.txt")
43+
.stdin(fs_wrapper::read("interesting.ll"))
44+
.run();
45+
}

0 commit comments

Comments
 (0)