public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* review of graphite testsuite patch
@ 2008-08-20 22:24 Janis Johnson
  2008-08-21 19:19 ` Sebastian Pop
  0 siblings, 1 reply; 4+ messages in thread
From: Janis Johnson @ 2008-08-20 22:24 UTC (permalink / raw)
  To: sebpop; +Cc: gcc-patches

This review of the testsuite changes for graphite is based on

  http://cri.ensmp.fr/people/pop/gcc/1142_testsuite.diff

The tests all use -fdump-tree-graphite-all and then look at and
remove only the dump file with "graphite" in the name.  Is there a
different dump option to get just that one file?  If not and there are
additional dump files generated, make sure they are also cleaned up.

Procedure scan-graphite-dump-times looks for "Graphite loop
optimizations cannot be used"; would that message be given only for
particular code, or for any compilation by a particular compiler?
If the latter then there should instead be a new effective-target
keyword "graphite", enabled by adding check_effective_target_graphite
in target-supports.exp similar to the other procedures in that file.
The whole directory of tests could be skipped by having graphite.exp
return early if "graphite" is not true.  This would also mean that
the graphite tests can use scan-tree-dump-files instead of a special
scan procedure.

If scan-graphite-dump-times is needed then it should support XFAIL
and a target list, like the other scan functions.  See scan-dump in
scandump.exp for how this is done.

All of the tests use "dg-do compile".  You could set dg-do-what-default
to "compile" and not need to use dg-do within the tests.  This is just
a suggestion.

Almost all tests use the same options.  Within the graphite.exp files
in the test suites you could set GRAPHITE_FLAGS to the options that are
used for most tests and pass that, instead of DEFAULT_CFLAGS, to
dg-runtest.  Those could be overridden in individual tests.  Again,
this is just a suggestion.

Test gfortran.dg/graphite/scop-1.f doesn't do any checking, what is
its purpose?

Janis


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: review of graphite testsuite patch
  2008-08-20 22:24 review of graphite testsuite patch Janis Johnson
@ 2008-08-21 19:19 ` Sebastian Pop
  2008-08-21 20:47   ` Janis Johnson
  0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Pop @ 2008-08-21 19:19 UTC (permalink / raw)
  To: janis187; +Cc: gcc-patches, rajagopal, dwarak

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On Wed, Aug 20, 2008 at 4:40 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> This review of the testsuite changes for graphite is based on
>

Thanks for the review.

> The tests all use -fdump-tree-graphite-all and then look at and
> remove only the dump file with "graphite" in the name.  Is there a
> different dump option to get just that one file?  If not and there are
> additional dump files generated, make sure they are also cleaned up.
>

There are no dump files other than "*.graphite" that are generated.

> Procedure scan-graphite-dump-times looks for "Graphite loop
> optimizations cannot be used"; would that message be given only for
> particular code, or for any compilation by a particular compiler?

This message is emitted only when GCC does not have the graphite
framework built in, and when one of the options -fgraphite, -floop-*
are used.

> If the latter then there should instead be a new effective-target
> keyword "graphite", enabled by adding check_effective_target_graphite
> in target-supports.exp similar to the other procedures in that file.
> The whole directory of tests could be skipped by having graphite.exp
> return early if "graphite" is not true.  This would also mean that
> the graphite tests can use scan-tree-dump-files instead of a special
> scan procedure.
>
> If scan-graphite-dump-times is needed then it should support XFAIL
> and a target list, like the other scan functions.  See scan-dump in
> scandump.exp for how this is done.
>
> All of the tests use "dg-do compile".  You could set dg-do-what-default
> to "compile" and not need to use dg-do within the tests.  This is just
> a suggestion.
>

Attached is a patch that fixes the testsuite according to these
comments.  Committed to graphite-branch.

> Almost all tests use the same options.  Within the graphite.exp files
> in the test suites you could set GRAPHITE_FLAGS to the options that are
> used for most tests and pass that, instead of DEFAULT_CFLAGS, to
> dg-runtest.  Those could be overridden in individual tests.  Again,
> this is just a suggestion.

This is not yet fixed.

>
> Test gfortran.dg/graphite/scop-1.f doesn't do any checking, what is
> its purpose?
>

scop-1.f was a bug that we reduced.  We test it for compilation only,
but we can also write a test for the number of SCoPs it contains.

Dwarak and Sebastian

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 1168_testsuite.diff --]
[-- Type: text/x-diff; name=1168_testsuite.diff, Size: 16257 bytes --]

2008-08-20  Dwarakanath Rajagopal  <dwarak.rajagopal@amd.com>
	    Sebastian Pop  <sebastian.pop@amd.com>

	* testsuite/lib/target-supports.exp
	(check_effective_target_fgraphite): New.

	* testsuite/gcc.dg/graphite/graphite.exp: Early exit when
	check_effective_target_fgraphite returns false.
	Set dg-do-what-default to compile.
	(scan-graphite-dump-times): Removed.
	* testsuite/gfortran.dg/graphite/graphite.exp: Same.

	* testsuite/gcc.dg/graphite/scop-0.c: Do not use "dg-do compile".
	Use scan-tree-dump-times instead of scan-graphite-dump-times.
	* testsuite/gcc.dg/graphite/scop-1.c: Same.
	* testsuite/gcc.dg/graphite/scop-2.c: Same.
	* testsuite/gcc.dg/graphite/scop-3.c: Same.
	* testsuite/gcc.dg/graphite/scop-4.c: Same.
	* testsuite/gcc.dg/graphite/scop-5.c: Same.
	* testsuite/gcc.dg/graphite/scop-6.c: Same.
	* testsuite/gcc.dg/graphite/scop-7.c: Same.
	* testsuite/gcc.dg/graphite/scop-8.c: Same.
	* testsuite/gcc.dg/graphite/scop-9.c: Same.
	* testsuite/gcc.dg/graphite/scop-10.c: Same.
	* testsuite/gcc.dg/graphite/scop-11.c: Same.
	* testsuite/gcc.dg/graphite/scop-12.c: Same.
	* testsuite/gcc.dg/graphite/scop-13.c: Same.
	* testsuite/gcc.dg/graphite/scop-matmult.c: Same.
	* testsuite/gcc.dg/graphite/scop-14.c: Same.
	* testsuite/gcc.dg/graphite/scop-15.c: Same.
	* testsuite/gcc.dg/graphite/block-0.c: Same.
	* testsuite/gcc.dg/graphite/scop-16.c: Same.
	* testsuite/gcc.dg/graphite/block-1.c: Same.
	* testsuite/gcc.dg/graphite/scop-17.c: Same.
	* testsuite/gcc.dg/graphite/scop-18.c: Same.
	* testsuite/gfortran.dg/graphite/block-1.f90: Same.
	* testsuite/gfortran.dg/graphite/scop-1.f: Same.
	* testsuite/gfortran.dg/graphite/block-2.f: Same.

Index: testsuite/gcc.dg/graphite/scop-0.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-0.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-0.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 int foo (void);
@@ -22,3 +21,4 @@ int toto()
 
 /* { dg-final { scan-tree-dump-times "number of SCoPs: 1" 1 "graphite"} } */ 
 /* { dg-final { cleanup-tree-dump "graphite" } } */
+
Index: testsuite/gcc.dg/graphite/scop-1.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-1.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-1.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/scop-2.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-2.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-2.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/scop-3.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-3.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-3.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 int toto()
@@ -27,5 +26,5 @@ int toto()
   return a[3][5] + b[1];
 }
 
-/* { dg-final { scan-graphite-dump-times "number of SCoPs: 3" 1 "graphite"} } */ 
+/* { dg-final { scan-tree-dump-times "number of SCoPs: 3" 1 "graphite"} } */ 
 /* { dg-final { cleanup-tree-dump "graphite" } } */
Index: testsuite/gcc.dg/graphite/scop-4.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-4.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-4.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-5.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-5.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-5.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-6.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-6.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-6.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/scop-7.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-7.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-7.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/scop-8.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-8.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-8.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 int bar (void);
Index: testsuite/gcc.dg/graphite/scop-9.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-9.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-9.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/graphite.exp
===================================================================
--- testsuite/gcc.dg/graphite/graphite.exp	(revision 139047)
+++ testsuite/gcc.dg/graphite/graphite.exp	(working copy)
@@ -19,59 +19,21 @@
 # Load support procs.
 load_lib gcc-dg.exp
 
+if ![check_effective_target_fgraphite] {
+  return
+}
+
+# The default action for a test is 'compile'.  Save current default.
+global dg-do-what-default
+set save-dg-do-what-default ${dg-do-what-default}
+set dg-do-what-default compile
+
 # If a testcase doesn't have special options, use these.
 global DEFAULT_CFLAGS
 if ![info exists DEFAULT_CFLAGS] then {
     set DEFAULT_CFLAGS " -ansi -pedantic-errors"
 }
 
-
-# Call pass if pattern is present given number of times, otherwise fail.
-# Argument 0 is the regexp to match
-# Argument 1 is number of times the regexp must be found
-# Argument 2 is the name of the dumped tree pass
-# Argument 3 handles expected failures and the like
-proc scan-graphite-dump-times { args } {
-
-    if { [llength $args] < 3 } {
-	error "scan-graphite-dump: too few arguments"
-	return
-    }
-    if { [llength $args] >= 4 } {
-	error "scan-graphite-dump: too many arguments"
-	return
-    }
-
-    # This assumes that we are three frames down from dg-test, and that
-    # it still stores the filename of the testcase in a local variable "name".
-    # A cleaner solution would require a new DejaGnu release.
-    upvar 2 name testcase
-
-    set suf [dump-suffix "\[0-9\]\[0-9\]\[0-9\]t.[lindex $args 2]"]
-    set testname "$testcase scan-graphite-dump-times $suf \"[lindex $args 0]\" [lindex $args 1]"
-    set src [file tail [lindex $testcase 0]]
-    set output_file "[glob -nocomplain $src.\[0-9\]\[0-9\]\[0-9\]t.[lindex $args 2]]"
-    if { $output_file == "" } {
-	fail "$testname: dump file does not exist"
-	return
-    }
-
-    set fd [open $output_file r]
-    set text [read $fd]
-    close $fd
-
-    if { [llength [regexp -inline -all -- [lindex $args 0] $text]] == [lindex $args 1]} {
-        pass "$testname"
-    } else {
-	if { [llength [regexp -inline -all -- "Graphite loop optimizations cannot be used" $text]] >= 1} {
-	    pass "$testname"
-	} else {
-	    fail "$testname"
-	}
-    }
-}
-
-
 # Initialize `dg'.
 dg-init
 
