Skip to content

Commit

Permalink
Revamp codegen tests to check IR quality instead of quantity
Browse files Browse the repository at this point in the history
The current codegen tests only compare IR line counts between similar
rust and C programs, the latter getting compiled with clang. That looked
like a good idea back then, but actually things like lifetime intrinsics
mean that less IR isn't always better, so the metric isn't really
helpful.

Instead, we can start doing tests that check specific aspects of the
generated IR, like attributes or metadata. To do that, we can use LLVM's
FileCheck tool which has a number of useful features for such tests.

To start off, I created some tests for a few things that were recently
added and/or broken.
  • Loading branch information
dotdash committed May 27, 2015
1 parent ba0e1cd commit 6773675
Show file tree
Hide file tree
Showing 25 changed files with 209 additions and 524 deletions.
5 changes: 0 additions & 5 deletions mk/tests.mk
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,6 @@ ifeq ($(CFG_LLDB),)
CTEST_DISABLE_debuginfo-lldb = "no lldb found"
endif

ifeq ($(CFG_CLANG),)
CTEST_DISABLE_codegen = "no clang found"
endif

ifneq ($(CFG_OSTYPE),apple-darwin)
CTEST_DISABLE_debuginfo-lldb = "lldb tests are only run on darwin"
endif
Expand Down Expand Up @@ -645,7 +641,6 @@ CTEST_COMMON_ARGS$(1)-T-$(2)-H-$(3) := \
--run-lib-path $$(TLIB$(1)_T_$(2)_H_$(3)) \
--rustc-path $$(HBIN$(1)_H_$(3))/rustc$$(X_$(3)) \
--rustdoc-path $$(HBIN$(1)_H_$(3))/rustdoc$$(X_$(3)) \
--clang-path $(if $(CFG_CLANG),$(CFG_CLANG),clang) \
--llvm-bin-path $(CFG_LLVM_INST_DIR_$(CFG_BUILD))/bin \
--aux-base $$(S)src/test/auxiliary/ \
--stage-id stage$(1)-$(2) \
Expand Down
3 changes: 0 additions & 3 deletions src/compiletest/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ pub struct Config {
// The python executable
pub python: String,

// The clang executable
pub clang_path: Option<PathBuf>,

// The llvm binaries path
pub llvm_bin_path: Option<PathBuf>,

Expand Down
25 changes: 4 additions & 21 deletions src/compiletest/compiletest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use std::fs;
use std::path::{Path, PathBuf};
use getopts::{optopt, optflag, reqopt};
use common::Config;
use common::{Pretty, DebugInfoGdb, DebugInfoLldb, Codegen};
use common::{Pretty, DebugInfoGdb, DebugInfoLldb};
use util::logv;

pub mod procsrv;
Expand Down Expand Up @@ -63,7 +63,6 @@ pub fn parse_config(args: Vec<String> ) -> Config {
reqopt("", "rustc-path", "path to rustc to use for compiling", "PATH"),
reqopt("", "rustdoc-path", "path to rustdoc to use for compiling", "PATH"),
reqopt("", "python", "path to python to use for doc tests", "PATH"),
optopt("", "clang-path", "path to executable for codegen tests", "PATH"),
optopt("", "valgrind-path", "path to Valgrind executable for Valgrind tests", "PROGRAM"),
optflag("", "force-valgrind", "fail if Valgrind tests cannot be run under Valgrind"),
optopt("", "llvm-bin-path", "path to directory holding llvm binaries", "DIR"),
Expand Down Expand Up @@ -133,7 +132,6 @@ pub fn parse_config(args: Vec<String> ) -> Config {
rustc_path: opt_path(matches, "rustc-path"),
rustdoc_path: opt_path(matches, "rustdoc-path"),
python: matches.opt_str("python").unwrap(),
clang_path: matches.opt_str("clang-path").map(|s| PathBuf::from(&s)),
valgrind_path: matches.opt_str("valgrind-path"),
force_valgrind: matches.opt_present("force-valgrind"),
llvm_bin_path: matches.opt_str("llvm-bin-path").map(|s| PathBuf::from(&s)),
Expand Down Expand Up @@ -284,13 +282,7 @@ pub fn make_tests(config: &Config) -> Vec<test::TestDescAndFn> {
let file = file.unwrap().path();
debug!("inspecting file {:?}", file.display());
if is_test(config, &file) {
let t = make_test(config, &file, || {
match config.mode {
Codegen => make_metrics_test_closure(config, &file),
_ => make_test_closure(config, &file)
}
});
tests.push(t)
tests.push(make_test(config, &file))
}
}
tests
Expand Down Expand Up @@ -323,16 +315,15 @@ pub fn is_test(config: &Config, testfile: &Path) -> bool {
return valid;
}

pub fn make_test<F>(config: &Config, testfile: &Path, f: F) -> test::TestDescAndFn where
F: FnOnce() -> test::TestFn,
pub fn make_test(config: &Config, testfile: &Path) -> test::TestDescAndFn
{
test::TestDescAndFn {
desc: test::TestDesc {
name: make_test_name(config, testfile),
ignore: header::is_test_ignored(config, testfile),
should_panic: test::ShouldPanic::No,
},
testfn: f(),
testfn: make_test_closure(config, &testfile),
}
}

Expand All @@ -357,14 +348,6 @@ pub fn make_test_closure(config: &Config, testfile: &Path) -> test::TestFn {
}))
}

