PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Jagasia, Harsha]

Hi,

Sorry, I forgot to add the below:

This patch is intended to be used for graphite when it gets merged with
mainline. The intent of this patch is to allow a testcase to be
specified with 2 different flag sets. The test will be built and with 2
flag sets and the output of the runs will be compared. If the outputs
match then a pass will be reported, else a fail will be reported. The
purpose is to ensure that there are no failures with graphite. To avoid
making changes to the dejagnu package, some code in dejagnu had to be
replicated in the gcc testsuite with modifications.

Please review and comment if it's ok for mainline?

Thanks,
Harsha

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Janis Johnson]

On Thu, 2008-05-01 at 13:47 -0500, Jagasia, Harsha wrote:
> Hi,
> 
> Sorry, I forgot to add the below:
> 
> This patch is intended to be used for graphite when it gets merged with
> mainline. The intent of this patch is to allow a testcase to be
> specified with 2 different flag sets. The test will be built and with 2
> flag sets and the output of the runs will be compared. If the outputs
> match then a pass will be reported, else a fail will be reported. The
> purpose is to ensure that there are no failures with graphite. To avoid
> making changes to the dejagnu package, some code in dejagnu had to be
> replicated in the gcc testsuite with modifications.
> 
> Please review and comment if it's ok for mainline?

I don't understand the point of the new functionality.  Why isn't
it sufficient to just run the entire testsuite with -fgraphite,
specified as part of RUNTESTFLAGS?  Each test already checks for
its expected result, whatever that might be, and many tests are
already run with multiple sets of options.  Am I missing some
fundamental concept about how Graphite works?

Janis

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[H.J. Lu]

Hi Janis,

It may be useful for some flags which aren't compatible with
others. For example, if I add -msse4 to RUNTESTFLAGS, many x86
specific tests will fail since -msse4 conflicts with dg-options in
them.


H.J.
On Thu, May 1, 2008 at 3:55 PM, Janis Johnson <janis187@us.ibm.com> wrote:
>
> On Thu, 2008-05-01 at 13:47 -0500, Jagasia, Harsha wrote:
>  > Hi,
>  >
>  > Sorry, I forgot to add the below:
>  >
>  > This patch is intended to be used for graphite when it gets merged with
>  > mainline. The intent of this patch is to allow a testcase to be
>  > specified with 2 different flag sets. The test will be built and with 2
>  > flag sets and the output of the runs will be compared. If the outputs
>  > match then a pass will be reported, else a fail will be reported. The
>  > purpose is to ensure that there are no failures with graphite. To avoid
>  > making changes to the dejagnu package, some code in dejagnu had to be
>  > replicated in the gcc testsuite with modifications.
>  >
>  > Please review and comment if it's ok for mainline?
>
>  I don't understand the point of the new functionality.  Why isn't
>  it sufficient to just run the entire testsuite with -fgraphite,
>  specified as part of RUNTESTFLAGS?  Each test already checks for
>  its expected result, whatever that might be, and many tests are
>  already run with multiple sets of options.  Am I missing some
>  fundamental concept about how Graphite works?
>
>  Janis
>
>

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Sebastian Pop]

On Thu, 2008-05-01 at 13:47 -0500, Jagasia, Harsha wrote:
> Hi,
>
> Sorry, I forgot to add the below:
>
> This patch is intended to be used for graphite when it gets merged with
> mainline.

This is not entirely true: the patch has nothing to do with graphite,
and can be used for any code transform.  In particular, we will use
this functionality with graphite flags for random combinations of loop
transforms.

On Thu, May 1, 2008 at 5:55 PM, Janis Johnson <janis187@us.ibm.com> wrote:
>  I don't understand the point of the new functionality.  Why isn't
>  it sufficient to just run the entire testsuite with -fgraphite,
>  specified as part of RUNTESTFLAGS?  Each test already checks for
>  its expected result, whatever that might be, and many tests are
>  already run with multiple sets of options.

