Skip to content

Commit

Permalink
Auto merge of #125165 - Oneirical:pgo-branch-weights, r=<try>
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Jun 11, 2024
2 parents 3ea5e23 + 0bb84af commit 55dde6a
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 42 deletions.
8 changes: 5 additions & 3 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod command;
pub mod diff;
mod drop_bomb;
pub mod fs_wrapper;
pub mod llvm_readobj;
pub mod llvm;
pub mod run;
pub mod rustc;
pub mod rustdoc;
Expand All @@ -29,8 +29,10 @@ pub use wasmparser;
pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc};
pub use clang::{clang, Clang};
pub use diff::{diff, Diff};
pub use llvm_readobj::{llvm_readobj, LlvmReadobj};
pub use run::{cmd, run, run_fail};
pub use llvm::{
llvm_filecheck, llvm_profdata, llvm_readobj, LlvmFilecheck, LlvmProfdata, LlvmReadobj,
};
pub use run::{cmd, run, run_fail, run_with_args};
pub use rustc::{aux_build, rustc, Rustc};
pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};

Expand Down
123 changes: 123 additions & 0 deletions src/tools/run-make-support/src/llvm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
use std::path::{Path, PathBuf};

use crate::{env_var, Command};

/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
/// at `$LLVM_BIN_DIR/llvm-readobj`.
pub fn llvm_readobj() -> LlvmReadobj {
LlvmReadobj::new()
}

/// Construct a new `llvm-profdata` invocation. This assumes that `llvm-profdata` is available
/// at `$LLVM_BIN_DIR/llvm-profdata`.
pub fn llvm_profdata() -> LlvmProfdata {
LlvmProfdata::new()
}

/// Construct a new `llvm-filecheck` invocation. This assumes that `llvm-filecheck` is available
/// at `$LLVM_FILECHECK`.
pub fn llvm_filecheck() -> LlvmFilecheck {
LlvmFilecheck::new()
}

/// A `llvm-readobj` invocation builder.
#[derive(Debug)]
pub struct LlvmReadobj {
cmd: Command,
}

/// A `llvm-profdata` invocation builder.
#[derive(Debug)]
pub struct LlvmProfdata {
cmd: Command,
}

/// A `llvm-filecheck` invocation builder.
#[derive(Debug)]
pub struct LlvmFilecheck {
cmd: Command,
}

crate::impl_common_helpers!(LlvmReadobj);
crate::impl_common_helpers!(LlvmProfdata);
crate::impl_common_helpers!(LlvmFilecheck);

/// Generate the path to the bin directory of LLVM.
pub fn llvm_bin_dir() -> PathBuf {
let llvm_bin_dir = env_var("LLVM_BIN_DIR");
PathBuf::from(llvm_bin_dir)
}

impl LlvmReadobj {
/// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available
/// at `$LLVM_BIN_DIR/llvm-readobj`.
pub fn new() -> Self {
let llvm_readobj = llvm_bin_dir().join("llvm-readobj");
let cmd = Command::new(llvm_readobj);
Self { cmd }
}

/// Provide an input file.
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
self
}

/// Pass `--file-header` to display file headers.
pub fn file_header(&mut self) -> &mut Self {
self.cmd.arg("--file-header");
self
}
}

impl LlvmProfdata {
/// Construct a new `llvm-profdata` invocation. This assumes that `llvm-profdata` is available
/// at `$LLVM_BIN_DIR/llvm-profdata`.
pub fn new() -> Self {
let llvm_profdata = llvm_bin_dir().join("llvm-profdata");
let cmd = Command::new(llvm_profdata);
Self { cmd }
}

/// Provide an input file.
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
self
}

/// Specify the output file path.
pub fn output<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg("-o");
self.cmd.arg(path.as_ref());
self
}

/// Take several profile data files generated by PGO instrumentation and merge them
/// together into a single indexed profile data file.
pub fn merge(&mut self) -> &mut Self {
self.cmd.arg("merge");
self
}
}

impl LlvmFilecheck {
/// Construct a new `llvm-filecheck` invocation. This assumes that `llvm-filecheck` is available
/// at `$LLVM_FILECHECK`.
pub fn new() -> Self {
let llvm_filecheck = env_var("LLVM_FILECHECK");
let cmd = Command::new(llvm_filecheck);
Self { cmd }
}

/// Pipe a read file into standard input containing patterns that will be matched against the .patterns(path) call.
pub fn stdin<I: AsRef<[u8]>>(&mut self, input: I) -> &mut Self {
self.cmd.set_stdin(input.as_ref().to_vec().into_boxed_slice());
self
}

/// Provide the patterns that need to be matched.
pub fn patterns<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
self
}
}
23 changes: 20 additions & 3 deletions src/tools/run-make-support/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ use crate::{cwd, env_var, is_windows, set_host_rpath};
use super::handle_failed_output;