pub fn make_metrics_test_closure(config: &Config, testfile: &Path) -> test::TestFn {
let config = (*config).clone();
let testfile = testfile.to_path_buf();
test::DynMetricFn(box move |mm: &mut test::MetricMap| {
runtest::run_metrics(config, &testfile, mm)
})
}

fn extract_gdb_version(full_version_line: Option<String>) -> Option<String> {
match full_version_line {
Some(ref full_version_line)
Expand Down
131 changes: 13 additions & 118 deletions src/compiletest/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ use std::iter::repeat;
use std::net::TcpStream;
use std::path::{Path, PathBuf};
use std::process::{Command, Output, ExitStatus};
use std::str;
use test::MetricMap;

pub fn run(config: Config, testfile: &Path) {
match &*config.target {
Expand All @@ -43,11 +41,6 @@ pub fn run(config: Config, testfile: &Path) {
_=> { }
}

let mut _mm = MetricMap::new();
run_metrics(config, testfile, &mut _mm);
}

pub fn run_metrics(config: Config, testfile: &Path, mm: &mut MetricMap) {
if config.verbose {
// We're going to be dumping a lot of info. Start on a new line.
print!("\n\n");
Expand All @@ -64,7 +57,7 @@ pub fn run_metrics(config: Config, testfile: &Path, mm: &mut MetricMap) {
Pretty => run_pretty_test(&config, &props, &testfile),
DebugInfoGdb => run_debuginfo_gdb_test(&config, &props, &testfile),
DebugInfoLldb => run_debuginfo_lldb_test(&config, &props, &testfile),
Codegen => run_codegen_test(&config, &props, &testfile, mm),
Codegen => run_codegen_test(&config, &props, &testfile),
Rustdoc => run_rustdoc_test(&config, &props, &testfile),
}
}
Expand Down Expand Up @@ -1685,26 +1678,15 @@ fn _arm_push_aux_shared_library(config: &Config, testfile: &Path) {
}
}

// codegen tests (vs. clang)

fn append_suffix_to_stem(p: &Path, suffix: &str) -> PathBuf {
if suffix.is_empty() {
p.to_path_buf()
} else {
let mut stem = p.file_stem().unwrap().to_os_string();
stem.push("-");
stem.push(suffix);
p.with_file_name(&stem)
}
}
// codegen tests (using FileCheck)

fn compile_test_and_save_bitcode(config: &Config, props: &TestProps,
fn compile_test_and_save_ir(config: &Config, props: &TestProps,
testfile: &Path) -> ProcRes {
let aux_dir = aux_output_dir_name(config, testfile);
// FIXME (#9639): This needs to handle non-utf8 paths
let mut link_args = vec!("-L".to_string(),
aux_dir.to_str().unwrap().to_string());
let llvm_args = vec!("--emit=llvm-bc,obj".to_string(),
let llvm_args = vec!("--emit=llvm-ir".to_string(),
"--crate-type=lib".to_string());
link_args.extend(llvm_args.into_iter());
let args = make_compile_args(config,
Expand All @@ -1717,121 +1699,34 @@ fn compile_test_and_save_bitcode(config: &Config, props: &TestProps,
compose_and_run_compiler(config, props, testfile, args, None)
}

fn compile_cc_with_clang_and_save_bitcode(config: &Config, _props: &TestProps,
testfile: &Path) -> ProcRes {
let bitcodefile = output_base_name(config, testfile).with_extension("bc");
let bitcodefile = append_suffix_to_stem(&bitcodefile, "clang");
let testcc = testfile.with_extension("cc");
let proc_args = ProcArgs {
// FIXME (#9639): This needs to handle non-utf8 paths
prog: config.clang_path.as_ref().unwrap().to_str().unwrap().to_string(),
args: vec!("-c".to_string(),
"-emit-llvm".to_string(),
"-o".to_string(),
bitcodefile.to_str().unwrap().to_string(),
testcc.to_str().unwrap().to_string())
};
compose_and_run(config, testfile, proc_args, Vec::new(), "", None, None)
}

fn extract_function_from_bitcode(config: &Config, _props: &TestProps,
fname: &str, testfile: &Path,
suffix: &str) -> ProcRes {
let bitcodefile = output_base_name(config, testfile).with_extension("bc");
let bitcodefile = append_suffix_to_stem(&bitcodefile, suffix);
let extracted_bc = append_suffix_to_stem(&bitcodefile, "extract");
let prog = config.llvm_bin_path.as_ref().unwrap().join("llvm-extract");
fn check_ir_with_filecheck(config: &Config, testfile: &Path) -> ProcRes {
let irfile = output_base_name(config, testfile).with_extension("ll");
let prog = config.llvm_bin_path.as_ref().unwrap().join("FileCheck");
let proc_args = ProcArgs {
// FIXME (#9639): This needs to handle non-utf8 paths
prog: prog.to_str().unwrap().to_string(),
args: vec!(format!("-func={}", fname),
format!("-o={}", extracted_bc.to_str().unwrap()),
bitcodefile.to_str().unwrap().to_string())
args: vec!(format!("-input-file={}", irfile.to_str().unwrap()),
testfile.to_str().unwrap().to_string())
};
compose_and_run(config, testfile, proc_args, Vec::new(), "", None, None)
}

fn disassemble_extract(config: &Config, _props: &TestProps,
testfile: &Path, suffix: &str) -> ProcRes {
let bitcodefile = output_base_name(config, testfile).with_extension("bc");
let bitcodefile = append_suffix_to_stem(&bitcodefile, suffix);
let extracted_bc = append_suffix_to_stem(&bitcodefile, "extract");
let extracted_ll = extracted_bc.with_extension("ll");
let prog = config.llvm_bin_path.as_ref().unwrap().join("llvm-dis");
let proc_args = ProcArgs {
// FIXME (#9639): This needs to handle non-utf8 paths
prog: prog.to_str().unwrap().to_string(),
args: vec!(format!("-o={}", extracted_ll.to_str().unwrap()),
extracted_bc.to_str().unwrap().to_string())
};
compose_and_run(config, testfile, proc_args, Vec::new(), "", None, None)
}


fn count_extracted_lines(p: &Path) -> usize {
let mut x = Vec::new();
File::open(&p.with_extension("ll")).unwrap().read_to_end(&mut x).unwrap();
let x = str::from_utf8(&x).unwrap();
x.lines().count()
}


fn run_codegen_test(config: &Config, props: &TestProps,
testfile: &Path, mm: &mut MetricMap) {
fn run_codegen_test(config: &Config, props: &TestProps, testfile: &Path) {

if config.llvm_bin_path.is_none() {
fatal("missing --llvm-bin-path");
}

if config.clang_path.is_none() {
fatal("missing --clang-path");
}

let mut proc_res = compile_test_and_save_bitcode(config, props, testfile);
if !proc_res.status.success() {
fatal_proc_rec("compilation failed!", &proc_res);
}

proc_res = extract_function_from_bitcode(config, props, "test", testfile, "");
if !proc_res.status.success() {
fatal_proc_rec("extracting 'test' function failed",
&proc_res);
}

proc_res = disassemble_extract(config, props, testfile, "");
if !proc_res.status.success() {
fatal_proc_rec("disassembling extract failed", &proc_res);
}


let mut proc_res = compile_cc_with_clang_and_save_bitcode(config, props, testfile);
let mut proc_res = compile_test_and_save_ir(config, props, testfile);
if !proc_res.status.success() {
fatal_proc_rec("compilation failed!", &proc_res);
}

proc_res = extract_function_from_bitcode(config, props, "test", testfile, "clang");
proc_res = check_ir_with_filecheck(config, testfile);
if !proc_res.status.success() {
fatal_proc_rec("extracting 'test' function failed",
fatal_proc_rec("verification with 'FileCheck' failed",
&proc_res);
}

proc_res = disassemble_extract(config, props, testfile, "clang");
if !proc_res.status.success() {
fatal_proc_rec("disassembling extract failed", &proc_res);
}

let base = output_base_name(config, testfile);
let base_extract = append_suffix_to_stem(&base, "extract");

let base_clang = append_suffix_to_stem(&base, "clang");
let base_clang_extract = append_suffix_to_stem(&base_clang, "extract");

let base_lines = count_extracted_lines(&base_extract);
let clang_lines = count_extracted_lines(&base_clang_extract);

mm.insert_metric("clang-codegen-ratio",
(base_lines as f64) / (clang_lines as f64),
0.001);
}

fn charset() -> &'static str {
Expand Down
95 changes: 95 additions & 0 deletions src/test/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -C no-prepopulate-passes

#![feature(allocator)]

pub struct S {
_field: [i64; 4],
}

pub struct UnsafeInner {
_field: std::cell::UnsafeCell<i16>,
}

// CHECK: zeroext i1 @boolean(i1 zeroext)
#[no_mangle]
pub fn boolean(x: bool) -> bool {
x
}

// CHECK: @readonly_borrow(i32* noalias readonly dereferenceable(4))
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn readonly_borrow(_: &i32) {
}

// CHECK: @static_borrow(i32* noalias readonly dereferenceable(4))
// static borrow may be captured
#[no_mangle]
pub fn static_borrow(_: &'static i32) {
}

// CHECK: @named_borrow(i32* noalias readonly dereferenceable(4))
// borrow with named lifetime may be captured
#[no_mangle]
pub fn named_borrow<'r>(_: &'r i32) {
}

// CHECK: @unsafe_borrow(%UnsafeInner* dereferenceable(2))
// unsafe interior means this isn't actually readonly and there may be aliases ...
#[no_mangle]
pub fn unsafe_borrow(_: &UnsafeInner) {
}

// CHECK: @mutable_unsafe_borrow(%UnsafeInner* noalias dereferenceable(2))
// ... unless this is a mutable borrow, those never alias
#[no_mangle]
pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) {
}

// CHECK: @mutable_borrow(i32* noalias dereferenceable(4))
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn mutable_borrow(_: &mut i32) {
}

// CHECK: @indirect_struct(%S* noalias nocapture dereferenceable(32))
#[no_mangle]
pub fn indirect_struct(_: S) {
}

// CHECK: @borrowed_struct(%S* noalias readonly dereferenceable(32))
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn borrowed_struct(_: &S) {
}

// CHECK: noalias dereferenceable(4) i32* @_box(i32* noalias dereferenceable(4))
#[no_mangle]
pub fn _box(x: Box<i32>) -> Box<i32> {
x
}

// CHECK: @struct_return(%S* noalias nocapture sret dereferenceable(32))
#[no_mangle]
pub fn struct_return() -> S {
S {
_field: [0, 0, 0, 0]
}
}

// CHECK: noalias i8* @allocator()
#[no_mangle]
#[allocator]
pub fn allocator() -> *const i8 {
std::ptr::null()
}
Loading

0 comments on commit 6773675

Please sign in to comment.