@@ -79,5 +41,8 @@ dg-init
 dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
 	"" $DEFAULT_CFLAGS
 
+# Clean up.
+set dg-do-what-default ${save-dg-do-what-default}
+
 # All done.
 dg-finish
Index: testsuite/gcc.dg/graphite/scop-10.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-10.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-10.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar (void);
Index: testsuite/gcc.dg/graphite/scop-11.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-11.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-11.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-12.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-12.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-12.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-13.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-13.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-13.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-matmult.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-matmult.c	(revision 139327)
+++ testsuite/gcc.dg/graphite/scop-matmult.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 float A[1000][1000], B[1000][1000], C[1000][1000];
Index: testsuite/gcc.dg/graphite/scop-14.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-14.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-14.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 void bar ();
Index: testsuite/gcc.dg/graphite/scop-15.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-15.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-15.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -fgraphite -fdump-tree-graphite-all" } */
 
 #  define EXTERN(type, array)  extern type array[]
Index: testsuite/gcc.dg/graphite/block-0.c
===================================================================
--- testsuite/gcc.dg/graphite/block-0.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/block-0.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O -floop-block -fdump-tree-graphite-all" } */
 
 #define N 1000
@@ -22,5 +21,5 @@ main()
   return toto();
 }
 
-/* { dg-final { scan-graphite-dump-times "Loop blocked" 1 "graphite"} } */ 
+/* { dg-final { scan-tree-dump-times "Loop blocked" 1 "graphite"} } */ 
 /* { dg-final { cleanup-tree-dump "graphite" } } */
