This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: regression/btest-gcc.sh: Support --add-passes-despite-regression



On 05/06/2005, at 8:32 PM, Hans-Peter Nilsson wrote:


ISTR I've suggested this functionality before but it was turned
down as the default because it could cause spurious PASSes.  But
that's not a good enough reason: what statistically happens is
*not* that some regression somehow causes spurious PASSes, but
that a *single* long-standing regression stops
regression-checking of test-cases added later.  Not necessarily
good for a script that's supposed to test regressions...

No, the concern is that a patch might break the compiler (for example, have it always return silently with exit status 0) which would cause both regressions and spurious passes; then the patch is fixed, and suddenly a bunch of "regressions" appear which are not really regressions, requiring manual intervention to clear.


Given that explanation, do you still want this patch? You might consider that if you're going to be intervening manually anyway, you can just as easily add new passes by hand (after reviewing them) as delete them...

I'll also take the opportunity to ping the following patches to
btest-gcc.sh:
<URL:http://gcc.gnu.org/ml/gcc-patches/2005-04/msg00766.html>
Make libstdc++.sum optional
<URL:http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01654.html>
Don't pass --with-newlib when target is "*-linux*".

I've already reviewed these in <http://gcc.gnu.org/ml/gcc-patches/2005-05/msg01816.html>.

In case the lack of review is due to a policy of never changing
or adding anything to this file, I guess the best action would
be to make a copy, so useful changes can be made.

Ok to commit?

@@ -172,17 +191,34 @@ for LOG in $TESTLOGS ; do
done | sort | uniq > $FAILED || exit 1
comm -12 $FAILED $PASSES >> $REGRESS || exit 1
NUMREGRESS=`wc -l < $REGRESS | tr -d ' '`
-if [ $NUMREGRESS -ne 0 ] ; then
+if [ $NUMREGRESS -ne 0 ] && [ $add_passes_despite_regression -eq 0 ] ; then
echo regress-$NUMREGRESS > $RESULT
exit 1
fi


-# It passed. Update the state.
+# Update the state.
for LOG in $TESTLOGS ; do
L=`basename $LOG`
awk '/^PASS: / { print "'$L'",$2; }' $LOG || exit 1
done | sort | uniq | comm -23 - $FAILED > ${PASSES}~ || exit 1
[ -s ${PASSES}~ ] || exit 1
-mv ${PASSES}~ ${PASSES} || exit 1
+if [ $NUMREGRESS -ne 0 ] ; then
+ # While in regression, merge in previous PASSes, since there's a
+ # regressive difference we don't want to forget. We don't do this
+ # when PASS (no or non-regressive difference), so we get rid of PASS
+ # lines for removed, moved or otherwise changed tests which may be
+ # added back with a different meaning later on.
+ cat ${PASSES}~ ${PASSES} | sort -u > ${PASSES}~~
+ mv ${PASSES}~~ ${PASSES} || exit 1
+ rm ${PASSES}~ || exit 1
+else
+ mv ${PASSES}~ ${PASSES} || exit 1
+fi
+
+if [ $NUMREGRESS -ne 0 ] ; then
+ echo regress-$NUMREGRESS > $RESULT
+ exit 1
+fi
+
echo pass > $RESULT
exit 0

It would be better to avoid the duplicated code here, and just surround the 'Update the state' code with 'if [ $NUMREGRESS -eq 0 -o $add_passes_despite_regression -ne 0]'.

Attachment: smime.p7s
Description: S/MIME cryptographic signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]