Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. #57514

Merged
merged 10 commits into from
Jan 31, 2019
Merged
4 changes: 4 additions & 0 deletions config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@
# that your host compiler ships with libc++.
#use-libcxx = true

# The value specified here will be passed as `-DLLVM_USE_LINKER` to CMake.
#use-linker = "lld"


# =============================================================================
# General build configuration options
# =============================================================================
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub struct Config {
pub llvm_experimental_targets: String,
pub llvm_link_jobs: Option<u32>,
pub llvm_version_suffix: Option<String>,
pub llvm_use_linker: Option<String>,

pub lld_enabled: bool,
pub lldb_enabled: bool,
Expand Down Expand Up @@ -255,6 +256,7 @@ struct Llvm {
version_suffix: Option<String>,
clang_cl: Option<String>,
use_libcxx: Option<bool>,
use_linker: Option<String>,
}

#[derive(Deserialize, Default, Clone)]
Expand Down Expand Up @@ -517,6 +519,7 @@ impl Config {
config.llvm_version_suffix = llvm.version_suffix.clone();
config.llvm_clang_cl = llvm.clang_cl.clone();
set(&mut config.llvm_use_libcxx, llvm.use_libcxx);
config.llvm_use_linker = llvm.use_linker.clone();
}

if let Some(ref rust) = toml.rust {
Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ impl Step for Llvm {
cfg.define("LLVM_VERSION_SUFFIX", suffix);
}

if let Some(ref linker) = builder.config.llvm_use_linker {
cfg.define("LLVM_USE_LINKER", linker);
}

if let Some(ref python) = builder.config.python {
cfg.define("PYTHON_EXECUTABLE", python);
}
Expand Down
24 changes: 21 additions & 3 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1108,9 +1108,7 @@ impl Step for Compiletest {
};
let lldb_exe = if builder.config.lldb_enabled && !target.contains("emscripten") {
// Test against the lldb that was just built.
builder.llvm_out(target)
.join("bin")
.join("lldb")
builder.llvm_out(target).join("bin").join("lldb")
} else {
PathBuf::from("lldb")
};
Expand All @@ -1127,6 +1125,26 @@ impl Step for Compiletest {
}
}

if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
match &var.to_string_lossy().to_lowercase()[..] {
"1" | "yes" | "on" => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh since this is an internal env var I think matching on only "yes" or "1" is probably fine, we tend to not really need that much error handling and support for internal features like this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to try and make it less likely to have a typo causing things to be silently ignored. What would be really needed though would be fuzzy matching on env var names...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True yeah, although we could also just check for existence here instead of checking for value perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what wanted to do at first but then I remembered the CARGO_INCREMENTAL=0 confusion. Since it's not really complicated to support a few different options here, I just did that.

assert!(builder.config.lldb_enabled,
"RUSTBUILD_FORCE_CLANG_BASED_TESTS needs Clang/LLDB to \
be built.");
let clang_exe = builder.llvm_out(target).join("bin").join("clang");
cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
}
"0" | "no" | "off" => {
// Nothing to do.
}
other => {
// Let's make sure typos don't get unnoticed
panic!("Unrecognized option '{}' set in \
RUSTBUILD_FORCE_CLANG_BASED_TESTS", other);
}
}
}