Index: testsuite/gcc.dg/graphite/scop-16.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-16.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-16.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } */
 #define N 10000
 void foo (int);
@@ -22,5 +21,5 @@ int test ()
 }
 
 /* Interchange is legal for loops 0 and 1 of the first two SCoPs */
-/* { dg-final { scan-graphite-dump-times "Interchange valid for loops 0 and 1:" 2 "graphite"} } */
+/* { dg-final { scan-tree-dump-times "Interchange valid for loops 0 and 1:" 2 "graphite"} } */
 /* { dg-final { cleanup-tree-dump "graphite" } } */
Index: testsuite/gcc.dg/graphite/block-1.c
===================================================================
--- testsuite/gcc.dg/graphite/block-1.c	(revision 139337)
+++ testsuite/gcc.dg/graphite/block-1.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } */
 
 #define MAX 8192
@@ -28,5 +27,5 @@ int main()
   return sum;
 }
 
-/* { dg-final { scan-graphite-dump-times "Loop blocked" 3 "graphite"} } */ 
+/* { dg-final { scan-tree-dump-times "Loop blocked" 3 "graphite"} } */ 
 /* { dg-final { cleanup-tree-dump "graphite" } } */
Index: testsuite/gcc.dg/graphite/scop-17.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-17.c	(revision 139047)
+++ testsuite/gcc.dg/graphite/scop-17.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } */
 #define N 10000
 void foo (int);
