testsuite -Wcomment [was: Re: [RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]]
On Wed, 30 Oct 2013 10:41:33 +0100
Bernhard Reutner-Fischer wrote:
> > Hi!
> >
> > I've noticed that this testcase doesn't clean up after itself.
> > Fixed thusly, committed as obvious to trunk.
>
> This was nagging me last weekend.. ;)
> What about automating this?
>
> Manual part is attached.
> The "Adjust all callers" below is too big to send to the list:
> git grep -l -E "(cleanup-.*-dump|cleanup-saved-temps)" | \
> egrep -v "(ChangeLog|/lib/)" | sed -e "s|[^/]*$||" | sort | uniq | \
> while read d;
> do
> find $d -type f \
> -exec sed -i -e "/cleanup-[^-]*[-]*dump/d;/cleanup-saved-temps/d" {} +
> done
>
>
> Full regstrap on x86_64-unknown-linux-gnu with no regressions with
> trunk@204119 for
> configure \
> --enable-bootstrap \
> --with-system-zlib \
> --without-included-gettext \
> --disable-werror \
> --enable-link-mutex \
> --enable-nls \
> --enable-plugin \
> --enable-__cxa_atexit \
> --enable-debug \
> --enable-checking=yes,rtl \
> --enable-gather-detailed-mem-stats \
> --enable-multilib \
> --enable-multiarch \
> --with-linker-hash-style=both \
> --with-as=$BINU/as \
> --with-ld=$BINU/ld.gold \
> --enable-languages=c,c++,fortran,lto,go,objc,obj-c++ \
> && make -k check -j4
>
> Ok for trunk?
> Comments?
>
> Given the "Fix comment delimiter" hunks in the manual patch, i'd suggest
> to add -Wcomment as default flags where possible to catch these early on
> in the future.
I should not dare to ping this proposal just yet again, now, without due
patience. But, maybe, we want to automatically add -Wcomment to the
testsuite unless a testcase mentions "comment" in their flags to catch
such malformed comments with dejagnu directives, next stage 1?
The handling should probably be about like what we did with the
-fno-ident that was added to the testsuite as to not confuse
scan-assembler test due to local branch names which was applied in
eb6ffc66825a8d36cf89881517624ff2df510aa9 ( r9-2792 )
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=eb6ffc66825a8d36cf89881517624ff2df510aa9
iff maybe this can aid as a guide for someone to fix this for good.
TIA,
>
> gcc/testsuite/ChangeLog
>
> 2013-10-12 Bernhard Reutner-Fischer
>
> * lib/gcc-dg.exp (cleanup-ipa-dump, cleanup-rtl-dump,
> cleanup-tree-dump, cleanup-dump): Remove. Adjust all callers.
> (schedule-cleanups): New proc.
> (gcc-dg-test-1): Call it.
> * lib/profopt.exp (profopt-execute): Likewise.
> * g++.dg/cdce3.C: Adjust expected line numbers.
> * gcc.dg/cdce1.c: Likewise.
> * gcc.dg/cdce2.c: Likewise.
> * gcc.dg/strlenopt-22.c: Fix comment delimiter.
> * gcc.dg/strlenopt-24.c: Likewise.
> * gcc.dg/tree-ssa/vrp26.c: Likewise.
> * gcc.dg/tree-ssa/vrp28.c: Likewise.
> * obj-c++.dg/encode-2.mm: Likewise.
>
> libgomp/ChangeLog
>
> 2013-10-12 Bernhard Reutner-Fischer
>
> * testsuite/libgomp.graphite/bounds.c: Adjust for
> cleanup-tree-dump removal.
> * testsuite/libgomp.graphite/force-parallel-1.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-2.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-3.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-4.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-5.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-6.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-7.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-8.c: Likewise.
> * testsuite/libgomp.graphite/force-parallel-9.c: Likewise.
> * testsuite/libgomp.graphite/pr41118.c: Likewise.
>
>
> gcc/ChangeLog
>
> 2013-10-12 Bernhard Reutner-Fischer
>
> * config/arm/neon-testgen.ml (emit_epilogue): Remove manual call
> to cleanup-saved-temps.
>
> gcc/doc/ChangeLog
>
> 2013-10-12 Bernhard Reutner-Fischer
>
> * doc/sourcebuild.texi (Clean up generated test files): Expand
> introduction.
> (cleanup-ipa-dump, cleanup-rtl-dump, cleanup-tree-dump,
> cleanup-saved-temps): Remove.
Re: [RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]
On Thu, Oct 31, 2013 at 09:39:11AM +0100, Jakub Jelinek wrote:
> On Thu, Oct 31, 2013 at 09:34:41AM +0100, Bernhard Reutner-Fischer wrote:
> > The cleanup routine would currently run 7 regexes on the incoming
> > compiler-flags which is supposedly pretty fast.
> > But yes, we could as well key off scan-dump. If we do that, i'd
> > suggest to simply wipe all potential dumps, regardless of the "family"
> > etc, like:
> > "$ltrans\[0-9\]\[0-9\]\[0-9\][itr].*"
> > What do you think?
>
> Many tests (e.g. in gcc.dg/vect/) pass -fdump-* flags and require cleanups,
> even if they don't have any scan directives.
Right.
Mike, attaching a new, slightly simplified patch.
I do not have time nor interest to persue this any further.
Ok for trunk at this stage?
Otherwise i would suggest to either drop this idea altogether or that
you take over if you think this is a nice thing to have for the next
stage1.
Ontop of this patch you would have to
git grep -l -E "(cleanup-.*-dump|cleanup-saved-temps)" | egrep -v
"(ChangeLog|/lib/)" | sed -e "s|[^/]*$||" | sort | uniq | \
while read d;
do
find $d -type f -exec sed -i -e
"/cleanup-[^-]*[-]*dump/d;/cleanup-saved-temps/d" {} +
done
diffstat -s:
4874 files changed, 111 insertions(+), 5099 deletions(-)
Tested the same way as the initial incarnation against r205304 with no
regressions.
The ChangeLogs remain the same, i.e.:
gcc/testsuite/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* lib/gcc-dg.exp (cleanup-ipa-dump, cleanup-rtl-dump,
cleanup-tree-dump, cleanup-dump): Remove. Adjust all callers.
(schedule-cleanups): New proc.
(gcc-dg-test-1): Call it.
* lib/profopt.exp (profopt-execute): Likewise.
* g++.dg/cdce3.C: Adjust expected line numbers.
* gcc.dg/cdce1.c: Likewise.
* gcc.dg/cdce2.c: Likewise.
* gcc.dg/strlenopt-22.c: Fix comment delimiter.
* gcc.dg/strlenopt-24.c: Likewise.
* gcc.dg/tree-ssa/vrp26.c: Likewise.
* gcc.dg/tree-ssa/vrp28.c: Likewise.
* obj-c++.dg/encode-2.mm: Likewise.
* lib/dg-pch.exp(pch-init): Remove pch-check objects.
libgomp/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* testsuite/libgomp.graphite/bounds.c: Adjust for
cleanup-tree-dump removal.
* testsuite/libgomp.graphite/force-parallel-1.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-2.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-3.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-4.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-5.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-6.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-7.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-8.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-9.c: Likewise.
* testsuite/libgomp.graphite/pr41118.c: Likewise.
gcc/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* config/arm/neon-testgen.ml (emit_epilogue): Remove manual call
to cleanup-saved-temps.
gcc/doc/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* doc/sourcebuild.texi (Clean up generated test files): Expand
introduction.
(cleanup-ipa-dump, cleanup-rtl-dump, cleanup-tree-dump,
cleanup-saved-temps): Remove.
PS: As you can see, i had to touch a couple of files that had broken
comments like the vrp and strlenopt files. Should the testsuite
add -Wcomment per default?
cheers,
>From ac5690774eb2134b063464be56bfc56826305d01 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
Date: Fri, 18 Oct 2013 21:08:49 +0200
Subject: [PATCH] auto-wipe dump files
Signed-off-by: Bernhard Reutner-Fischer
---
gcc/config/arm/neon-testgen.ml| 1 -
gcc/doc/sourcebuild.texi | 19 ++---
gcc/testsuite/g++.dg/cdce3.C | 5 +-
gcc/testsuite/gcc.dg/cdce1.c | 3 +-
gcc/testsuite/gcc.dg/cdce2.c | 3 +-
gcc/testsuite/gcc.dg/strlenopt-22.c | 3 +-
gcc/testsuite/gcc.dg/strlenopt-24.c | 3 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp26.c | 3 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp28.c | 3 +-
gcc/testsuite/lib/dg-pch.exp | 2 +
gcc/testsuite/lib/gcc-dg.exp | 134 +++---
gcc/testsuite/lib/profopt.exp | 3 +
gcc/testsuite/obj-c++.dg/encode-2.mm | 3 +-
13 files changed, 111 insertions(+), 74 deletions(-)
diff --git a/gcc/config/arm/neon-testgen.ml b/gcc/config/arm/neon-testgen.ml
index 543318b..4734ac0 100644
--- a/gcc/config/arm/neon-testgen.ml
+++ b/gcc/config/arm/neon-testgen.ml
@@ -139,7 +139,6 @@ let emit_epilogue chan features regexps =
else
()
);
-Printf.fprintf chan "/* { dg-final { cleanup-saved-temps } } */\n"
(* Check a list of C types to determine which ones are pointers and which
ones are const. *)
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 2947ac8..84ca
Re: [RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]
On Thu, Oct 31, 2013 at 09:34:41AM +0100, Bernhard Reutner-Fischer wrote: > The cleanup routine would currently run 7 regexes on the incoming > compiler-flags which is supposedly pretty fast. > But yes, we could as well key off scan-dump. If we do that, i'd > suggest to simply wipe all potential dumps, regardless of the "family" > etc, like: > "$ltrans\[0-9\]\[0-9\]\[0-9\][itr].*" > What do you think? Many tests (e.g. in gcc.dg/vect/) pass -fdump-* flags and require cleanups, even if they don't have any scan directives. Jakub
Re: [RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]
On 31 October 2013 01:01, Mike Stump wrote:
> On Oct 30, 2013, at 2:41 AM, Bernhard Reutner-Fischer
> wrote:
>>> I've noticed that this testcase doesn't clean up after itself.
>
>> This was nagging me last weekend.. ;)
>> What about automating this?
>
> So, the idea sounds very nice.
>
> One thing that I worry about is the testing speed hit for people (test cases)
> that don't need cleanups. I don't know the speed hit of the code, so, don't
> know how necessary it is to try and go faster.
>
> I was thinking the presence of a scan-tree-dump, would set a bit that said,
> do a scan-tree-dump style cleanup.
Well, since the -fdump-* are the one to produce the dumps, i keyed the
cleanup off that.
Initially i had the idea to use an exact per-pass wiper but that
turned out to be too complicated for no real benefit.
>
> The common code then does, if cleanups needed, do cleanups
>
> The idea, most test cases don't do this, and don't need the big cleanup
> routine to fire. A scan-tree-dump would setup a cleanup tree dumps flags,
> and then in the big cleanup routine, you have:
The cleanup routine would currently run 7 regexes on the incoming
compiler-flags which is supposedly pretty fast.
But yes, we could as well key off scan-dump. If we do that, i'd
suggest to simply wipe all potential dumps, regardless of the "family"
etc, like:
"$ltrans\[0-9\]\[0-9\]\[0-9\][itr].*"
What do you think?
>
> do cleanups()
> {
> if (need tree cleanups) do tree cleanups();
> if (need rtl cleanups) do rtl cleanups();
> }
>
> this way, we avoid randomly doing cleanups for things we don't need them for,
> and avoid even asking if we need any cleanups, as we can have a global flag
> that says if we need any extra, special cleanups.
>
> So, all that would be bad to do, if the speed hit is small… Can you collect
> with/without numbers and post them? If you can, include user, sys and
> elapsed. You can run a subset of one testsuite, say, dg.exp, as
> representative.
The pristine trunk, i.e. with manual cleanup was a couple of seconds
slower than with the patch (10s or 20s IIRC, let me measure this on an
idle box again now).
As you can see, the number of globs before and after the patch
decreases quite a lot, i guess the fact that we glob less is
responsible for the improved runtime. Let me double-check that,
though.
thanks,
Re: [RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]
On Oct 30, 2013, at 2:41 AM, Bernhard Reutner-Fischer
wrote:
>> I've noticed that this testcase doesn't clean up after itself.
> This was nagging me last weekend.. ;)
> What about automating this?
So, the idea sounds very nice.
One thing that I worry about is the testing speed hit for people (test cases)
that don't need cleanups. I don't know the speed hit of the code, so, don't
know how necessary it is to try and go faster.
I was thinking the presence of a scan-tree-dump, would set a bit that said, do
a scan-tree-dump style cleanup.
The common code then does, if cleanups needed, do cleanups
The idea, most test cases don't do this, and don't need the big cleanup routine
to fire. A scan-tree-dump would setup a cleanup tree dumps flags, and then in
the big cleanup routine, you have:
do cleanups()
{
if (need tree cleanups) do tree cleanups();
if (need rtl cleanups) do rtl cleanups();
}
this way, we avoid randomly doing cleanups for things we don't need them for,
and avoid even asking if we need any cleanups, as we can have a global flag
that says if we need any extra, special cleanups.
So, all that would be bad to do, if the speed hit is small… Can you collect
with/without numbers and post them? If you can, include user, sys and elapsed.
You can run a subset of one testsuite, say, dg.exp, as representative.
[RFC/CFT] auto-wipe dump files [was: Re: [committed] Fix up bb-slp-31.c testcase]
> Hi!
>
> I've noticed that this testcase doesn't clean up after itself.
> Fixed thusly, committed as obvious to trunk.
This was nagging me last weekend.. ;)
What about automating this?
Manual part is attached.
The "Adjust all callers" below is too big to send to the list:
git grep -l -E "(cleanup-.*-dump|cleanup-saved-temps)" | \
egrep -v "(ChangeLog|/lib/)" | sed -e "s|[^/]*$||" | sort | uniq | \
while read d;
do
find $d -type f \
-exec sed -i -e "/cleanup-[^-]*[-]*dump/d;/cleanup-saved-temps/d" {} +
done
Full regstrap on x86_64-unknown-linux-gnu with no regressions with
trunk@204119 for
configure \
--enable-bootstrap \
--with-system-zlib \
--without-included-gettext \
--disable-werror \
--enable-link-mutex \
--enable-nls \
--enable-plugin \
--enable-__cxa_atexit \
--enable-debug \
--enable-checking=yes,rtl \
--enable-gather-detailed-mem-stats \
--enable-multilib \
--enable-multiarch \
--with-linker-hash-style=both \
--with-as=$BINU/as \
--with-ld=$BINU/ld.gold \
--enable-languages=c,c++,fortran,lto,go,objc,obj-c++ \
&& make -k check -j4
Ok for trunk?
Comments?
Given the "Fix comment delimiter" hunks in the manual patch, i'd suggest
to add -Wcomment as default flags where possible to catch these early on
in the future.
gcc/testsuite/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* lib/gcc-dg.exp (cleanup-ipa-dump, cleanup-rtl-dump,
cleanup-tree-dump, cleanup-dump): Remove. Adjust all callers.
(schedule-cleanups): New proc.
(gcc-dg-test-1): Call it.
* lib/profopt.exp (profopt-execute): Likewise.
* g++.dg/cdce3.C: Adjust expected line numbers.
* gcc.dg/cdce1.c: Likewise.
* gcc.dg/cdce2.c: Likewise.
* gcc.dg/strlenopt-22.c: Fix comment delimiter.
* gcc.dg/strlenopt-24.c: Likewise.
* gcc.dg/tree-ssa/vrp26.c: Likewise.
* gcc.dg/tree-ssa/vrp28.c: Likewise.
* obj-c++.dg/encode-2.mm: Likewise.
libgomp/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* testsuite/libgomp.graphite/bounds.c: Adjust for
cleanup-tree-dump removal.
* testsuite/libgomp.graphite/force-parallel-1.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-2.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-3.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-4.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-5.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-6.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-7.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-8.c: Likewise.
* testsuite/libgomp.graphite/force-parallel-9.c: Likewise.
* testsuite/libgomp.graphite/pr41118.c: Likewise.
gcc/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* config/arm/neon-testgen.ml (emit_epilogue): Remove manual call
to cleanup-saved-temps.
gcc/doc/ChangeLog
2013-10-12 Bernhard Reutner-Fischer
* doc/sourcebuild.texi (Clean up generated test files): Expand
introduction.
(cleanup-ipa-dump, cleanup-rtl-dump, cleanup-tree-dump,
cleanup-saved-temps): Remove.
>From dc181880947cbfb3d652c6d9530cea07cf8280d8 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
Date: Fri, 18 Oct 2013 21:08:49 +0200
Subject: [PATCH] auto-wipe dump files
---
gcc/config/arm/neon-testgen.ml| 1 -
gcc/doc/sourcebuild.texi | 19 ++--
gcc/testsuite/g++.dg/cdce3.C | 5 +-
gcc/testsuite/gcc.dg/cdce1.c | 3 +-
gcc/testsuite/gcc.dg/cdce2.c | 3 +-
gcc/testsuite/gcc.dg/strlenopt-22.c | 3 +-
gcc/testsuite/gcc.dg/strlenopt-24.c | 3 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp26.c | 3 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp28.c | 3 +-
gcc/testsuite/lib/dg-pch.exp | 18 +++-
gcc/testsuite/lib/gcc-dg.exp | 160 --
gcc/testsuite/lib/profopt.exp | 3 +
gcc/testsuite/obj-c++.dg/encode-2.mm | 3 +-
13 files changed, 151 insertions(+), 76 deletions(-)
diff --git a/gcc/config/arm/neon-testgen.ml b/gcc/config/arm/neon-testgen.ml
index 543318b..4734ac0 100644
--- a/gcc/config/arm/neon-testgen.ml
+++ b/gcc/config/arm/neon-testgen.ml
@@ -139,7 +139,6 @@ let emit_epilogue chan features regexps =
else
()
);
-Printf.fprintf chan "/* { dg-final { cleanup-saved-temps } } */\n"
(* Check a list of C types to determine which ones are pointers and which
ones are const. *)
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 1a70916..7e0ebd9 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2145,13 +2145,17 @@ Check branch and/or call counts, in addition to line counts, in
@subsubsection Clean up generated test files
+Usually the test-framework removes files that were generated during
+testing. If a testcase, for example, uses any dumping mechanism to
+inspect a passes dump file, the testsuite recognize
