[testsuite, ada] Convert ACATS to DejaGnu (PR testsuite/37703)

[Rainer Orth]

Mike Stump <mikestump@comcast.net> writes:

> On Jan 31, 2011, at 8:24 AM, Rainer Orth wrote:
>> As I've mentioned before, I've long meant to convert the ACATS testsuite
>> to DejaGnu to provide all dg features (timeouts, xfails, multilib
>> support, don't copy all sources into the build dir).  After the gnat.dg
>> cleanup patch (and requiring it), here's the first version of a patch
>> that does this.  It is by no means ready or polished, but I'm asking for
>> general comments, suggestions for fixes or improvements, and testers.
>
> So, there is a good chance this just doesn't work with canadian crosses...
>
> So, lines like:
>
>> +    file mkdir tests
>> +    file mkdir support
>
> tend to be wrong.  The build machine, typically doesn't have much on it, and so there are few filesystem operations that apply to it.  Typically, most go to the host or the target.  See remote_file from testsuite/*.exp for ideas.

This is exactly the kind of information I'm looking for: which
operations are supposed to run on the build, host and target systems and
do those three (or only build and host) share a common view of the
filesystem (either in general or as a GCC-specific restriction in
particular)?

> Now, that said, if people have never tested that way for this testssuite, well, you can skip that as a feature before putting it in, but... at some point, it would be nice to set up a cross and test it, and a canadian cross and test it.

Right: this might be even something as simple as a cross from
Solaris/SPARC to Solaris/x86: this should catch many issues already.

> +    # FIXME: How to properly locate host CC?
> +    # Place into support subdir.
> +    remote_exec host [which gcc] "-c $srcdir/cd/cd300051.c -o support/cd300051.o"
>
> This is non-sensical.  You want the path on the local machine, which is unrelated to the host machine and unrelated to the target machine, and then you want to use that on an unrelated machine...  Oops.  You do this systematically though out the patch.

That's what I feared and why I was asking for review/comment.  I was
trying to redo what the current non-dg ACATS testsuite is doing, but it
doesn't deal with the canadian-cross case.

> if ![is_remote host] you can do this, this way, otherwise, this won't work.  See default_gcc_version in gcc.exp for example.  Now, that said, this will work 99% of the time, so you can do that to start as expedient, but it is still wrong.  Using just a simple tool name would be better, then, just document that the testing environment must contain that command in the path for the testing to work.

Again, I'd rather get this right from the start, rather than rush in
something that's only 70 (or even 90)% ready.

> +# FIXME: CFLAGS seems wrong for Ada, cf. gcc/ada/Make*.
>
> No, CFLAGS is fine, the C stands for compilation, don't you know...  :-)  I think this is right, if you want to retain the functionality of the rest of the infrastructure that can set DEFAULT_CFLAGS.

I don't think this is compilation flags in general: AFAIK this is from
the default make rules, and they have separate CFLAGS, FFLAGS, PFLAGS
and several more.

> +    # FIXME: Better use set_board_info, unset_board_info from targetdb.exp?
> +    # Or global var or uplevel N info proc ...?
> +    set board_info(gnat,acats) 1
>
> +    # FIXME: What's right here, target_info or board_info?
> +    if [info exists board_info(gnat,acats)] {
> +	# Pass -Isupport to locate support files.
> +	# FIXME: Why -gnatws?
> +	lappend options "additional_flags=-Isupport -gnatws"
> +    }
>
> Kinda a nasty way to do this...  Others have code like:
>
>     if { [istarget "i?86-*-*"]
>          || [istarget "m68k-*-*"] } then {
>       uplevel 1 lappend additional_flags "-ffloat-store"
>
> or
>
>     if { [string match "*-*-darwin*" $target_triplet] } {
>         lappend ALWAYS_CXXFLAGS "ldflags=-multiply_defined suppress"
>        }

But this is per-target, not per (section of a) testsuite.

> to set up specific flags for the compiler or for ld.  I'd think you could just set a normal variable and use that variable down below...  Don't see the need to drop into board_info, as this data isn't about the board.  It is about the testcase.

Again, I couldn't find a good explanation when to use board_info, when
target_info, ...

> +	switch -glob $basename {
> +	    "cxb30*" {
> +		# FIXME: Handle via
> +		# -- { dg-additional-files "cxb30040.o cxb30060.o cxb30130.o cxb30131.o" }
> +		# but already passes without!?
>
> This is nasty.  Generally we put in .x files, that if present, they are run, and they can do whatever you need to.

I'm using dg-additional-sources now.  The version of the patch I sent
was quite early.

> +    switch $target_triplet {
> +	"ia64*-*-*" -
> +	"s390*-*-*" {
> +	    set target_insn "nop 0"
> +	}
> +	"mmix-*-*" {
> +	    set target_insn "swym 0"
> +	}
> +	default {
> +	    set target_insn "nop"
> +	}
> +    }
>
> This is nasty...  I'm sure you know why...  I think breaking this out to a target.exp, and then running it here, would be better.

Certainly more general than embedding it in one particular testsuite.

> You can have both infrastructures side by side and leave intact the old method.  You can wire up testing to run both methods, and report differences.  Twice the testing time, but trivial to spot unintentional divergences, and possibly a whole lot more testing.  If you do this, you can have a single flag you can flip to use the old scheme, the new scheme, or both schemes.  With that, you can push the changes into the repo with the new scheme off (old on), and not worry about impacting people.  People that want to help test but are two lazy to download your patch, can just flip it on.  Also, if we weren't in stage4, you could flip it on for a week with both running, watch the results mailing lists, and see if there were any faults.

Ok: my latest patch just ripped out the old check-acats target, but I
can easily leave it in.  There may well be issues since the dg
infrastructure will do multilib testing whereas the old one only did the
default multilib.

> I think the most progress will be made the fastest, when you just start putting it in the tree with it on, or both on.  The only problem with that right now, is the poor timing wrt the release.  If you can build consensus with the Ada people and possibly the RMs on timing issues, you could run experiments in trunk, but...  I think I'd say don't even try.

I didn't even consider that and still don't :-)

> I would encourage you, if there is a way to do the changes to be acceptable to the Ada folks in terms of safety (can be mitigated by interested parties trying it out), on put all the base infrastructure in and off (for now) and encourage people to turn it on in their environments.  If it prove safe enough to the Ada folks, you could then seek to flip the switch.

Yup: I'll do it this way once trunk opens again for 4.7 if I'm satisfied
the patch is in a reasonable state.

> Also, be sure to do a make -j16 check to ensure it works (or have someone else do this)...  easy for it not to, and by the time you flip it on, you'd want that to be a known to work feature.

That has been one of my goals from the beginning: the current
parallelism os check-gnats is too small for many of my systems
(especially the SPARC boxes), and I even run make -j16 -k check on my
laptop (4 core Core-i7, hyperthreading enabled).  I was happy to notice
that testing both multilibs on Solaris 11/x86 with my patch didn't take
twice as long as the current testsuite, so this is going in the right
direction.

> The Ada people will have to weigh in on the addition of -O2 to the files in the testsuite.

Sure: I'll see how to handle this properly.

> Otherwise, looks like a reasonable start.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University