@@ -21,5 +20,5 @@ int test ()
 }
 
 /* Interchange is not legal for loops 0 and 1 of SCoP 2.  */
-/* { dg-final { scan-graphite-dump-times "Interchange not valid for loops 0 and 1:" 1 "graphite"} } */
+/* { dg-final { scan-tree-dump-times "Interchange not valid for loops 0 and 1:" 1 "graphite"} } */
 /* { dg-final { cleanup-tree-dump "graphite" } } */
Index: testsuite/gcc.dg/graphite/scop-18.c
===================================================================
--- testsuite/gcc.dg/graphite/scop-18.c	(revision 139327)
+++ testsuite/gcc.dg/graphite/scop-18.c	(working copy)
@@ -1,4 +1,3 @@
-/* { dg-do compile } */ 
 /* { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } */
 
 #define N 24
Index: testsuite/lib/target-supports.exp
===================================================================
--- testsuite/lib/target-supports.exp	(revision 139047)
+++ testsuite/lib/target-supports.exp	(working copy)
@@ -554,6 +554,15 @@ proc check_effective_target_tls_runtime 
     }]
 }
 
+# Return 1 if compilation with -fgraphite is error-free for trivial 
+# code, 0 otherwise.
+
+proc check_effective_target_fgraphite {} {
+    return [check_no_compiler_messages fgraphite object {
+	void foo (void) { }
+    } "-fgraphite"]
+}
+
 # Return 1 if compilation with -fopenmp is error-free for trivial
 # code, 0 otherwise.
 
Index: testsuite/gfortran.dg/graphite/block-1.f90
===================================================================
--- testsuite/gfortran.dg/graphite/block-1.f90	(revision 139337)
+++ testsuite/gfortran.dg/graphite/block-1.f90	(working copy)
@@ -1,4 +1,3 @@
-! { dg-do compile } 
 ! { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } 
 
 subroutine matrix_multiply(a,b,c,n)
Index: testsuite/gfortran.dg/graphite/scop-1.f
===================================================================
--- testsuite/gfortran.dg/graphite/scop-1.f	(revision 139047)
+++ testsuite/gfortran.dg/graphite/scop-1.f	(working copy)
@@ -1,4 +1,3 @@
-C { dg-do compile }
 C { dg-options "-O2 -fgraphite" }
 
       dimension p1(2),t(6,4),b1(2),b2(2),al1(2),al2(2),g1(2),g2(2)