The patch from Harsha checks that the observational semantics is
preserved after a transform: it checks that the outputs of the program
are the same with and without a flag.  Compiling the entire testsuite
with and without a flag is not enough and too long: there are too many
irrelevant testcases, some not containing loops, and others that have
no checks for their outputs.  Furthermore there is no way to ask
people to test more than once all the testsuite with different flags.

-- 
Sebastian Pop
AMD - GNU Tools

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Janis Johnson]

On Mon, 2008-05-05 at 14:05 -0500, Sebastian Pop wrote:
> On Thu, May 1, 2008 at 5:55 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> >  I don't understand the point of the new functionality.  Why isn't
> >  it sufficient to just run the entire testsuite with -fgraphite,
> >  specified as part of RUNTESTFLAGS?  Each test already checks for
> >  its expected result, whatever that might be, and many tests are
> >  already run with multiple sets of options.
> 
> The patch from Harsha checks that the observational semantics is
> preserved after a transform: it checks that the outputs of the program
> are the same with and without a flag.  Compiling the entire testsuite
> with and without a flag is not enough and too long: there are too many
> irrelevant testcases, some not containing loops, and others that have
> no checks for their outputs.  Furthermore there is no way to ask
> people to test more than once all the testsuite with different flags.

The same can be done with TORTURE_OPTIONS for a particular set
of execute tests, or by adding to the TORTURE_OPTIONS for
existing sets of execute tests (e.g. those in gcc.dg/torture
and in gcc.c-torture/execute).  Torture options are split into
those used for tests with and without loops, although the logic
for that split would need to change for options whose names
don't include "loop".  TORTURE_OPTIONS can be overridden for a
particular test run, and we occasionally add to the default
TORTURE_OPTIONS.

Janis

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Janis Johnson]

On Mon, 2008-05-05 at 14:05 -0500, Sebastian Pop wrote:
> On Thu, 2008-05-01 at 13:47 -0500, Jagasia, Harsha wrote:
> > Hi,
> >
> > Sorry, I forgot to add the below:
> >
> > This patch is intended to be used for graphite when it gets merged with
> > mainline.
> 
> This is not entirely true: the patch has nothing to do with graphite,
> and can be used for any code transform.  In particular, we will use
> this functionality with graphite flags for random combinations of loop
> transforms.
> 
> On Thu, May 1, 2008 at 5:55 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> >  I don't understand the point of the new functionality.  Why isn't
> >  it sufficient to just run the entire testsuite with -fgraphite,
> >  specified as part of RUNTESTFLAGS?  Each test already checks for
> >  its expected result, whatever that might be, and many tests are
> >  already run with multiple sets of options.
> 
> The patch from Harsha checks that the observational semantics is
> preserved after a transform: it checks that the outputs of the program
> are the same with and without a flag.  Compiling the entire testsuite
> with and without a flag is not enough and too long: there are too many
> irrelevant testcases, some not containing loops, and others that have
> no checks for their outputs.  Furthermore there is no way to ask
> people to test more than once all the testsuite with different flags.

I cleaned up the use of torture options so now it should be easier to
add a suite that combines a specified list of torture options with
another specified list; see gcc/testsuite/objc/execute/execute.exp
for an example.  All of the tests in a directory will be run with all
of those options.  Does that do what you need?

Janis

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Sebastian Pop]

On Thu, May 15, 2008 at 5:44 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> I cleaned up the use of torture options so now it should be easier to
> add a suite that combines a specified list of torture options with
> another specified list; see gcc/testsuite/objc/execute/execute.exp
> for an example.  All of the tests in a directory will be run with all
> of those options.  Does that do what you need?
>

No, this torture infrastructure is complementary to the changes
proposed by Harsha.

