From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81022 invoked by alias); 9 Jun 2016 17:54:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 80998 invoked by uid 89); 9 Jun 2016 17:54:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=awaiting, exciting, whilst, his X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 09 Jun 2016 17:53:59 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 32B8BC00B8FE; Thu, 9 Jun 2016 17:53:58 +0000 (UTC) Received: from vpn-239-243.phx2.redhat.com (vpn-239-243.phx2.redhat.com [10.3.239.243]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u59HrvfF030943; Thu, 9 Jun 2016 13:53:57 -0400 Message-ID: <1465494836.4029.30.camel@redhat.com> Subject: Re: [PATCH] Add selftest for pretty-print.c (v2) From: David Malcolm To: Jeff Law , David Edelsohn , Jakub Jelinek Cc: Bernd Schmidt , GCC Patches Date: Thu, 09 Jun 2016 17:54:00 -0000 In-Reply-To: <53de810e-6382-d36b-c95c-9b4e71ceab8d@redhat.com> References: <983cf868-9e9f-71d4-3bae-09c5d0a12cbe@redhat.com> <20160609131038.GQ7387@tucnak.redhat.com> <53de810e-6382-d36b-c95c-9b4e71ceab8d@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00733.txt.bz2 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. Dave