Support revisions for codegen tests

nagisa · nagisa · commit 4d97b2889345 · 2019-01-24T20:13:51.000+02:00
`compile-flags: -Copt-level` will avoid adding -O. Similarly for -g and
-Cdebuglevel.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<String>) {
+        // 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<space>*/"opt-level="];
+        const DEBUG_FLAGS: &[&str] = &["-g", "-Cdebuginfo=", /*-C<space>*/"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");
         }

Original file line number	Diff line number	Diff line change
`@@ -155,8 +155,6 @@ pub fn from_fn_attrs(`
`155`	`155`	`let codegen_fn_attrs = id.map(\|id\| cx.tcx.codegen_fn_attrs(id))`
`156`	`156`	`.unwrap_or_else(\|\| CodegenFnAttrs::new());`
`157`	`157`
`158`		`- inline(cx, llfn, codegen_fn_attrs.inline);`
`159`		`-`
`160`	`158`	`match codegen_fn_attrs.optimize {`
`161`	`159`	`OptimizeAttr::None => {`
`162`	`160`	`match cx.tcx.sess.opts.optimize {`
`@@ -173,7 +171,7 @@ pub fn from_fn_attrs(`
`173`	`171`	`OptLevel::No => {`
`174`	`172`	`llvm::Attribute::MinSize.unapply_llfn(Function, llfn);`
`175`	`173`	`llvm::Attribute::OptimizeForSize.unapply_llfn(Function, llfn);`
`176`		`- llvm::Attribute::OptimizeNone.apply_llfn(Function, llfn);`
	`174`	`+ llvm::Attribute::OptimizeNone.unapply_llfn(Function, llfn);`
`177`	`175`	`}`
`178`	`176`	`_ => {}`
`179`	`177`	`}`
`@@ -190,6 +188,8 @@ pub fn from_fn_attrs(`
`190`	`188`	`}`
`191`	`189`	`}`
`192`	`190`
	`191`	`+ inline(cx, llfn, codegen_fn_attrs.inline);`
	`192`	`+`
`193`	`193`	// The `uwtable` attribute according to LLVM is:
`194`	`194`	`//`
`195`	`195`	`// This attribute indicates that the ABI being targeted requires that an`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ fn declare_raw_fn(`
`65`	`65`	`}`
`66`	`66`	`}`
`67`	`67`
`68`		`- // FIXME(opt): this is kinda duplicated with similar code in attributes::from_fm_attrs…`
	`68`	`+ // FIXME(opt): this is kinda duplicated with similar code in attributes::from_fn_attrs…`
`69`	`69`	`match cx.tcx.sess.opts.optimize {`
`70`	`70`	`OptLevel::Size => {`
`71`	`71`	`llvm::Attribute::MinSize.unapply_llfn(Function, llfn);`
`@@ -80,7 +80,7 @@ fn declare_raw_fn(`
`80`	`80`	`OptLevel::No => {`
`81`	`81`	`llvm::Attribute::MinSize.unapply_llfn(Function, llfn);`
`82`	`82`	`llvm::Attribute::OptimizeForSize.unapply_llfn(Function, llfn);`
`83`		`- llvm::Attribute::OptimizeNone.apply_llfn(Function, llfn);`
	`83`	`+ llvm::Attribute::OptimizeNone.unapply_llfn(Function, llfn);`
`84`	`84`	`}`
`85`	`85`	`_ => {}`
`86`	`86`	`}`