With the torture infrastructure one should encode the check for the
correctness of the results in the testcase itself.  This is difficult:
we want a simpler way to do this checking, avoiding to write the check
explicitely in the testcase.  We want to avoid writing testcases like
gcc/testsuite/gcc.dg/vect/vect-25.c where the check is explicitely in
the testcase:

 /* check results:  */
  for (i = 0; i < N; i++)
    {
      if (ib[i] != k)
        abort ();
    }

The patch proposed by Harsha allows to just write the testcase and
compare outputs, avoiding the need of a check of the computation with
abort calls.

-- 
Sebastian Pop
AMD - GNU Tools

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Janis Johnson]

On Fri, 2008-05-16 at 10:41 -0500, Sebastian Pop wrote:
> On Thu, May 15, 2008 at 5:44 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> > I cleaned up the use of torture options so now it should be easier to
> > add a suite that combines a specified list of torture options with
> > another specified list; see gcc/testsuite/objc/execute/execute.exp
> > for an example.  All of the tests in a directory will be run with all
> > of those options.  Does that do what you need?
> >
> 
> No, this torture infrastructure is complementary to the changes
> proposed by Harsha.
> 
> With the torture infrastructure one should encode the check for the
> correctness of the results in the testcase itself.  This is difficult:
> we want a simpler way to do this checking, avoiding to write the check
> explicitely in the testcase.  We want to avoid writing testcases like
> gcc/testsuite/gcc.dg/vect/vect-25.c where the check is explicitely in
> the testcase:
> 
>  /* check results:  */
>   for (i = 0; i < N; i++)
>     {
>       if (ib[i] != k)
>         abort ();
>     }
> 
> The patch proposed by Harsha allows to just write the testcase and
> compare outputs, avoiding the need of a check of the computation with
> abort calls.

Except for tests of library output functions, GCC tests should
be self-checking.  We wouldn't be able to notice new errors if
a test got the same wrong answers when compiled with two sets
of optimizations.

Janis

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Sebastian Pop]

On Thu, May 22, 2008 at 5:22 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> Except for tests of library output functions, GCC tests should
> be self-checking.

What does "self-checking" mean?

>  We wouldn't be able to notice new errors if
> a test got the same wrong answers when compiled with two sets
> of optimizations.
>

I'm not worried about that: if a testcase is already miscompiled at
-O0, we should also miscompile it at "-O2 -fgraphite -foo -fbar -etc"
in the exact same way!  I just want these testcases to produce the
exact same answer whatever compilation flag is passed to gcc.

Sebastian
-- 
AMD - GNU Tools

Re: PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Janis Johnson]

On Thu, 2008-05-22 at 23:44 -0500, Sebastian Pop wrote:
> On Thu, May 22, 2008 at 5:22 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> > Except for tests of library output functions, GCC tests should
> > be self-checking.
> 
> What does "self-checking" mean?

It means that an executable test has code like

 /* check results:  */
  for (i = 0; i < N; i++)
    {
      if (ib[i] != k)
        abort ();
    }

to check that the results are correct, rather than comparing the
output file against expected output.  If the test returns 0 it
passes, otherwise it fails.

> >  We wouldn't be able to notice new errors if
> > a test got the same wrong answers when compiled with two sets
> > of optimizations.
> >
> 
> I'm not worried about that: if a testcase is already miscompiled at
> -O0, we should also miscompile it at "-O2 -fgraphite -foo -fbar -etc"
> in the exact same way!  I just want these testcases to produce the
> exact same answer whatever compilation flag is passed to gcc.

I just want all testcases that fail to report failures.

Do you plan to write new tests for Graphite, or to use existing
tests and run them multiple times, or both?

Janis

PATCH 01: Testsuite: compare outputs of a testcase compiled and run with 2 flags

[Harsha Jagasia]

	* gcc.dg/tree-ssa/runcmp-example.c: New testcase.
	* lib/gcc-dg.exp (gcc-get-options, gcc-options-runcmp,
	chk_compile_time_err, chk_run_time_status): New.
	(gcc-dg-test-1): Find the gcc-* command comments in the testcase.
	Execute each of these gcc-* commands.  Implements the runcmp command.

