review of graphite testsuite patch

[Sebastian Pop]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1168_testsuite.diff
Type: text/x-diff
Size: 16257 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20080821/39a60899/attachment.bin>