From 4d97b2889345d69fa72233b787b30cff73232465 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sun, 18 Nov 2018 20:51:56 +0200 Subject: [PATCH] Support revisions for codegen tests `compile-flags: -Copt-level` will avoid adding -O. Similarly for -g and -Cdebuglevel. --- src/librustc_codegen_llvm/attributes.rs | 6 +- src/librustc_codegen_llvm/declare.rs | 4 +- .../codegen/inline-always-works-always.rs | 21 ++++++ src/test/codegen/optimize-attr-1.rs | 59 +++++++-------- src/tools/compiletest/src/header.rs | 2 + src/tools/compiletest/src/runtest.rs | 72 +++++++++++++++---- 6 files changed, 115 insertions(+), 49 deletions(-) create mode 100644 src/test/codegen/inline-always-works-always.rs diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 5a39e4f8b7ff..a7d4f910f7a2 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -155,8 +155,6 @@ pub fn from_fn_attrs( let codegen_fn_attrs = id.map(|id| cx.tcx.codegen_fn_attrs(id)) .unwrap_or_else(|| CodegenFnAttrs::new()); - inline(cx, llfn, codegen_fn_attrs.inline); - match codegen_fn_attrs.optimize { OptimizeAttr::None => { match cx.tcx.sess.opts.optimize { @@ -173,7 +171,7 @@ pub fn from_fn_attrs( OptLevel::No => { llvm::Attribute::MinSize.unapply_llfn(Function, llfn); llvm::Attribute::OptimizeForSize.unapply_llfn(Function, llfn); - llvm::Attribute::OptimizeNone.apply_llfn(Function, llfn); + llvm::Attribute::OptimizeNone.unapply_llfn(Function, llfn); } _ => {} } @@ -190,6 +188,8 @@ pub fn from_fn_attrs( } } + inline(cx, llfn, codegen_fn_attrs.inline); + // The `uwtable` attribute according to LLVM is: // // This attribute indicates that the ABI being targeted requires that an diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs index 166c6ab9ae9c..da45e0200d85 100644 --- a/src/librustc_codegen_llvm/declare.rs +++ b/src/librustc_codegen_llvm/declare.rs @@ -65,7 +65,7 @@ fn declare_raw_fn( } } - // FIXME(opt): this is kinda duplicated with similar code in attributes::from_fm_attrs… + // FIXME(opt): this is kinda duplicated with similar code in attributes::from_fn_attrs… match cx.tcx.sess.opts.optimize { OptLevel::Size => { llvm::Attribute::MinSize.unapply_llfn(Function, llfn); @@ -80,7 +80,7 @@ fn declare_raw_fn( OptLevel::No => { llvm::Attribute::MinSize.unapply_llfn(Function, llfn); llvm::Attribute::OptimizeForSize.unapply_llfn(Function, llfn); - llvm::Attribute::OptimizeNone.apply_llfn(Function, llfn); + llvm::Attribute::OptimizeNone.unapply_llfn(Function, llfn); } _ => {} } diff --git a/src/test/codegen/inline-always-works-always.rs b/src/test/codegen/inline-always-works-always.rs new file mode 100644 index 000000000000..912af782a8f3 --- /dev/null +++ b/src/test/codegen/inline-always-works-always.rs @@ -0,0 +1,21 @@ +// revisions: NO-OPT SIZE-OPT SPEED-OPT +//[NO-OPT] compile-flags: -Copt-level=0 +//[SIZE-OPT] compile-flags: -Copt-level=s +//[SPEED-OPT] compile-flags: -Copt-level=3 + +#![crate_type="rlib"] + +#[no_mangle] +#[inline(always)] +pub extern "C" fn callee() -> u32 { + 4 + 4 +} + +// CHECK-LABEL: caller +// SIZE-OPT: ret i32 8 +// SPEED-OPT: ret i32 8 +// NO-OPT: ret i32 8 +#[no_mangle] +pub extern "C" fn caller() -> u32 { + callee() +} diff --git a/src/test/codegen/optimize-attr-1.rs b/src/test/codegen/optimize-attr-1.rs index b742a361da96..376447e5b5db 100644 --- a/src/test/codegen/optimize-attr-1.rs +++ b/src/test/codegen/optimize-attr-1.rs @@ -1,49 +1,50 @@ // revisions: NO-OPT SIZE-OPT SPEED-OPT -// [NO-OPT]compile-flags: -Copt-level=0 -// [SIZE-OPT]compile-flags: -Copt-level=s -// [SPEED-OPT]compile-flags: -Copt-level=3 +//[NO-OPT] compile-flags: -Copt-level=0 -Ccodegen-units=1 +//[SIZE-OPT] compile-flags: -Copt-level=s -Ccodegen-units=1 +//[SPEED-OPT] compile-flags: -Copt-level=3 -Ccodegen-units=1 #![feature(optimize_attribute)] #![crate_type="rlib"] -// NO-OPT: Function Attrs:{{.*}}optnone -// NO-OPT-NOT: {{optsize|minsize}} -// NO-OPT-NEXT: @nothing +// CHECK-LABEL: define i32 @nothing +// CHECK-SAME: [[NOTHING_ATTRS:#[0-9]+]] // NO-OPT: ret i32 %1 -// -// SIZE-OPT: Function Attrs:{{.*}}optsize -// SIZE-OPT-NOT: {{minsize|optnone}} -// SIZE-OPT-NEXT: @nothing -// SIZE-OPT-NEXT: start -// SIZE-OPT-NEXT: ret i32 4 -// -// SPEED-OPT: Function Attrs: -// SPEED-OPT-NOT: {{minsize|optnone|optsize}} -// SPEED-OPT-NEXT: @nothing -// SPEED-OPT-NEXT: start -// SPEED-OPT-NEXT: ret i32 4 +// SIZE-OPT: ret i32 4 +// SPEEC-OPT: ret i32 4 #[no_mangle] pub fn nothing() -> i32 { 2 + 2 } -// CHECK: Function Attrs:{{.*}} minsize{{.*}}optsize -// CHECK-NEXT: @size -// CHECK-NEXT: start -// CHECK-NEXT: ret i32 4 +// CHECK-LABEL: define i32 @size +// CHECK-SAME: [[SIZE_ATTRS:#[0-9]+]] +// NO-OPT: ret i32 %1 +// SIZE-OPT: ret i32 6 +// SPEED-OPT: ret i32 6 #[optimize(size)] #[no_mangle] pub fn size() -> i32 { - 2 + 2 + 3 + 3 } -// CHECK: Function Attrs: -// CHECK-NOT: {{minsize|optsize|optnone}} -// CHECK-NEXT: @speed -// CHECK-NEXT: start -// CHECK-NEXT: ret i32 4 +// CHECK-LABEL: define i32 @speed +// NO-OPT-SAME: [[NOTHING_ATTRS]] +// SPEED-OPT-SAME: [[NOTHING_ATTRS]] +// SIZE-OPT-SAME: [[SPEED_ATTRS:#[0-9]+]] +// NO-OPT: ret i32 %1 +// SIZE-OPT: ret i32 8 +// SPEED-OPT: ret i32 8 #[optimize(speed)] #[no_mangle] pub fn speed() -> i32 { - 2 + 2 + 4 + 4 } + +// NO-OPT-DAG: attributes [[SIZE_ATTRS]] = {{.*}}minsize{{.*}}optsize{{.*}} +// SPEED-OPT-DAG: attributes [[SIZE_ATTRS]] = {{.*}}minsize{{.*}}optsize{{.*}} +// SIZE-OPT-DAG: attributes [[NOTHING_ATTRS]] = {{.*}}optsize{{.*}} +// SIZE-OPT-DAG: attributes [[SIZE_ATTRS]] = {{.*}}minsize{{.*}}optsize{{.*}} + +// SIZE-OPT: attributes [[SPEED_ATTRS]] +// SIZE-OPT-NOT: minsize +// SIZE-OPT-NOT: optsize diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 5eb1f5ec5ffd..394e408e4158 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -542,6 +542,8 @@ fn iter_header(testfile: &Path, cfg: Option<&str>, it: &mut dyn FnMut(&str)) { "#" }; + // FIXME: would be nice to allow some whitespace between comment and brace :) + // It took me like 2 days to debug why compile-flags weren’t taken into account for my test :) let comment_with_brace = comment.to_string() + "["; let rdr = BufReader::new(File::open(testfile).unwrap()); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 400c205d44b2..375dc1c12833 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -552,9 +552,10 @@ impl<'test> TestCx<'test> { .args(&["--target", &self.config.target]) .arg("-L") .arg(&aux_dir) - .args(self.split_maybe_args(&self.config.target_rustcflags)) .args(&self.props.compile_flags) .envs(self.props.exec_env.clone()); + self.maybe_add_external_args(&mut rustc, + self.split_maybe_args(&self.config.target_rustcflags)); let src = match read_from { ReadFrom::Stdin(src) => Some(src), @@ -587,6 +588,15 @@ impl<'test> TestCx<'test> { } } + fn set_revision_flags(&self, cmd: &mut Command) { + if let Some(revision) = self.revision { + // Normalize revisions to be lowercase and replace `-`s with `_`s. + // Otherwise the `--cfg` flag is not valid. + let normalized_revision = revision.to_lowercase().replace("-", "_"); + cmd.args(&["--cfg", &normalized_revision]); + } + } + fn typecheck_source(&self, src: String) -> ProcRes { let mut rustc = Command::new(&self.config.rustc_path); @@ -612,12 +622,9 @@ impl<'test> TestCx<'test> { .arg(&self.config.build_base) .arg("-L") .arg(aux_dir); - - if let Some(revision) = self.revision { - rustc.args(&["--cfg", revision]); - } - - rustc.args(self.split_maybe_args(&self.config.target_rustcflags)); + self.set_revision_flags(&mut rustc); + self.maybe_add_external_args(&mut rustc, + self.split_maybe_args(&self.config.target_rustcflags)); rustc.args(&self.props.compile_flags); self.compose_and_run_compiler(rustc, Some(src)) @@ -1119,6 +1126,35 @@ impl<'test> TestCx<'test> { Some(new_options.join(" ")) } + fn maybe_add_external_args(&self, cmd: &mut Command, args: Vec) { + // Filter out the arguments that should not be added by runtest here. + // + // Notable use-cases are: do not add our optimisation flag if + // `compile-flags: -Copt-level=x` and similar for debug-info level as well. + const OPT_FLAGS: &[&str] = &["-O", "-Copt-level=", /*-C*/"opt-level="]; + const DEBUG_FLAGS: &[&str] = &["-g", "-Cdebuginfo=", /*-C*/"debuginfo="]; + + // FIXME: ideally we would "just" check the `cmd` itself, but it does not allow inspecting + // its arguments. They need to be collected separately. For now I cannot be bothered to + // implement this the "right" way. + let have_opt_flag = self.props.compile_flags.iter().any(|arg| { + OPT_FLAGS.iter().any(|f| arg.starts_with(f)) + }); + let have_debug_flag = self.props.compile_flags.iter().any(|arg| { + DEBUG_FLAGS.iter().any(|f| arg.starts_with(f)) + }); + + for arg in args { + if OPT_FLAGS.iter().any(|f| arg.starts_with(f)) && have_opt_flag { + continue; + } + if DEBUG_FLAGS.iter().any(|f| arg.starts_with(f)) && have_debug_flag { + continue; + } + cmd.arg(arg); + } + } + fn check_debugger_output(&self, debugger_run_result: &ProcRes, check_lines: &[String]) { let num_check_lines = check_lines.len(); @@ -1707,10 +1743,7 @@ impl<'test> TestCx<'test> { rustc.arg(&format!("--target={}", target)); } - - if let Some(revision) = self.revision { - rustc.args(&["--cfg", revision]); - } + self.set_revision_flags(&mut rustc); if !is_rustdoc { if let Some(ref incremental_dir) = self.props.incremental_dir { @@ -1810,9 +1843,11 @@ impl<'test> TestCx<'test> { } if self.props.force_host { - rustc.args(self.split_maybe_args(&self.config.host_rustcflags)); + self.maybe_add_external_args(&mut rustc, + self.split_maybe_args(&self.config.host_rustcflags)); } else { - rustc.args(self.split_maybe_args(&self.config.target_rustcflags)); + self.maybe_add_external_args(&mut rustc, + self.split_maybe_args(&self.config.target_rustcflags)); if !is_rustdoc { if let Some(ref linker) = self.config.linker { rustc.arg(format!("-Clinker={}", linker)); @@ -2065,12 +2100,19 @@ impl<'test> TestCx<'test> { .arg("--input-file") .arg(irfile) .arg(&self.testpaths.file); + // It would be more appropriate to make most of the arguments configurable through + // a comment-attribute similar to `compile-flags`. For example, --check-prefixes is a very + // useful flag. + // + // For now, though… + if let Some(rev) = self.revision { + let prefixes = format!("CHECK,{}", rev); + filecheck.args(&["--check-prefixes", &prefixes]); + } self.compose_and_run(filecheck, "", None, None) } fn run_codegen_test(&self) { - assert!(self.revision.is_none(), "revisions not relevant here"); - if self.config.llvm_filecheck.is_none() { self.fatal("missing --llvm-filecheck"); }