Stream: t-compiler

Topic: Tests with optimization-level specified by the test?


nagisa (Nov 02 2018 at 12:33, on Zulip):

I remember us having a lot of trouble with tests not being able to specify the optimisation level they are aimed to. Usually the workaround was to use -Cno-prepopulate-passes. Well, now that I’m trying to make some tests for #[optimize] attribute, that does not work, because doing a -Cno-prepopulate-passes does not actually influence the optimisation flags, so something like the following code:

#![feature(optimize_attribute)]

pub fn foo() -> i32 {
    2 + 2
}

#[optimize(size)]
pub fn size() -> i32 {
    2 + 2
}

#[optimize(speed)]
pub fn speed() -> i32 {
    2 + 2
}

cannot be tested with various sets of -O and -Copt-level flags to ensure the correct codegen. Any easy way out?

nagisa (Nov 02 2018 at 12:36, on Zulip):

There are notable differences in how these functions are compiled with different flags (esp. the foo) function. This is what is generated with no optimisation flags:

; Function Attrs: nonlazybind optnone uwtable
define i32 @test::foo() unnamed_addr #0 {
start:
  %0 = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 2, i32 2)
  %1 = extractvalue { i32, i1 } %0, 0
  %2 = extractvalue { i32, i1 } %0, 1
  br i1 %2, label %panic, label %bb1, !prof !1

bb1:                                              ; preds = %start
<snip>

panic:                                            ; preds = %start
<snip>
}

whereas if compiled with -Copt-level=z you get

; Function Attrs: minsize norecurse nounwind nonlazybind optsize readnone uwtable
define i32 @test::foo() unnamed_addr #0 {
start:
  ret i32 4
}

and so on and forth.

nagisa (Nov 02 2018 at 12:39, on Zulip):

It would be ideal, even, if we could use the same test file with multiple configurations… just like LLVM does it.

nagisa (Nov 02 2018 at 13:54, on Zulip):

TL;DR I’m looking a way to write a codegen test that can vary over an optimization level flag.

nikomatsakis (Nov 02 2018 at 18:27, on Zulip):

@nagisa can you use revisions to do it?

nikomatsakis (Nov 02 2018 at 18:27, on Zulip):

you can do // revisions: foo bar for example in a test

nikomatsakis (Nov 02 2018 at 18:27, on Zulip):

and then do //[foo] compile-flags: ...

nikomatsakis (Nov 02 2018 at 18:27, on Zulip):

and //[bar] compile-flags: ...

nagisa (Nov 03 2018 at 05:07, on Zulip):

revisions sound nice, but can one specify arbitrary compile-flags? What I need is specifically -Copt-level. I recall it wasn’t possible before.

nagisa (Nov 03 2018 at 09:09, on Zulip):
// 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

#![feature(optimize_attribute)]

// NO-OPT: Function Attrs:{{.*}}optnone
// NO-OPT-NOT: {{optsize|minsize}}
// NO-OPT-NEXT: @nothing
// 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
#[no_mangle]
pub fn nothing() -> i32 {
    2 + 2
}

// CHECK: Function Attrs:{{.*}} minsize{{.*}}optsize
// CHECK-NEXT: @size
// CHECK-NEXT: start
// CHECK-NEXT: ret i32 4
#[optimize(size)]
#[no_mangle]
pub fn size() -> i32 {
    2 + 2
}

// CHECK: Function Attrs:
// CHECK-NOT: {{minsize|optsize|optnone}}
// CHECK-NEXT: @speed
// CHECK-NEXT: start
// CHECK-NEXT: ret i32 4
#[optimize(speed)]
#[no_mangle]
pub fn speed() -> i32 {
    2 + 2
}
nagisa (Nov 03 2018 at 09:12, on Zulip):

is the test I’m trying to write. Alas, the codegen tests do not support "revisions" in the first place:

---- [codegen] codegen/optimize-attr-1.rs#NO-OPT stdout ----
thread '[codegen] codegen/optimize-attr-1.rs#NO-OPT' panicked at 'revisions not relevant here', tools/compiletest/src/runtest.rs:2093:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
pnkfelix (Nov 06 2018 at 10:33, on Zulip):

I was just encountering this yesterday. providing -Copt-level in a // compile-flags is no good because the test driver will also inject its own -O and then the rustc invocation will error out because of multiple opt-level options being provided

pnkfelix (Nov 06 2018 at 10:34, on Zulip):

Maybe we can add a // opt-level: ... annotation for the test, and then that overrides whatever the default is in compiletest ?

pnkfelix (Nov 06 2018 at 10:34, on Zulip):

(my own scenario was a run-pass test, not a codegen one, but I imagine the issues are similar...)

pnkfelix (Nov 06 2018 at 13:20, on Zulip):

Hmm. Doesn't the same scenario arguably arise with debuginfo-tests? I had to actually manually add -g to a test just now via // compile-flags: ..., which strikes me as odd because the config.toml.example says that debuginfo-tests=true is the default...

mw (Nov 06 2018 at 13:31, on Zulip):

@nagisa It shouldn't be too hard to add revisions support for codegen tests. runtest.rs has most of the code.

nagisa (Nov 06 2018 at 15:54, on Zulip):

yeah, but revisions is of least concern to me, I’ll have to work on making sure -Copt-level works in compile-flags.

pnkfelix (Nov 06 2018 at 15:57, on Zulip):

@nagisa what do you think of instead adding a // opt-level: N (and // debuginfo: N) property to tests?

pnkfelix (Nov 06 2018 at 15:58, on Zulip):

(that then overrides any default in use by the compiletest environment)

nagisa (Nov 06 2018 at 16:35, on Zulip):

doesn’t sound general enough – I’m sure we’ll end up hitting more obscure flags like these in the future.

nagisa (Nov 06 2018 at 16:35, on Zulip):

I’d rather just parse the compile-flags line and override all default options in compiletest.

pnkfelix (Nov 06 2018 at 18:55, on Zulip):

hmm. well at some point we'll have to wonder if we're better off pulling the option definitions out into their own crate that compiletest could link to...

mw (Nov 07 2018 at 09:20, on Zulip):

maybe add something like // ignore-default-flags: <flags not to add by default>

pnkfelix (Nov 07 2018 at 11:01, on Zulip):

@mw that sounds just has hard to implement as what @nagisa is describing, and harder to use, no?

mw (Nov 07 2018 at 16:17, on Zulip):

oh right, I didn't see that. Seems like a fine solution.

Last update: Nov 22 2019 at 05:25UTC