From patchwork Tue May 8 03:39:51 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 157551 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 07B59B6FB4 for ; Tue, 8 May 2012 13:40:17 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1337053219; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:Received:Date:Message-Id:From:To: Subject:MIME-Version:Content-Type:Content-Transfer-Encoding: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=NmGMQjRftvK1VRTdgJLd 86dvCdA=; b=dKvK1tZ387maqMYRi8GbMKizcCHyEoV0H2YYXYpT3ZtgPrChD2i7 hNP/E7jf6KrJjJVBknfG65PaRruy7L6JTU1BLls19tl5SmL6I7IT6EHZsiKsosOp YP07Frjks6NZXnJ0VRdsg4CYp9uQlRbCyqgXeKdS6hOuyFuksDXmhDg= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:Received:Date:Message-Id:From:To:Subject:MIME-Version:Content-Type:Content-Transfer-Encoding:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=CSD8HTYmPrNi9m4zCxwaon3jEkPWVm0nTSWvcVPilSSTFTYCD0G67Z8kzC3qdX 0S4rFQIaGBM3POf1ESmWhSG6xz3QDnpsM5ZljQbGpf9wju6rkYsl3ByrcH5ktn2b 06vRGr61mJPDyq17xSIrDr19dyOQ8pqP20388IcWHA9pQ=; Received: (qmail 3141 invoked by alias); 8 May 2012 03:40:11 -0000 Received: (qmail 3112 invoked by uid 22791); 8 May 2012 03:40:09 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_NO, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from ra.se.axis.com (HELO ra.se.axis.com) (195.60.68.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 May 2012 03:39:55 +0000 Received: from localhost (localhost [127.0.0.1]) by ra.se.axis.com (Postfix) with ESMTP id A378111F03 for ; Tue, 8 May 2012 05:39:53 +0200 (CEST) Received: from ra.se.axis.com ([127.0.0.1]) by localhost (ra.se.axis.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 1p1aeuqYW4uG for ; Tue, 8 May 2012 05:39:52 +0200 (CEST) Received: from thoth.se.axis.com (thoth.se.axis.com [10.0.2.173]) by ra.se.axis.com (Postfix) with ESMTP id 4311711F00 for ; Tue, 8 May 2012 05:39:52 +0200 (CEST) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by thoth.se.axis.com (Postfix) with ESMTP id 2FA5A3407C; Tue, 8 May 2012 05:39:52 +0200 (CEST) Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id q483dqmt003679; Tue, 8 May 2012 05:39:52 +0200 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id q483dp8q003675; Tue, 8 May 2012 05:39:51 +0200 Date: Tue, 8 May 2012 05:39:51 +0200 Message-Id: <201205080339.q483dp8q003675@ignucius.se.axis.com> From: Hans-Peter Nilsson To: gcc-patches@gcc.gnu.org Subject: Heads-up, PR53273: testsuite separation and dilution problem. Fix for PR53272 MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org The problem was spotted while fixing PR53272, a target bug with crisv32-* involving the error-prone notice_update_cc function. When wrapping up the test-case to use as a run-test, adding main and auxiliary functions to the reduced test-case unexpectedly made the bug go away. This despite all functions (except main) being decorated with noinline, noclone and the special marker asm ("") ad finitum. See below: putting the two-file test-case in a single file causes different code for the rtc_update_irq_enable function in the .expand stage already. That REALLY shouldn't happen. I hope this is just a bug and not as it's supposed to be, but this is not the first time I notice this general problem, hence this rant and PR53273: It is, and you don't understand how this can be a problem, or think I should just add the brand new function attribute nofrobnicate? Well, having to add two files for each test-case is enough of a problem on its own. The bigger problems is the integrity of test-cases: there's no way to know that a future optimization doesn't see through those separated files (like, an LTO that is enabled always). There *must* be a future-proof way to write test-cases marking where cross-function optimizations should not happen. If it's implemented as function-attribute-nofrobnicate so be it, but it must not be limited to only the optimizations in place today. Otherwise, some new middle-end generic optimization will optimize away the test-case (and always return success), most likely eliminating the point of the test. When the point of the test-case is to cover code in a port or lower levels, optimizing away the test-cases opens up for bugs to silently creep in; the original bug or bugs in the functionality being covered. This optimization-limiting mechanism really should, almost-must, work within a single file. In a (semi-)perfect world, someone would interate over the testsuite, adding such attributes to the existing tests; I fear a lot of those that don't use "noclone" are silently already just eliminated to "exit (0)". We're on a slippery slope here: it started with having to add "noinline" attributes, then "noclone" attributes, then the asm("") marker. Now that doesn't work anymore either. Can we just have a way to limit those pesky cross-function optimizations and all their kin once and for all? Ok, enough ranting. If anyone knows off-hand why the code would differ, feel free to add to PR53273, else I'll eventually analyze it. It might just be a minor bug after all; like some static branch prediction not being cleared when seeing a noinline-marked function. The test-case below and patch will be committed to trunk and the 4.7 branch as soon as testing for crisv32-elf finishes. gcc/testsuite: PR target/53272 * gcc.dg/torture/pr53272-1.c, gcc.dg/torture/pr53272-2.c: New test. gcc: PR target/53272 * config/cris/cris.c (cris_normal_notice_update_cc): For TARGET_V32, when a constant source operand matches an "I" constraint, the "no CC0 change" applies to a register-destination only, not a strict_low_part-destination. brgds, H-P --- /dev/null Tue Oct 29 15:57:07 2002 +++ gcc/testsuite/gcc.dg/torture/pr53272-1.c Tue May 8 03:07:52 2012 @@ -0,0 +1,39 @@ +/* { dg-do run } */ +/* { dg-additional-sources "pr53272-2.c" } */ +struct rtc_class_ops { + int (*f)(void *, unsigned int enabled); +}; + +struct rtc_device +{ + void *owner; + const struct rtc_class_ops *ops; + int ops_lock; +}; + +__attribute__ ((__noinline__, __noclone__)) +extern int foo(void *); +__attribute__ ((__noinline__, __noclone__)) +extern void foobar(void *); + +__attribute__ ((__noinline__, __noclone__)) +int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled) +{ + int err; + asm volatile (""); + + err = foo(&rtc->ops_lock); + + if (err) + return err; + + if (!rtc->ops) + err = -19; + else if (!rtc->ops->f) + err = -22; + else + err = rtc->ops->f(rtc->owner, enabled); + + foobar(&rtc->ops_lock); + return err; +} --- /dev/null Tue Oct 29 15:57:07 2002 +++ gcc/testsuite/gcc.dg/torture/pr53272-2.c Tue May 8 02:23:21 2012 @@ -0,0 +1,39 @@ +__attribute__ ((__noinline__, __noclone__)) +int foo(void *x) +{ + asm (""); + return *(int *) x != 42; +} + +__attribute__ ((__noinline__, __noclone__)) +void foobar(void *x) +{ + asm (""); + if (foo(x)) + __builtin_abort(); +} + +struct rtc_class_ops { + int (*f)(void *, unsigned int enabled); +}; + +struct rtc_device +{ + void *owner; + struct rtc_class_ops *ops; + int ops_lock; +}; + +extern __attribute__ ((__noinline__, __noclone__)) +int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int); + +int main(void) +{ + struct rtc_class_ops ops = {(void *) 0}; + struct rtc_device dev1 = {0, &ops, 42}; + + if (rtc_update_irq_enable (&dev1, 1) != -22) + __builtin_abort (); + + __builtin_exit (0); +} Index: gcc/config/cris/cris.c =================================================================== --- gcc/config/cris/cris.c (revision 187269) +++ gcc/config/cris/cris.c (working copy) @@ -1695,6 +1695,7 @@ cris_normal_notice_update_cc (rtx exp, r && (REGNO (SET_SRC (exp)) > CRIS_LAST_GENERAL_REGISTER)) || (TARGET_V32 + && REG_P (SET_DEST (exp)) && satisfies_constraint_I (SET_SRC (exp)))) { /* There's no CC0 change for this case. Just check