[RFC,v2] Test coverage for --param boundary values

On 07/15/2016 09:22 AM, Thomas Schwinge wrote:
> Hi!
> 
> On Fri, 8 Jul 2016 14:47:46 +0200, Martin Liška <mliska@suse.cz> wrote:
>> From f84ce7be4a998089541fb4512e19f54a4ec25cf6 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Fri, 8 Jul 2016 10:59:24 +0200
>> Subject: [PATCH] Add tests that test boundary values of params
> 
> This became r238249.  Yay for pushing testing boundaries; next is some
> fuzzy testing for GCC?  ;-D
> 
>> gcc/ChangeLog:
>>
>> 2016-07-08  Martin Liska  <mliska@suse.cz>
>>
>> 	* Makefile.in: Append rule for params-options.h.
>> 	* params-options.h: New file.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-07-08  Martin Liska  <mliska@suse.cz>
>>
>> 	* gcc.dg/params/blocksort-part.c: New test.
>> 	* gcc.dg/params/params.exp: New file.
> 
> :-/ (GNU-style ChangeLogs are just so useful... not.)
> 
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/params/blocksort-part.c
>> @@ -0,0 +1,706 @@
>> +
>> +/*-------------------------------------------------------------*/
>> +/*--- Block sorting machinery                               ---*/
>> +/*---                                           blocksort.c ---*/
>> +/*-------------------------------------------------------------*/
>> +
>> +/* ------------------------------------------------------------------
>> +   This file is part of bzip2/libbzip2, a program and library for
>> +   lossless, block-sorting data compression.
>> +
>> +   bzip2/libbzip2 version 1.0.6 of 6 September 2010
>> +   Copyright (C) 1996-2010 Julian Seward <jseward@bzip.org>
>> +
>> +   Please read the WARNING, DISCLAIMER and PATENTS sections in the 
>> +   README file.
>> +
>> +   This program is released under the terms of the license contained
>> +   in the file LICENSE.
>> +[...]
> 

Hi Thomas.

> Are there any issues with including that file without the
> accompanying/referenced README and LICENSE files?

Sure, I'll include these files.

> 
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/params/params.exp
>> @@ -0,0 +1,64 @@
>> +[...]
>> +# GCC testsuite that uses the `dg.exp' driver.
>> +
>> +# Load support procs.
>> +load_lib gcc-dg.exp
>> +
>> +# Initialize `dg'.
>> +dg-init
>> +
>> +proc param_run_test { param_name param_value } {
>> +    global srcdir
>> +    global subdir
>> +
>> +    dg-runtest $srcdir/$subdir/blocksort-part.c "" "-O3 --param $param_name=$param_value"
>> +}
>> +
>> +set fd [open "$objdir/../../params.options" r]
> 
> (I do understand what you're doing there, but) is it kosher to refer to a
> file from GCC's build tree inside the test suite?  I thought the idea was
> that you could run the testsuite without the build tree being available
> -- as much that's possible.  As far as I remember there are a few
> exception to this rule already, so maybe adding one more is not much of a
> problem.  ;-) (I do know that for our internal testing we have a list of
> files that need to be preserved from the GCC build directory for later
> testing without the build directory being available; so I'll add this
> file to the list; Joseph CCed in case he has some additional comments
> after returning from his vacations.)
> 
> <http://news.gmane.org/find-root.php?message_id=%3C201607141327.u6EDRKGV028717%40toolchainslave5.caveonetworks.com%3E>
> shows one instance of the problem, that I could quickly find:
> 
>     [...]
>     ERROR: tcl error sourcing /home/jenkins/workspace/BuildToolchainThunder_elf_test_upstream/toolchain/src/gcc/testsuite/gcc.dg/params/params.exp.
>     ERROR: couldn't open "/home/jenkins/workspace/BuildToolchainThunder_elf_test_upstream/toolchain/testresults/aarch64-thunderx-elf/../../params.options": no such file or directory
>     [...]

You are right, I was inspired by what we do for GCC plugins in:
gcc/testsuite/lib/plugin-support.exp

where we have following comment:
    # Note that the plugin test support currently only works when the GCC
    # build tree is available. (We make sure that is the case in plugin.exp.)
    # Once we have figured out how/where to package/install GCC header files
    # for general plugin support, we should modify the following include paths
    # accordingly.

Well, I can imaging a guard which will test whether "$objdir/../../params.options" file exits,
and if so, then the tests are executed? Is it acceptable approach?

> 
>> +set text [read $fd]
>> +close $fd
>> +
>> +# Main loop.
>> +foreach params [split $text "\n"] {
>> +    set parts [split $params "="]
>> +    set name [string trim [lindex $parts 0] '"']
>> +    set values [split [lindex $parts 1] ","]
>> +    if { [llength $values] == 3 } {
>> +	set default [lindex $values 0]
>> +	set min [lindex $values 1]
>> +	set max [lindex $values 2]
>> +	set int_max "INT_MAX"
>> +
>> +	if { $min != -1 } {
>> +	    param_run_test $name $min
>> +	}
>> +	if { $max != $min && $max > 0 && $max != $int_max } {
>> +	    param_run_test $name $max
>> +	}
>> +    }
>> +    if { [llength $values] == 5 } {
>> +	foreach v $values {
>> +	    param_run_test $name $v
>> +	}
>> +    }
>> +}
> 
> This produces ambiguous test result lines:
> 
>     Running /scratch/tschwing/nvidiak20-2/gcc/trunk-light/source-gcc/gcc/testsuite/gcc.dg/params/params.exp ...
>     PASS: gcc.dg/params/blocksort-part.c (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c (test for excess errors)
>     [many more]
> 
> OK to fix this as follows:
> 
>     PASS: gcc.dg/params/blocksort-part.c -O3 --param predictable-branch-outcome=0 (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c -O3 --param predictable-branch-outcome=50 (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c -O3 --param inline-min-speedup=0 (test for excess errors)
>     PASS: gcc.dg/params/blocksort-part.c -O3 --param max-inline-insns-single=0 (test for excess errors)
>     [...]
> 
> commit ae350ce9af062d7d2c05225280596b1bc982728b
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Fri Jul 15 09:13:19 2016 +0200
> 
>     Disambiguate gcc.dg/params/params.exp test results
>     
>     	gcc/testsuite/
>     	* gcc.dg/params/params.exp (param_run_test): Swap flags and
>     	default-extra-flags arguments when calling dg-runtest.
> ---
>  gcc/testsuite/gcc.dg/params/params.exp |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git gcc/testsuite/gcc.dg/params/params.exp gcc/testsuite/gcc.dg/params/params.exp
> index f2f397e..dd82a5d 100644
> --- gcc/testsuite/gcc.dg/params/params.exp
> +++ gcc/testsuite/gcc.dg/params/params.exp
> @@ -28,7 +28,7 @@ proc param_run_test { param_name param_value } {
>      global srcdir
>      global subdir
>  
> -    dg-runtest $srcdir/$subdir/blocksort-part.c "" "-O3 --param $param_name=$param_value"
> +    dg-runtest $srcdir/$subdir/blocksort-part.c "-O3 --param $param_name=$param_value" ""
>  }
>  
>  set fd [open "$objdir/../../params.options" r]
> 
> 
> Grüße
>  Thomas
> 

Thanks for the patch, that's probably the same problem that Andreas spotted.
I'm sending candidate patch.

Martin

[RFC,v2] Test coverage for --param boundary values

Commit Message

Comments

Patch