llvm dev - Nov 2010 - [LLVMdev] LLVM unit and regression tests, enabled targets and conditions

Hello everyone,

I'm wrestling with the LLVM unit and regression tests now and would
like to discuss some changes to make.
I will be preparing patches, but any input/ideas are welcome.

I leave "where the tests should run" question out for the scope yet.
Let's review what kind of tests we have and how to handle them
correctly.

We have 3 types of unit and regression tests:

1.) Target-independent tests

These tests should be run once per build and they do not depend on
what and how many targets LLVM is configured and built for.

For example, lli interpreter tests are the type 1 tests.

2.) Target-dependent tests

These tests should be run multiple times per build, once for each of
the targets LLVM is built for. These tests are valid for any supported
target but need to be configures/started on some target-specific way.

For example, the test/CodeGen/Generic/2003-07-07-BadLongConst.ll test
is the type 2 test.

3.) Conditional tests

These tests are subjects to some conditions, like target(s), or
triple(s), or host platform.
These tests should be run only if specified conditions are met or
considered UNSUPPORTED otherwise.
Both types (1) Target-independent and (2) Target-dependent tests could
be also type 3 tests.

For example, lli JIT tests are the type 3 tests and should be run only
for the host platform if it is supported by lli.
test/DebugInfo/2010-08-04-StackVariable.ll is another example of such tests.

As it is now, tests are treated as type 1 by default, we do not have
support for type 2 tests and we define type 3 tests by using
XTARGET/XFAIL declarations (for example
llvm/test/FrontendC/2009-01-20-k8.c) and/or DejaGNU configuration
files for group of tests (like llvm/test/MC/X86/dg.exp).

We have a nice way to keep tests self-declared. Each unit or
regression test contain 2 parts: declarative part with meta-data of
how the test should be run and how the result should be interpreted,
and the test itself. I like this. It allows us to keep all related
information in one place and make each test quite explicit.

To follow this pattern, I think, we need to extend the declarative
part by adding an explicit dependency statement. Technically, we need
to declare a dependency (or a run condition), and for what targets the
test should be run.

I propose

A.) A new condition statement, something like

; DEPENDS: <condition>
or
; REQUIRE: <condition>

0 or many could be declared with any test. If more than one is
declared, all must be met to run the test, otherwise it gets marked as
UNSUPPORTED.

For example, the test/DebugInfo/2010-08-04-StackVariable.ll test could
have the declarative part as

; REQUIRE: llc -mtriple=arm-apple-darwin    < /dev/null
; REQUIRE: llc -mtriple=x86_64-apple-darwin < /dev/null
; RUN: llc -O0 -mtriple=arm-apple-darwin    < %s | grep DW_OP_fbreg
; RUN: llc -O0 -mtriple=x86_64-apple-darwin < %s | grep DW_OP_fbreg
; Use DW_OP_fbreg in variable's location expression if the variable is
in a stack slot.

B.) $each_target placeholder, which could be used in the test run
command to indicate that this is the type 2 test. This placeholder
will be replaced with the appropriate value at the test run time.

For example, if the test/CodeGen/Generic/2003-07-07-BadLongConst.ll
will be defined as

; RUN: llc -march=$each_target < %s

it will be executed as

llc -march=arm  <
/opt/test-buildslave/osuosl/slave/build-x-4-armeabi-hardfloat/llvm.src/test/CodeGen/Generic/2003-07-07-BadLongConst.ll
llc -march=thumb  <
/opt/test-buildslave/osuosl/slave/build-x-4-armeabi-hardfloat/llvm.src/test/CodeGen/Generic/2003-07-07-BadLongConst.ll

for llvm configured with --enable-targets=arm.

Note that $each_target and targets/llvm_supports_target() are
different. In the above example $each_target is [arm, thumb] (see llc
--version), and targets is [ARM].

What do you think?

Few related random notes:

* We can keep the DejaGNU config files (or make similar LIT configs)
to turn off group of tests.

* XTARGET makes test failure treated as expected. This is something
different than unsupported tests. Unsupported tests shouldn't even run
since could crash or behaves unexpectedly. Expected failure test
should run because they could produce unexpected successful result. Up
to me, we should deprecate XTARGET, if XFAIL accepts <condition>.

* We can extend declarative language to define variables. In this case
$each_target could be a test variable instead of a pre-defined
placeholder.

* target_triple is actually the host_triple. Shell we rename it?


Thanks

Galina

Good afternoon, Galina.
This is cool suggestion, I feel. :)

2010/11/24 Galina Kistanova <gkistanova at gmail.com>:
> We have 3 types of unit and regression tests:
> 1.) Target-independent tests
> 2.) Target-dependent tests
> 3.) Conditional tests
Shall we think of potentially "host-dependent" tests?