#[track_caller]
fn run_common(name: &str) -> Command {
fn run_common(name: &str, args: Option<&[&str]>) -> Command {
let mut bin_path = PathBuf::new();
bin_path.push(cwd());
bin_path.push(name);
let ld_lib_path_envvar = env_var("LD_LIB_PATH_ENVVAR");
let mut cmd = Command::new(bin_path);
if let Some(args) = args {
for arg in args {
cmd.arg(arg);
}
}
cmd.env(&ld_lib_path_envvar, {
let mut paths = vec![];
paths.push(cwd());
Expand Down Expand Up @@ -43,7 +48,19 @@ fn run_common(name: &str) -> Command {
#[track_caller]
pub fn run(name: &str) -> CompletedProcess {
let caller = panic::Location::caller();
let mut cmd = run_common(name);
let mut cmd = run_common(name, None);
let output = cmd.run();
if !output.status().success() {
handle_failed_output(&cmd, output, caller.line());
}
output
}

/// Run a built binary with one or more argument(s) and make sure it succeeds.
#[track_caller]
pub fn run_with_args(name: &str, args: &[&str]) -> CompletedProcess {
let caller = panic::Location::caller();
let mut cmd = run_common(name, Some(args));
let output = cmd.run();
if !output.status().success() {
handle_failed_output(&cmd, output, caller.line());
Expand All @@ -55,7 +72,7 @@ pub fn run(name: &str) -> CompletedProcess {
#[track_caller]
pub fn run_fail(name: &str) -> CompletedProcess {
let caller = panic::Location::caller();
let mut cmd = run_common(name);
let mut cmd = run_common(name, None);
let output = cmd.run_fail();
if output.status().success() {
handle_failed_output(&cmd, output, caller.line());
Expand Down
18 changes: 18 additions & 0 deletions src/tools/run-make-support/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,24 @@ impl Rustc {
self
}

/// Specify directory path used for profile generation
pub fn profile_generate<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
let mut arg = OsString::new();
arg.push("-Cprofile-generate=");
arg.push(path.as_ref());
self.cmd.arg(&arg);
self
}

/// Specify directory path used for profile usage
pub fn profile_use<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
let mut arg = OsString::new();
arg.push("-Cprofile-use=");
arg.push(path.as_ref());
self.cmd.arg(&arg);
self
}

/// Specify error format to use
pub fn error_format(&mut self, format: &str) -> &mut Self {
self.cmd.arg(format!("--error-format={format}"));
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/allowed_run_make_makefiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ run-make/pass-linker-flags/Makefile
run-make/pass-non-c-like-enum-to-c/Makefile
run-make/pdb-alt-path/Makefile
run-make/pdb-buildinfo-cl-cmd/Makefile
run-make/pgo-branch-weights/Makefile
run-make/pgo-gen-lto/Makefile
run-make/pgo-gen-no-imp-symbols/Makefile
run-make/pgo-gen/Makefile
Expand Down
35 changes: 0 additions & 35 deletions tests/run-make/pgo-branch-weights/Makefile

This file was deleted.

45 changes: 45 additions & 0 deletions tests/run-make/pgo-branch-weights/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// This test generates an instrumented binary - a program which
// will keep track of how many times it calls each function, a useful
// feature for optimization. Then, an argument (aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc)
// is passed into the instrumented binary, which should react with a number of function calls
// fully known in advance. (For example, the letter 'a' results in calling f1())

// If the test passes, the expected function call count was added to the use-phase LLVM-IR.
// See https://github.com/rust-lang/rust/pull/66631

//@ needs-profiler-support
//@ ignore-cross-compile

// (This test has problems generating profdata on mingw. This could use further investigation.)
//@ ignore-windows-gnu

use run_make_support::{fs_wrapper, llvm_filecheck, llvm_profdata, run_with_args, rustc};
use std::fs;
use std::path::Path;

fn main() {
let path_prof_data_dir = Path::new("prof_data_dir");
let path_merged_profdata = path_prof_data_dir.join("merged.profdata");
rustc().input("opaque.rs").run();
fs_wrapper::create_dir_all(&path_prof_data_dir);
rustc()
.input("interesting.rs")
.profile_generate(&path_prof_data_dir)
.opt()
.codegen_units(1)
.run();
rustc().input("main.rs").profile_generate(&path_prof_data_dir).opt().run();
run_with_args("main", &["aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc"]);
llvm_profdata().merge().output(&path_merged_profdata).input(path_prof_data_dir).run();
rustc()
.input("interesting.rs")
.profile_use(path_merged_profdata)
.opt()
.codegen_units(1)
.emit("llvm-ir")
.run();
llvm_filecheck()
.patterns("filecheck-patterns.txt")
.stdin(fs_wrapper::read("interesting.ll"))
.run();
}

0 comments on commit 55dde6a

Please sign in to comment.