Index: testsuite/gfortran.dg/graphite/block-2.f
===================================================================
--- testsuite/gfortran.dg/graphite/block-2.f	(revision 139337)
+++ testsuite/gfortran.dg/graphite/block-2.f	(working copy)
@@ -1,4 +1,3 @@
-! { dg-do compile } 
 ! { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } 
 
       SUBROUTINE MATRIX_MUL_UNROLLED (A, B, C, L, M, N)
Index: testsuite/gfortran.dg/graphite/graphite.exp
===================================================================
--- testsuite/gfortran.dg/graphite/graphite.exp	(revision 139047)
+++ testsuite/gfortran.dg/graphite/graphite.exp	(working copy)
@@ -19,6 +19,15 @@
 # Load support procs.
 load_lib gfortran-dg.exp
 
+if ![check_effective_target_fgraphite] {
+  return
+}
+
+# The default action for a test is 'compile'.  Save current default.
+global dg-do-what-default
+set save-dg-do-what-default ${dg-do-what-default}
+set dg-do-what-default compile
+
 # If a testcase doesn't have special options, use these.
 set DEFAULT_GRAPHITE_FLAGS ""
 
@@ -32,6 +41,8 @@ gfortran-dg-runtest [lsort \
 gfortran-dg-runtest [lsort \
        [glob -nocomplain $srcdir/$subdir/g77/*.\[fF\] ] ] $DEFAULT_GRAPHITE_FLAGS
 
+# Clean up.
+set dg-do-what-default ${save-dg-do-what-default}
 
 # All done.
 dg-finish

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: review of graphite testsuite patch
  2008-08-21 19:19 ` Sebastian Pop
@ 2008-08-21 20:47   ` Janis Johnson
  2008-08-21 21:47     ` Sebastian Pop
  0 siblings, 1 reply; 4+ messages in thread
From: Janis Johnson @ 2008-08-21 20:47 UTC (permalink / raw)
  To: Sebastian Pop; +Cc: gcc-patches, rajagopal, dwarak

On Thu, 2008-08-21 at 14:06 -0500, Sebastian Pop wrote:
> On Wed, Aug 20, 2008 at 4:40 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> > This review of the testsuite changes for graphite is based on

> Attached is a patch that fixes the testsuite according to these
> comments.  Committed to graphite-branch.

That looks good, merge the tests to mainline when appropriate.

> > Almost all tests use the same options.  Within the graphite.exp files
> > in the test suites you could set GRAPHITE_FLAGS to the options that are
> > used for most tests and pass that, instead of DEFAULT_CFLAGS, to
> > dg-runtest.  Those could be overridden in individual tests.  Again,
> > this is just a suggestion.
> 
> This is not yet fixed.

As I said, it's just a suggestion, they're fine as they are.

> > Test gfortran.dg/graphite/scop-1.f doesn't do any checking, what is
> > its purpose?
> >
> scop-1.f was a bug that we reduced.  We test it for compilation only,
> but we can also write a test for the number of SCoPs it contains.

If you don't add the test to check the dump file, add a comment
explaining that the test caused a compilation failure during
development.

Janis

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: review of graphite testsuite patch
  2008-08-21 20:47   ` Janis Johnson
@ 2008-08-21 21:47     ` Sebastian Pop
  0 siblings, 0 replies; 4+ messages in thread
From: Sebastian Pop @ 2008-08-21 21:47 UTC (permalink / raw)
  To: janis187; +Cc: gcc-patches, rajagopal, dwarak

On Thu, Aug 21, 2008 at 3:22 PM, Janis Johnson <janis187@us.ibm.com> wrote:
> If you don't add the test to check the dump file, add a comment
> explaining that the test caused a compilation failure during
> development.

Okay, I will add a test.  Thanks for the review,

Sebastian

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-08-21 21:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-20 22:24 review of graphite testsuite patch Janis Johnson
2008-08-21 19:19 ` Sebastian Pop
2008-08-21 20:47   ` Janis Johnson
2008-08-21 21:47     ` Sebastian Pop

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).