With one of my tasks, we should know llc -march=x86-64 behaves host-dependent.
I took the way to specify explicit -mtriple=x86_64-{linux|win32} then.
(are they needed; darwin, cygwin, or mingw32? :p)
You know, Win32 x64(aka Win64) ABI is different from AMD64 ABI.
> To follow this pattern, I think, we need to extend the declarative
> part by adding an explicit dependency statement. Technically, we need
> to declare a dependency (or a run condition), and for what targets the
> test should be run.
>
> I propose
>
> A.) A new condition statement, something like
>
> ; DEPENDS: <condition>
> or
> ; REQUIRE: <condition>
Lit has "REQUIRES:" for features.ToT llvm/test does not use
"features".
(clang/test uses it, and one of my patches is under review)

I am so poor for English, though, I would like to suggest "ASSUMES:".
> For example, the test/DebugInfo/2010-08-04-StackVariable.ll test could
> have the declarative part as
>
> ; REQUIRE: llc -mtriple=arm-apple-darwin    < /dev/null
> ; REQUIRE: llc -mtriple=x86_64-apple-darwin < /dev/null
> ; RUN: llc -O0 -mtriple=arm-apple-darwin    < %s | grep DW_OP_fbreg
> ; RUN: llc -O0 -mtriple=x86_64-apple-darwin < %s | grep DW_OP_fbreg
> ; Use DW_OP_fbreg in variable's location expression if the variable is
> in a stack slot.
I guess something label would be needed to extend to selective test. eg.

; ASSUMES-ARM: llc -mtriple=arm-apple-darwin    < /dev/null
; ASSUMES-X64: llc -mtriple=x86_64-apple-darwin < /dev/null
; RUN-ARM: llc -O0 -mtriple=arm-apple-darwin    < %s | grep DW_OP_fbreg
; RUN-X64: llc -O0 -mtriple=x86_64-apple-darwin < %s | grep DW_OP_fbreg

Anyway, I think, lit can know host and targets_to_build (and
subtargets) as literals.
With the case above, running llc might be overkill.
> B.) $each_target placeholder, which could be used in the test run
> command to indicate that this is the type 2 test. This placeholder
> will be replaced with the appropriate value at the test run time.
>
> For example, if the test/CodeGen/Generic/2003-07-07-BadLongConst.ll
> will be defined as
>
> ; RUN: llc -march=$each_target < %s
It's cool. Also I suggest matrix stuff, eg.

; $arch = i686,x86_64
; $os = darwin,linux,mingw32,win32
; RUN: llc -mtriple=$arch-$os < %s

To expect running 8 tests.

I wonder it would be too heavyweight. To attempt matrices, we might
need matrix-test mode for llc, or building dedicated-codegen-tester.


Thank you, ...Takumi

Hi Galina!

This is a great thread, and you bring up a lot of interesting topics.

Unfortunately, my attention span is short so I can only pay attention
to a few at a time. Can you break down some of these issues into
separate threads?

Here are my general comments:

(1) Regarding lit, I do not necessarily want to add many features to
the lit framework itself. I think REQUIRES / feature sets are useful,
but I am hesitant to introduce too many notions to lit itself.

Lit is intended to be a "meta" testing framework of sorts, and the
problem with adding features at the top-level is that they either
confuse the UI (when interoperating with other systems which can't
express those features) or they make interoperation more complicated.

(2) Regarding your division of tests:

One thing I don't agree with here is this statements: "These tests
should be run multiple times per build".

This is not quite in the spirit of how we do testing (at least, under
my interpretation).

I see our regressions tests as a form of "unit tests". For almost all
tests, the test should be designed to check some particular unit, we
merely use the command line tools as a convenient way to hit those
feature points.

The problem with says each test should be run multiple times per
build, is you aren't really testing one unit. I agree you are
increasing coverage, but if that coverage matters then you should
ensure that you have tests to cover each unit you care about.

One problem we have on LLVM is that we have conflated the testing
issue with the coverage issue by changing the tests on each target.
This is wrong, and I hope to eliminate it one day. I would like to
eliminate all the places where our testing matrix changes based on the
host (unless necessary).

Note that lit mostly has the infrastructure to do what you want w.r.t.
running multiple tests per config. However, I am not sure I want to
embed this in the tests themselves.

This is a complex topic, it overlaps with lit UI features (which I
have a planned roadmap on, and overlaps with other tasks), the LLVM
testing philosophy, buildbots, etc.

I'd like to move this to a separate thread from concrete other issues.

(3) I agree I don't like XFAIL or XTARGET, I prefer using a form of
REQUIRES for this.

(4) I think I am fine renaming target_triple, if you want to do it
just send a patch separately.

(5) Please let me know if I missed a topic you wanted feedback on.

Thanks for thinking about this stuff! :)
 - Daniel