Index: gcc.dg/tree-ssa/runcmp-example.c
===================================================================
--- gcc.dg/tree-ssa/runcmp-example.c	(revision 0)
+++ gcc.dg/tree-ssa/runcmp-example.c	(revision 0)
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { gcc-options-runcmp "-O2" "-O3" } */
+
+#include <stdio.h>
+double u[178];
+int foo(int N)
+{
+  int i;
+  int sum = 0;
+  for (i = 0; i < 178; i++)
+    {
+      u[i] = rand();
+      sum += u[i];
+      
+    }
+  sum = sum + N;
+  return sum;
+}
+
+int main()
+{
+  int sum = 0;
+  sum = foo (10);
+  printf("sum:%d\n", sum);
+  return 0;
+}
Index: lib/gcc-dg.exp
===================================================================
--- lib/gcc-dg.exp	(revision 134836)
+++ lib/gcc-dg.exp	(working copy)
@@ -27,6 +27,7 @@ load_lib prune.exp
 load_lib libgloss.exp
 load_lib target-libpath.exp
 
+
 # We set LC_ALL and LANG to C so that we get the same error messages as expected.
 setenv LC_ALL C
 setenv LANG C
@@ -75,12 +76,105 @@ foreach option $TORTURE_OPTIONS {
     }
 }
 