// Get paths from cmd args
let paths = match &builder.config.cmd {
Subcommand::Test { ref paths, .. } => &paths[..],
Expand Down
41 changes: 27 additions & 14 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::fs;
use std::env;
use std::iter;
use std::path::PathBuf;
use std::process::{Command, exit};
use std::collections::HashSet;
Expand Down Expand Up @@ -666,27 +665,41 @@ impl<'a> Builder<'a> {

// Add the llvm/bin directory to PATH since it contains lots of
// useful, platform-independent tools
if tool.uses_llvm_tools() {
if tool.uses_llvm_tools() && !self.config.dry_run {
let mut additional_paths = vec![];

if let Some(llvm_bin_path) = self.llvm_bin_path() {
if host.contains("windows") {
// On Windows, PATH and the dynamic library path are the same,
// so we just add the LLVM bin path to lib_path
lib_paths.push(llvm_bin_path);
} else {
let old_path = env::var_os("PATH").unwrap_or_default();
let new_path = env::join_paths(iter::once(llvm_bin_path)
.chain(env::split_paths(&old_path)))
.expect("Could not add LLVM bin path to PATH");
cmd.env("PATH", new_path);
}
additional_paths.push(llvm_bin_path);
}

// If LLD is available, add that too.
if self.config.lld_enabled {
let lld_install_root = self.ensure(native::Lld {
target: self.config.build,
});

let lld_bin_path = lld_install_root.join("bin");
additional_paths.push(lld_bin_path);
}

if host.contains("windows") {
// On Windows, PATH and the dynamic library path are the same,
// so we just add the LLVM bin path to lib_path
lib_paths.extend(additional_paths);
} else {
let old_path = env::var_os("PATH").unwrap_or_default();
let new_path = env::join_paths(additional_paths.into_iter()
.chain(env::split_paths(&old_path)))
.expect("Could not add LLVM bin path to PATH");
cmd.env("PATH", new_path);
}
}

add_lib_path(lib_paths, cmd);
}

fn llvm_bin_path(&self) -> Option<PathBuf> {
if self.config.llvm_enabled && !self.config.dry_run {
if self.config.llvm_enabled {
let llvm_config = self.ensure(native::Llvm {
target: self.config.build,
emscripten: false,
Expand Down
27 changes: 23 additions & 4 deletions src/ci/docker/x86_64-gnu-debug/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ubuntu:16.04
FROM ubuntu:18.10

RUN apt-get update && apt-get install -y --no-install-recommends \
g++ \
Expand All @@ -7,18 +7,37 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
curl \
ca-certificates \
python2.7 \
python2.7-dev \
libxml2-dev \
libncurses-dev \
libedit-dev \
swig \
doxygen \
git \
cmake \
sudo \
gdb \
xz-utils
xz-utils \
lld \
clang

COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

ENV RUSTBUILD_FORCE_CLANG_BASED_TESTS 1
ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1

ENV RUST_CONFIGURE_ARGS \
--build=x86_64-unknown-linux-gnu \
--enable-debug \
--enable-optimize
ENV SCRIPT python2.7 ../x.py build
--enable-lld \
--enable-lldb \
--enable-optimize \
--set llvm.use-linker=lld \
--set target.x86_64-unknown-linux-gnu.linker=clang \
--set target.x86_64-unknown-linux-gnu.cc=clang \
--set target.x86_64-unknown-linux-gnu.cxx=clang++

ENV SCRIPT \
python2.7 ../x.py build && \
python2.7 ../x.py test src/test/run-make-fulldeps --test-args clang
25 changes: 25 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-clang/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# needs-matching-clang

# This test makes sure that cross-language inlining actually works by checking
# the generated machine code.

-include ../tools.mk

all: cpp-executable rust-executable

cpp-executable:
$(RUSTC) -Zcross-lang-lto=on -o $(TMPDIR)/librustlib-xlto.a -Copt-level=2 -Ccodegen-units=1 ./rustlib.rs
$(CLANG) -flto=thin -fuse-ld=lld -L $(TMPDIR) -lrustlib-xlto -o $(TMPDIR)/cmain ./cmain.c -O3
# Make sure we don't find a call instruction to the function we expect to
# always be inlined.
llvm-objdump -d $(TMPDIR)/cmain | $(CGREP) -v -e "call.*rust_always_inlined"
# As a sanity check, make sure we do find a call instruction to a
# non-inlined function
llvm-objdump -d $(TMPDIR)/cmain | $(CGREP) -e "call.*rust_never_inlined"

rust-executable:
$(CLANG) ./clib.c -flto=thin -c -o $(TMPDIR)/clib.o -O2
(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
$(RUSTC) -Zcross-lang-lto=on -L$(TMPDIR) -Copt-level=2 -Clinker=$(CLANG) -Clink-arg=-fuse-ld=lld ./main.rs -o $(TMPDIR)/rsmain
llvm-objdump -d $(TMPDIR)/rsmain | $(CGREP) -e "call.*c_never_inlined"
llvm-objdump -d $(TMPDIR)/rsmain | $(CGREP) -v -e "call.*c_always_inlined"
9 changes: 9 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-clang/clib.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <stdint.h>

uint32_t c_always_inlined() {
return 1234;
}

__attribute__((noinline)) uint32_t c_never_inlined() {
return 12345;
}
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-clang/cmain.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include <stdint.h>

// A trivial function defined in Rust, returning a constant value. This should
// always be inlined.
uint32_t rust_always_inlined();


uint32_t rust_never_inlined();

int main(int argc, char** argv) {
return rust_never_inlined() + rust_always_inlined();
}
11 changes: 11 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-clang/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#[link(name = "xyz")]
extern "C" {
fn c_always_inlined() -> u32;
fn c_never_inlined() -> u32;
}

fn main() {
unsafe {
println!("blub: {}", c_always_inlined() + c_never_inlined());
}
}
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/cross-lang-lto-clang/rustlib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![crate_type="staticlib"]

#[no_mangle]
pub extern "C" fn rust_always_inlined() -> u32 {
42
}

#[no_mangle]
#[inline(never)]
pub extern "C" fn rust_never_inlined() -> u32 {
421
}
4 changes: 4 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ pub struct Config {
/// (or, alternatively, to silently run them like regular run-pass tests).
pub force_valgrind: bool,

/// The path to the Clang executable to run Clang-based tests with. If
/// `None` then these tests will be ignored.
pub run_clang_based_tests_with: Option<String>,

/// The directory containing the tests to run
pub src_base: PathBuf,

Expand Down
9 changes: 9 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ impl EarlyProps {
if ignore_llvm(config, ln) {
props.ignore = Ignore::Ignore;
}

if config.run_clang_based_tests_with.is_none() &&
config.parse_needs_matching_clang(ln) {
props.ignore = Ignore::Ignore;
}
}

if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoBoth) &&
Expand Down Expand Up @@ -705,6 +710,10 @@ impl Config {
}
}

fn parse_needs_matching_clang(&self, line: &str) -> bool {
self.parse_name_directive(line, "needs-matching-clang")
}

/// 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 {
Expand Down
7 changes: 7 additions & 0 deletions src/tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
"force-valgrind",
"fail if Valgrind tests cannot be run under Valgrind",
)
.optopt(
"",
"run-clang-based-tests-with",
"path to Clang executable",
"PATH",
)
.optopt(
"",
"llvm-filecheck",
Expand Down Expand Up @@ -298,6 +304,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
docck_python: matches.opt_str("docck-python").unwrap(),
valgrind_path: matches.opt_str("valgrind-path"),
force_valgrind: matches.opt_present("force-valgrind"),
run_clang_based_tests_with: matches.opt_str("run-clang-based-tests-with"),
llvm_filecheck: matches.opt_str("llvm-filecheck").map(|s| PathBuf::from(&s)),
src_base,
build_base: opt_path(matches, "build-base"),
Expand Down
4 changes: 4 additions & 0 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2587,6 +2587,10 @@ impl<'test> TestCx<'test> {
cmd.env("RUSTC_LINKER", linker);
}

if let Some(ref clang) = self.config.run_clang_based_tests_with {
cmd.env("CLANG", clang);
}

// We don't want RUSTFLAGS set from the outside to interfere with
// compiler flags set in the test cases:
cmd.env_remove("RUSTFLAGS");
Expand Down