On Tue, Nov 23, 2010 at 12:16 PM, Galina Kistanova <gkistanova at
gmail.com> wrote:
> Hello everyone,
>
> I'm wrestling with the LLVM unit and regression tests now and would
> like to discuss some changes to make.
> I will be preparing patches, but any input/ideas are welcome.
>
> I leave "where the tests should run" question out for the scope
yet.
> Let's review what kind of tests we have and how to handle them
> correctly.
>
> We have 3 types of unit and regression tests:
>
> 1.) Target-independent tests
>
> These tests should be run once per build and they do not depend on
> what and how many targets LLVM is configured and built for.
>
> For example, lli interpreter tests are the type 1 tests.
>
> 2.) Target-dependent tests
>
> These tests should be run multiple times per build, once for each of
> the targets LLVM is built for. These tests are valid for any supported
> target but need to be configures/started on some target-specific way.
>
> For example, the test/CodeGen/Generic/2003-07-07-BadLongConst.ll test
> is the type 2 test.
>
> 3.) Conditional tests
>
> These tests are subjects to some conditions, like target(s), or
> triple(s), or host platform.
> These tests should be run only if specified conditions are met or
> considered UNSUPPORTED otherwise.
> Both types (1) Target-independent and (2) Target-dependent tests could
> be also type 3 tests.
>
> For example, lli JIT tests are the type 3 tests and should be run only
> for the host platform if it is supported by lli.
> test/DebugInfo/2010-08-04-StackVariable.ll is another example of such
tests.
>
> As it is now, tests are treated as type 1 by default, we do not have
> support for type 2 tests and we define type 3 tests by using
> XTARGET/XFAIL declarations (for example
> llvm/test/FrontendC/2009-01-20-k8.c) and/or DejaGNU configuration
> files for group of tests (like llvm/test/MC/X86/dg.exp).
>
> We have a nice way to keep tests self-declared. Each unit or
> regression test contain 2 parts: declarative part with meta-data of
> how the test should be run and how the result should be interpreted,
> and the test itself. I like this. It allows us to keep all related
> information in one place and make each test quite explicit.
>
> To follow this pattern, I think, we need to extend the declarative
> part by adding an explicit dependency statement. Technically, we need
> to declare a dependency (or a run condition), and for what targets the
> test should be run.
>
> I propose
>
> A.) A new condition statement, something like
>
> ; DEPENDS: <condition>
> or
> ; REQUIRE: <condition>
>
> 0 or many could be declared with any test. If more than one is
> declared, all must be met to run the test, otherwise it gets marked as
> UNSUPPORTED.
>
> For example, the test/DebugInfo/2010-08-04-StackVariable.ll test could
> have the declarative part as
>
> ; REQUIRE: llc -mtriple=arm-apple-darwin    < /dev/null
> ; REQUIRE: llc -mtriple=x86_64-apple-darwin < /dev/null
> ; RUN: llc -O0 -mtriple=arm-apple-darwin    < %s | grep DW_OP_fbreg
> ; RUN: llc -O0 -mtriple=x86_64-apple-darwin < %s | grep DW_OP_fbreg
> ; Use DW_OP_fbreg in variable's location expression if the variable is
> in a stack slot.
>
> B.) $each_target placeholder, which could be used in the test run
> command to indicate that this is the type 2 test. This placeholder
> will be replaced with the appropriate value at the test run time.
>
> For example, if the test/CodeGen/Generic/2003-07-07-BadLongConst.ll
> will be defined as
>
> ; RUN: llc -march=$each_target < %s
>
> it will be executed as
>
> llc -march=arm  <
>
/opt/test-buildslave/osuosl/slave/build-x-4-armeabi-hardfloat/llvm.src/test/CodeGen/Generic/2003-07-07-BadLongConst.ll
> llc -march=thumb  <
>
/opt/test-buildslave/osuosl/slave/build-x-4-armeabi-hardfloat/llvm.src/test/CodeGen/Generic/2003-07-07-BadLongConst.ll
>
> for llvm configured with --enable-targets=arm.
>
> Note that $each_target and targets/llvm_supports_target() are
> different. In the above example $each_target is [arm, thumb] (see llc
> --version), and targets is [ARM].
>
> What do you think?
>
> Few related random notes:
>
> * We can keep the DejaGNU config files (or make similar LIT configs)
> to turn off group of tests.
>
> * XTARGET makes test failure treated as expected. This is something
> different than unsupported tests. Unsupported tests shouldn't even run
> since could crash or behaves unexpectedly. Expected failure test
> should run because they could produce unexpected successful result. Up
> to me, we should deprecate XTARGET, if XFAIL accepts <condition>.
>
> * We can extend declarative language to define variables. In this case
> $each_target could be a test variable instead of a pre-defined
> placeholder.
>
> * target_triple is actually the host_triple. Shell we rename it?
>
>
> Thanks
>
> Galina
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>