+proc gcc-get-options { prog } {
+    set result ""
+
+    set tmp [grep $prog "{\[ \t\]\+gcc-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line]
+    if ![string match "" $tmp] {
+	foreach i $tmp {
+	    regexp "(\[0-9\]\+)\[ \t\]\+{\[ \t\]+(gcc-\[-a-z\]+)\[ \t\]\+(.*)\[ \t\]+}\[^\}\]*(\n|$)" $i i line cmd args
+	    append result " { $cmd $line $args }"
+	}
+    }
+
+    return $result
+}
+
+proc gcc-options-runcmp { args } {
+
+    upvar flags1 flags1
+    upvar flags2 flags2
+
+    if { [llength $args] > 4 } {
+	error "[lindex $args 0]: too many arguments"
+	return
+    }
+    # FIXME: For now no target filter. Add support for target filter
+    # specified in test case
+    set flags1 [lindex $args 1]
+    set flags2 [lindex $args 2]
+}
+
+proc chk_compile_time_err { comp_output name flags } {
+    if [string match "*internal compiler error*" $comp_output] {
+	fail "$name (internal compiler error) $flags"
+    }
+}
+
+proc chk_run_time_status { output_file flags do_exp dg-output-text name} {
+
+    global tool
+
+    if ![file exists $output_file] {
+	warning "$name compilation with $flags failed to produce executable"
+    } else {
+	set status -1
+	set result [${tool}_load $output_file]
+	set status [lindex $result 0]
+	set output [lindex $result 1]
+	
+	if { $do_exp == "F" } {
+	    setup_xfail "*-*-*"
+	}
+	if { "$status" == "pass" } {
+	    pass "$name with $flags execution test"
+	    verbose "Exec with $flags succeeded." 3
+
+	    if { [llength ${dg-output-text}] > 1 } {
+		if { [lindex ${dg-output-text} 0] == "F" } {
+		    setup_xfail "*-*-*"
+		}
+		set texttmp [lindex ${dg-output-text} 1]
+		if { ![regexp $texttmp ${output}] } {
+		    fail "$name with $flags output pattern test, is ${output}, should match $texttmp"
+		    verbose "Failed test with $flags for output pattern $texttmp" 3
+		} else {
+		    pass "$name with $flags output pattern test, $texttmp"
+		    verbose "Passed test with $flags for output pattern $texttmp" 3
+		}
+		unset texttmp
+	    }
+	} elseif { "$status" == "fail" } {
+	    # It would be nice to get some info out of errorCode.
+	    if [info exists errorCode] {
+		verbose "Exec failed with $flags, errorCode: $errorCode" 3
+	    } else {
+		verbose "Exec failed with $flags, errorCode not defined!" 3
+	    }
+	    fail "$name with $flags execution test"
+	} else {
+	    $status "$name with $flags execution test"
+	}
+    }
+    
+    return $result
+}
+
+
 # Define gcc callbacks for dg.exp.
 
 proc gcc-dg-test-1 { target_compile prog do_what extra_tool_flags } {
     # Set up the compiler flags, based on what we're going to do.
 
+    global srcdir
+    global dg-do-what-default
+    # `dg-output-text' is a list of two elements: pass/fail and text.
+    # Leave second element off for now (indicates "don't perform test")
+    # FIXME: For now dg-output-text is a constant. Handle dg-output-text
+    # specified in test case.
+    set dg-output-text "P"
     set options [list]
+    set dg-do-what [list ${dg-do-what-default} "" P]
 
     # Tests should be able to use "dg-do repo".  However, the dg test
     # driver checks the argument to dg-do against a list of acceptable
@@ -90,6 +184,49 @@ proc gcc-dg-test-1 { target_compile prog
 	set do_what "repo"
     }
 
+    # Define our own "special function" `unknown' so we catch spelling errors.
+    # But first rename the existing one so we can restore it afterwards.
+    catch {rename dg-save-unknown ""}
+    rename unknown dg-save-unknown
+    proc unknown { args } {
+	return -code error "unknown dg option: $args"
+    }
+
+    set name [dg-trim-dirname $srcdir $prog]
+    # If we couldn't rip $srcdir out of `prog' then just do the best we can.
+    # The point is to reduce the unnecessary noise in the logs.  Don't strip
+    # out too much because different testcases with the same name can confuse
+    # `test-tool'.
+    if [string match "/*" $name] {
+	set name "[file tail [file dirname $prog]]/[file tail $prog]"
+    }
+
+    # Retreive flags to compare from testcase
+    set tmp [gcc-get-options $prog]
+    foreach op $tmp {
+	verbose "Processing option: $op" 3
+	set status [catch "$op" errmsg]
+	if { $status != 0 } {
+	    if ![info exists errorInfo] {
+		perror "$name: $errmsg for \"$op\"\n"
+	    }
+	    unresolved "$errmsg for \"$op\""
+	    return
+	}
+    }
+
+   # Restore normal error handling.
+    rename unknown ""
+    rename dg-save-unknown unknown
+
+    # Tests should be able to use "dg-do runcmp".  However, the dg test
+    # driver checks the argument to dg-do against a list of acceptable
+    # options, and "runcmp" is not among them.  Therefore, we resort to
+    # this ugly approach.
+    if { [info exists flags1] && [info exists flags2] && [string length $flags1] > 1 && [string length $flags2] > 1} {
+	set do_what "runcmp"
+    }
+
     switch $do_what {
 	"preprocess" {
 	    set compile_type "preprocess"
@@ -127,36 +264,100 @@ proc gcc-dg-test-1 { target_compile prog
 	    # created or not.  If it was, dg.exp will try to run it.
 	    catch { remote_file build delete $output_file }
 	}
+	"runcmp" {
+
+	    set compile_type "executable"
+
+	    set output_file_1 "./[file rootname [file tail $prog]]-a.exe"
+	    catch { remote_file build delete $output_file_1 }
+
+	    set output_file_2 "./[file rootname [file tail $prog]]-b.exe"
+	    catch { remote_file build delete $output_file_2 }
+	}
 	default {
 	    perror "$do_what: not a valid dg-do keyword"
 	    return ""
 	}
     }
 
-    if { $extra_tool_flags != "" } {
-	lappend options "additional_flags=$extra_tool_flags"
-    }
+    if { $do_what != "runcmp" } {
+	if { $extra_tool_flags != "" } {
+	    lappend options "additional_flags=$extra_tool_flags"
+	}
 
-    set comp_output [$target_compile "$prog" "$output_file" "$compile_type" $options]
+	set comp_output [$target_compile "$prog" "$output_file" "$compile_type" $options]
 
-    # Look for an internal compiler error, which sometimes masks the fact
-    # that we didn't get an expected error message.  An ICE always fails,
-    # there's no way to XFAIL it.
-    if [string match "*internal compiler error*" $comp_output] {
-	upvar 2 name name
-	fail "$name (internal compiler error)"
-    }
+	# Look for an internal compiler error, which sometimes masks the fact
+	# that we didn't get an expected error message.  An ICE always fails,
+	# there's no way to XFAIL it.
+	if [string match "*internal compiler error*" $comp_output] {
+	    upvar 2 name name
+	    fail "$name (internal compiler error)"
+	}
 
-    if { $do_what == "repo" } {
-	set object_file "$output_file"
-	set output_file "[file rootname [file tail $prog]].exe"
-	set comp_output \
-	    [ concat $comp_output \
-		  [$target_compile "$object_file" "$output_file" \
-		       "executable" $options] ]
-    }
+	if { $do_what == "repo" } {
+	    set object_file "$output_file"
+	    set output_file "[file rootname [file tail $prog]].exe"
+	    set comp_output \
+		[ concat $comp_output \
+		      [$target_compile "$object_file" "$output_file" \
+			   "executable" $options] ]
+	}
 
-    return [list $comp_output $output_file]
+	return [list $comp_output $output_file]
+
+    } else {
+
+	global dg-interpreter-batch-mode
+	upvar output_1 output_1
+	upvar output_2 output_2
+	upvar status_1 status_1
+	upvar status_2 status_2
+
+	set comp_output_1 [$target_compile "$prog" "$output_file_1" "$compile_type" additional_flags=$flags1]
+	set comp_output_2 [$target_compile "$prog" "$output_file_2" $compile_type additional_flags=$flags2]
+
+	chk_compile_time_err $comp_output_1 $name $flags1
+	chk_compile_time_err $comp_output_2 $name $flags2
+
+	# Compare the compilation output of 2 compiles
+	if [string compare $comp_output_1 $comp_output_2] {
+	    fail "$name comparing output of compilation with $flags1 and $flags2"
+	}
+
+	set results_1 [chk_run_time_status $output_file_1 $flags1 [lindex ${dg-do-what} 2] ${dg-output-text} $name]
+	set results_2 [chk_run_time_status $output_file_2 $flags2 [lindex ${dg-do-what} 2] ${dg-output-text} $name]
+
+	set status_1 [lindex $results_1 0]
+	set output_1 [lindex $results_1 1]
+
+	set status_2 [lindex $results_2 0]
+	set output_2 [lindex $results_2 1]
+
+	# Compare the status of 2 runs
+	if [string compare $status_1 $status_2] {
+	    fail "$name comparing status of runs with $flags1 and $flags2"
+	}
+
+	# Compare the output 2 runs
+	set message "$name comparing output of runs with $flags1 and $flags2"
+	if [string compare $output_1 $output_2] {
+	    fail $message
+	} else {
+	    pass $message
+	}
+
+	# Do some final clean up.
+	# When testing an interpreter, we don't compile something and leave an
+	# output file.
+	if { ${dg-interpreter-batch-mode} == 0 } {
+	    #We only delete one output_file, the other is deleted in 
+	    #the dejagnu caller.
+	    catch "exec rm -f $output_file_2"
+	}
+
+	return [list $comp_output_1 $output_file_1]
+    }
 }
 
 proc gcc-dg-test { prog do_what extra_tool_flags } {