Build ykllvm in target/<debug|release>/ykllvm.

Previously we tried building ykllvm in `OUT_DIR` but, unfortunately,
that means when testing we can't access the Cargo variable we defined in
rust-lang/cargo#10927. Previously, things
accidentally worked because we also required PATH to be set to
`/path/to/ykllvm/bin`, but that means you could pick up unexpected
versions of clang/clang++, for example.

This commit rethinks things. It builds a copy of ykllvm in
`target/<debug|release>/ykllvm`. [Technically this breaks Cargo's rules,
but since we're having to worm around a Cargo restriction, I think we
can deal with this: if Cargo's rules change in the future, we can change
the location easily enough.] `yk-config` gains `--cc` and `--cxx` flags
which give the user the path to the C/C++ compiler.

I've tried to keep this commit as minimal as possible: it would
certainly be possible to do various bits of cleaning-up at the same
time, but I think that would obscure what we're most interested in.

.buildbot.sh

@@ -16,14 +16,6 @@ export PATH=${CARGO_HOME}/bin/:$PATH
 
 rustup toolchain install nightly --allow-downgrade --component rustfmt
 
-# There are some feature-gated testing/debugging switches which slow the JIT
-# down a bit. Check that if we build the system without tests, those features
-# are not enabled.
-for mode in "" "--release"; do \
-    cargo -Z unstable-options build ${mode} --build-plan -p ykcapi | \
-        awk '/yk_testing/ { ec=1 } /yk_jitstate_debug/ { ec=1 } END {exit ec}'; \
-done
-
 cargo fmt --all -- --check
 
 # Check licenses.
@@ -37,11 +29,13 @@ mdbook build
 test -d book
 cd ..
 
-# Build LLVM for the C tests.
-cd ykllvm
-mkdir build
-cd build
-
+# We could let yk build two copies of LLVM, but we also want to: check that
+# YKB_YKLLVM_BIN_DIR works; and we want access to clang-format from a build
+# of LLVM. So we first build our own LLVM-with-assertions, use the
+# YKB_YKLLVM_BIN_DIR variable to have yk use that, and use its
+# clang-format.
+mkdir -p ykllvm/build
+cd ykllvm/build
 # Due to an LLVM bug, PIE breaks our mapper, and it's not enough to pass
 # `-fno-pie` to clang for some reason:
 # https://github.com/llvm/llvm-project/issues/57085
@@ -56,33 +50,47 @@ cmake -DCMAKE_INSTALL_PREFIX=`pwd`/../inst \
     ../llvm
 cmake --build .
 cmake --install .
-export PATH=`pwd`/../inst/bin:${PATH}
+export YKB_YKLLVM_BIN_DIR=`pwd`/../inst/bin
 cd ../../
 
 # Check that clang-format is installed.
-clang-format --version
+PATH=${YKB_YKLLVM_BIN_DIR}:${PATH} clang-format --version
 # Check C/C++ formatting using xtask.
-cargo xtask cfmt
-
+PATH=${YKB_YKLLVM_BIN_DIR}:${PATH} cargo xtask cfmt
 # This is used to check clang-tidy output, but the dirty submodule from building
 # ykllvm is also shown.
 # FIXME: Add build/ to .gitignore in ykllvm
 git diff --exit-code --ignore-submodules
 
-# Check that building `ykcapi` in isolation works. This is what we'd be doing
-# if we were building release binaries, as it would mean we get a system
-# without the (slower) `yk_testing` and `yk_jitstate_debug` features enabled.
-for mode in "" "--release"; do
-    cargo build ${mode} -p ykcapi;
-done
+# There are some feature-gated testing/debugging switches which slow the JIT
+# down a bit. Check that if we build the system without tests, those features
+# are not enabled.
+cargo -Z unstable-options build --build-plan -p ykcapi | \
+    awk '/yk_testing/ { ec=1 } /yk_jitstate_debug/ { ec=1 } END {exit ec}'
 
 for i in $(seq 10); do
     cargo test
-    cargo test --release
 done
 
-cargo bench
-
 # Run examples.
 cargo run --example hwtracer_example
+
+
+# We now want to test building with `--release`, which we also take as an
+# opportunity to check that yk can build ykllvm, which requires unsetting
+# YKB_YKLLVM_BIN_DIR. In essence, we now repeat much of what we did above but
+# with `--release`.
+unset YKB_YKLLVM_BIN_DIR
+
+cargo -Z unstable-options build --release --build-plan -p ykcapi | \
+    awk '/yk_testing/ { ec=1 } /yk_jitstate_debug/ { ec=1 } END {exit ec}'
+
+cargo build --release -p ykcapi
+
+for i in $(seq 10); do
+    cargo test --release
+done
+
 cargo run --release --example hwtracer_example
