This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add selftest for pretty-print.c (v2)
- From: David Edelsohn <dje dot gcc at gmail dot com>
- To: David Malcolm <dmalcolm at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>, Bernd Schmidt <bschmidt at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 9 Jun 2016 14:06:38 -0400
- Subject: Re: [PATCH] Add selftest for pretty-print.c (v2)
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnymN3H0BKUpKcf5ei5cDtd=ZVcpN9qoZrJ2W_2wNebHkYQ at mail dot gmail dot com> <983cf868-9e9f-71d4-3bae-09c5d0a12cbe at redhat dot com> <CAGWvny=cOjyxi3m8Rs2nRBEQp29JnO9TEp555S9rGv=zgUqB+A at mail dot gmail dot com> <20160609131038 dot GQ7387 at tucnak dot redhat dot com> <CAGWvny=j2umv4GtRz2Fq5UKh2zx_zen5a2175aX3atv4XsVN6A at mail dot gmail dot com> <53de810e-6382-d36b-c95c-9b4e71ceab8d at redhat dot com> <1465494836 dot 4029 dot 30 dot camel at redhat dot com>
On Thu, Jun 9, 2016 at 1:53 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2016-06-09 at 11:22 -0600, Jeff Law wrote:
>> On 06/09/2016 07:30 AM, David Edelsohn wrote:
>> >
>> > The self-tests specifically abort the build and break bootstrap
>> > upon
>> > failure. Most other changes that inadvertently have bugs or tickle
>> > a
>> > latent issue in a target will introduce some additional testsuite
>> > failures, not a bootstrap failure. x86 developers seem to get
>> > quite
>> > annoyed when a patch causes a bootstrap failure for an x86
>> > configuration.
>> >
>> > Second, all of the large changes that may have unknown effects on
>> > various targets have been tested extensively on multiple
>> > architectures, as have most global optimization changes. It may
>> > not
>> > be required, but it generally has been considered "good form" and
>> > has
>> > been a stipulation of patch approval by some reviewers. It would
>> > be
>> > very unfortunate for GCC to lower the bar for patches by some
>> > developers and not others.
>> Let's all calm down a bit here. Everyone here just wants to make a
>> better compiler and mistakes happen.
>
>> What I see in David Malcolm's change is a fairly minor bug. I don't
>> think David (or anyone) could have really expected that %p is printed
>> differently across different hosts and thus his patch would need
>> wider
>> host testing. And AFAICT David addressed this issue as soon as he
>> started his day.
>>
>> So let's all take a deep breath and get back to improving GCC rather
>> than taking jabs at each other.
>>
>
> Sorry about the breakage. I've committed a fix as r237271, which I've
> tested on PPC AIX (and on x86_64 linux).
>
> The selftest code is very new. I tested both it and the pretty-print.c
> tests for every known-good *target* in config-list.mk; the issue here
> was a *host*-specific issue.
>
> Maybe the current "fail the build on any selftest failures" is too
> aggressive. That said, note that if one knows which file the failing
> test is in (which we did), it's trivial to disable the tests in that
> file by hacking gcc/selftests-run-tests.c and commenting out/deleting
> the call:
>
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index 934e700..1c8128b 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -46,7 +46,7 @@ selftest::run_tests ()
> hash_map_tests_c_tests ();
> hash_set_tests_c_tests ();
> vec_c_tests ();
> - pretty_print_c_tests ();
> + //pretty_print_c_tests ();
> wide_int_cc_tests ();
>
>
> whilst the underlying failure is investigated, so adding a new selftest
> is presumably not as risky an event as, say, changing an optimizer: the
> change is localized and can be readily disabled if it turns out to have
> a config-specific assumption.
>
> The selftests currently in trunk aren't the most exciting; I'm much
> more interested in the ggc-tests.c patch (awaiting review), since this
> would finally give us self-testing of gengtype and ggc, which AFAIK we
> haven't been able to test directly before. I hate gengtype, and it's
> been a goal of mine to try to tame it since I started working on gcc.
> (FWIW I've successfully tested the ggc patch on AIX PPC now, for stage
> 1 at least, and for all targets in config-list.mk).
>
> Sorry again about the breakage.
Thanks for fixing this so quickly.
Maybe we need to consider some sort of "warn on failure" beta testing
period for new self-tests before they cause errors. If self-tests can
trigger host-dependent behavior and cause bootstrap failure as a
consequence, we need to think about how this interacts with other GCC
development policies.
Thanks, David