+
+cargo bench

docs/src/dev/env.md

@@ -17,12 +17,19 @@ build *without* the feature enabled, do `cargo build -p ykcapi`).
 
 ### `YKB_YKLLVM_BIN_DIR`
 
-Under normal circumstances, yk builds a copy of its LLVM fork "ykllvm" and uses
-it to build interpreters. You can use your own ykllvm build by specifying the
-directory where the executables (e.g. `clang`, `llvm-config`, and so on) are
-stored with `YKB_YKLLVM_BIN_DIR`. yk does not check your installation for
-compatibility: it is your responsibility to ensure that your ykllvm build
-matches that expected by yk.
+Under normal circumstances, yk builds a copy of its LLVM fork "ykllvm", which
+it also uses it to build interpreters (via the compiler's use of `yk-config`).
+You can use your own ykllvm build by specifying the directory where the
+executables (e.g. `clang`, `llvm-config`, and so on) are stored with
+`YKB_YKLLVM_BIN_DIR`.
+
+yk does not check your installation for compatibility: it is your
+responsibility to ensure that your ykllvm build matches that expected by yk.
+
+It is also undefined behaviour to move between defining this variable and not
+within a repository using `yk` (including the `yk` repository itself). If you
+want to set/unset `YKB_YKLLVM_BIN_DIR` then `cargo clean` any repositories
+using `yk` before rebuilding them.
 
 
 ## Run-time Variables

hwtracer/build.rs

@@ -7,7 +7,10 @@ use std::fs;
 use std::os::unix::fs as unix_fs;
 use std::path::{Path, PathBuf};
 use std::process::Command;
-use ykbuild::ccgen::{CCGenerator, CCLang};
+use ykbuild::{
+    ccgen::{CCGenerator, CCLang},
+    ykllvm_bin,
+};
 
 const FEATURE_CHECKS_PATH: &str = "feature_checks";
 
@@ -101,6 +104,7 @@ fn main() {
     // Generate a `compile_commands.json` database for clangd.
     let ccg = CCGenerator::new("hwtracer", &env::var("CARGO_MANIFEST_DIR").unwrap());
     env::set_var.call(ccg.build_env());
+    env::set_var("YK_COMPILER_PATH", ykllvm_bin("clang"));
     c_build.compiler(CCLang::C.compiler_wrapper());
 
     let c_deps_dir = make_c_deps_dir();

tests/benches/bench.rs

@@ -13,6 +13,7 @@ use std::{
 };
 use tempfile::TempDir;
 use tests::mk_compiler;
+use ykbuild::ykllvm_bin;
 
 const SAMPLE_SIZE: usize = 50;
 const MEASUREMENT_TIME: Duration = Duration::from_secs(30);
@@ -27,7 +28,7 @@ fn compile_runner(tempdir: &TempDir) -> PathBuf {
     exe.push(tempdir);
     exe.push(src.file_stem().unwrap());
 
-    let mut compiler = mk_compiler("clang", &exe, &src, "-O0", &[], false);
+    let mut compiler = mk_compiler(ykllvm_bin("clang"), &exe, &src, "-O0", &[], false);
     compiler.arg("-ltests");
     let out = compiler.output().unwrap();
     check_output(&out);

tests/langtest_c.rs

@@ -11,7 +11,10 @@ use std::{
 };
 use tempfile::TempDir;
 use tests::{mk_compiler, EXTRA_LINK};
-use ykbuild::ccgen::{CCGenerator, CCLang};
+use ykbuild::{
+    ccgen::{CCGenerator, CCLang},
+    ykllvm_bin,
+};
 
 const COMMENT: &str = "//";
 
@@ -50,6 +53,7 @@ fn run_suite(opt: &'static str, force_decoder: &'static str) {
     // Generate a `compile_commands.json` database for clangd.
     let ccg = CCGenerator::new("c_tests", &env::var("CARGO_MANIFEST_DIR").unwrap());
     env::set_var.call(ccg.build_env());
+    env::set_var("YK_COMPILER_PATH", ykllvm_bin("clang"));
 
     LangTester::new()
         .test_dir("c")
@@ -80,14 +84,15 @@ fn run_suite(opt: &'static str, force_decoder: &'static str) {
                 .map(|l| l.generate_obj(tempdir.path()))
                 .collect::<Vec<PathBuf>>();
 
-            let compiler = mk_compiler(
-                CCLang::C.compiler_wrapper().to_str().unwrap(),
+            let mut compiler = mk_compiler(
+                CCLang::C.compiler_wrapper().as_path(),
                 &exe,
                 p,
                 opt,
                 &extra_objs,
                 true,
             );
+            compiler.env("YK_COMPILER_PATH", ykllvm_bin("clang"));
             let mut runtime = Command::new(exe.clone());
             runtime.env("YKD_FORCE_TRACE_DECODER", force_decoder);
             vec![("Compiler", compiler), ("Run-time", runtime)]

tests/langtest_hwtracer_ykpt.rs

@@ -12,6 +12,7 @@ use std::{
 use tempfile::TempDir;
 use tests::mk_compiler;
 use tests::ExtraLinkage;
+use ykbuild::ykllvm_bin;
 
 const COMMENT: &str = "//";
 
@@ -24,8 +25,8 @@ pub static EXTRA_LINK_HWTRACER_YKPT: LazyLock<HashMap<&'static str, Vec<ExtraLin
             "foreign.c",
             vec![ExtraLinkage::new(
                 "%%TEMPDIR%%/call_me.o",
+                ykllvm_bin("clang").to_owned(),
                 &[
-                    "clang",
                     "-c",
                     "-O0",
                     "extra_linkage/call_me.c",
@@ -38,8 +39,8 @@ pub static EXTRA_LINK_HWTRACER_YKPT: LazyLock<HashMap<&'static str, Vec<ExtraLin
             "stifle_compressed_ret.c",
             vec![ExtraLinkage::new(
                 "%%TEMPDIR%%/fudge.o",
+                ykllvm_bin("clang").to_owned(),
                 &[
-                    "clang",
                     "-c",
                     "-O0",
                     "extra_linkage/fudge.s",
@@ -93,7 +94,7 @@ fn run_suite(opt: &'static str) {
                 .map(|l| l.generate_obj(tempdir.path()))
                 .collect::<Vec<PathBuf>>();
 
-            let mut compiler = mk_compiler("clang", &exe, p, opt, &extra_objs, false);
+            let mut compiler = mk_compiler(ykllvm_bin("clang"), &exe, p, opt, &extra_objs, false);
             compiler.arg("-ltests");
             let runtime = Command::new(exe.clone());
             vec![("Compiler", compiler), ("Run-time", runtime)]

tests/src/bin/gdb_c_test.rs

@@ -4,6 +4,7 @@ use clap::Parser;
 use std::{env, path::PathBuf, process::Command};
 use tempfile::TempDir;
 use tests::{mk_compiler, EXTRA_LINK};
+use ykbuild::ykllvm_bin;
 
 /// Run a C test under gdb.
 #[derive(Parser, Debug)]
@@ -54,7 +55,14 @@ fn main() {
 
     let binstem = PathBuf::from(args.test_file.file_stem().unwrap());
     let binpath = [tempdir.path(), &binstem].iter().collect::<PathBuf>();
-    let mut cmd = mk_compiler("clang", &binpath, &test_path, "-O0", &extra_objs, true);
+    let mut cmd = mk_compiler(
+        ykllvm_bin("clang").as_path(),
+        &binpath,
+        &test_path,
+        "-O0",
+        &extra_objs,
+        true,
+    );
     if !cmd.spawn().unwrap().wait().unwrap().success() {
         panic!("compilation failed");
     }

tests/src/lib.rs

@@ -10,6 +10,7 @@ use std::{
     process::Command,
     sync::LazyLock,
 };
+use ykbuild::ykllvm_bin;
 
 const TEMPDIR_SUBST: &str = "%%TEMPDIR%%";
 pub static EXTRA_LINK: LazyLock<HashMap<&'static str, Vec<ExtraLinkage>>> = LazyLock::new(|| {
@@ -30,8 +31,8 @@ pub static EXTRA_LINK: LazyLock<HashMap<&'static str, Vec<ExtraLinkage>>> = Lazy
             *test_file,
             vec![ExtraLinkage::new(
                 "%%TEMPDIR%%/call_me.o",
+                ykllvm_bin("clang").to_owned(),
                 &[
-                    "clang",
                     "-I../ykcapi",
                     "-c",
                     "-O0",
@@ -49,27 +50,33 @@ pub static EXTRA_LINK: LazyLock<HashMap<&'static str, Vec<ExtraLinkage>>> = Lazy
 pub struct ExtraLinkage<'a> {
     /// The name of the object file to be generated.
     output_file: &'a str,
-    /// The command that generates the object file.
-    gen_cmd: &'a [&'a str],
+    /// The path to the binary we want to run.
+    gen_bin: PathBuf,
+    /// Arguments to the binary.
+    gen_args: &'a [&'a str],
 }
 
 impl<'a> ExtraLinkage<'a> {
-    pub fn new(output_file: &'a str, gen_cmd: &'a [&'a str]) -> Self {
+    pub fn new(output_file: &'a str, gen_bin: PathBuf, gen_args: &'a [&'a str]) -> Self {
         Self {
             output_file,
-            gen_cmd,
+            gen_bin,
+            gen_args,
         }
     }
 
     /// Run the command to generate the object in `tempdir` and return the absolute path to the
     /// generated object.
     pub fn generate_obj(&self, tempdir: &Path) -> PathBuf {
-        let mut cmd = Command::new(self.gen_cmd[0]);
+        let mut cmd = Command::new(&self.gen_bin);
         let tempdir_s = tempdir.to_str().unwrap();
-        for arg in self.gen_cmd[1..].iter() {
+        for arg in self.gen_args.iter() {
             cmd.arg(arg.replace(TEMPDIR_SUBST, tempdir_s));
         }
-        let out = cmd.output().unwrap();
+        let out = match cmd.output() {
+            Ok(x) => x,
+            Err(e) => panic!("Error when running {:?} {:?}", cmd, e),
+        };
         assert!(tempdir.exists());
         if !out.status.success() {
             io::stdout().write_all(&out.stdout).unwrap();
@@ -87,7 +94,7 @@ impl<'a> ExtraLinkage<'a> {
 ///
 /// If `patch_cp` is `false` then the argument to patch the control point is omitted.
 pub fn mk_compiler(
-    compiler: &str,
+    compiler: &Path,
     exe: &Path,
     src: &Path,
     opt: &str,
@@ -117,6 +124,7 @@ pub fn mk_compiler(
         .output()
         .expect("failed to execute yk-config");
     if !yk_config_out.status.success() {
+        io::stderr().write_all(&yk_config_out.stderr).ok();
         panic!("yk-config exited with non-zero status");
     }
     let mut yk_flags = String::from_utf8(yk_config_out.stdout).unwrap();

ykbuild/build.rs

@@ -1,23 +1,42 @@
-use std::env;
+use std::{env, fs::create_dir_all, path::Path};
 use which::which;
 
-const YKLLVM: &str = "../ykllvm/llvm";
-const PROFILE: &str = "Release";
+/// Where is ykllvm's source code relative to this crate?
+const YKLLVM_SUBMODULE_PATH: &str = "../ykllvm/llvm";
 
 fn main() {
+    // If the user defines YKB_YKLLVM_BIN_DIR then we don't try to build ykllvm ourselves.
     if env::var("YKB_YKLLVM_BIN_DIR").is_ok() {
-        println!("cargo:ykllvm={}", env::var("YKB_YKLLVM_BIN_DIR").unwrap());
         return;
     }
 
+    // Build ykllvm in "target/[debug|release]". Note that the directory used here *must*
+    // be exactly the same as that produced by `ykbuild/src/lib.rs:llvm_bin_dir` and
+    // yk-config.
+    let mut ykllvm_dir = Path::new(&env::var("OUT_DIR").unwrap())
+        .parent()
+        .unwrap()
+        .parent()
+        .unwrap()
+        .parent()
+        .unwrap()
+        .to_owned();
+    {
+        let leaf = ykllvm_dir.file_name().unwrap().to_str().unwrap();
+        assert!(leaf == "debug" || leaf == "release");
+    }
+    ykllvm_dir.push("ykllvm");
+    create_dir_all(&ykllvm_dir).unwrap();
+
     // Ninja builds are faster, so use this if it's available.
     let use_ninja = which("ninja").is_ok();
     let is_debug = env::var("PROFILE").unwrap() == "debug";
     let nprocs = format!("-j {}", num_cpus::get());
 
-    let mut ykllvm = cmake::Config::new(YKLLVM);
+    let mut ykllvm = cmake::Config::new(YKLLVM_SUBMODULE_PATH);
     ykllvm
-        .profile(PROFILE)
+        .profile("Release")
+        .out_dir(ykllvm_dir)
         .generator(if use_ninja { "Ninja" } else { "Unix Makefiles" })
         .define("LLVM_INSTALL_UTILS", "ON")
         .define("BUILD_SHARED_LIBS", "ON")
@@ -52,10 +71,5 @@ fn main() {
         }
     }
 
-    let dsp = ykllvm.build();
-
-    // We need to be able to locate the ykllvm install llvm bins from other
-    // crates, so this sets a `DEP_YKBUILD_YKLLVM` env var which can be accessed
-    // from any other crate in the yk workspace.
-    println!("cargo:ykllvm={}/bin/", dsp.display())
+    ykllvm.build();
 }