From patchwork Wed Nov 7 12:55:10 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 197661 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 9B9582C014C for ; Wed, 7 Nov 2012 23:55:27 +1100 (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=1352897728; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Date:From:To:Cc:Subject:Message-ID:Reply-To: MIME-Version:Content-Type:Content-Disposition:User-Agent: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=R40Srz9FXapWoBcqSii7 24oIyaE=; b=MoPO/fboMLulcFNImjhiCrF9uM75ceX2jWe3DLxcC9R49b6CDb5n fOoyiGVApi2+DIe3YMtm9sDdW6JEBS0UApOdPtuoE59nO3oc/Sj2p+lMdDYeZbgR 8ZJDCcS1bAdJofZ0kWxB7hnzsdXmgg7aVKv+XfyEcFbCpr98e8m8weI= 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:Date:From:To:Cc:Subject:Message-ID:Reply-To:MIME-Version:Content-Type:Content-Disposition:User-Agent:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=ac1Bz1Dw/D8qJHF0GocC9JB+qCZJONb+MHa0rXGRaOFlLr2lZzbw7WaJkWp908 q63Jf4HQ53t2GPTn4t9htwW+syb+v8a4GlMCY9NV7hQVCLkSnX19jrEyGwkNx1u6 ZpiyjYYX4FUX4CgIzDjKYUqAmYnwe8RSCeBLI1Nc60Olw=; Received: (qmail 21465 invoked by alias); 7 Nov 2012 12:55:23 -0000 Received: (qmail 21457 invoked by uid 22791); 7 Nov 2012 12:55:23 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_SPAMHAUS_DROP, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Nov 2012 12:55:16 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qA7CtEhK012344 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 7 Nov 2012 07:55:14 -0500 Received: from zalov.redhat.com (vpn1-4-39.ams2.redhat.com [10.36.4.39]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qA7CtCBZ013979 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 7 Nov 2012 07:55:13 -0500 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.redhat.com (8.14.5/8.14.5) with ESMTP id qA7CtBPB004648; Wed, 7 Nov 2012 13:55:11 +0100 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id qA7CtB5k004647; Wed, 7 Nov 2012 13:55:11 +0100 Date: Wed, 7 Nov 2012 13:55:10 +0100 From: Jakub Jelinek To: gcc-patches@gcc.gnu.org Cc: Eric Botcazou , Richard Henderson Subject: [PATCH] Fix fold reassociation (PR c++/55137) Message-ID: <20121107125510.GB1881@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 Hi! The first (C++) testcase is rejected since my SIZEOF_EXPR folding deferral changes, the problem is that -1 + (int) (sizeof (int) - 1) is first changed into -1 + (int) ((unsigned) sizeof (int) + UINT_MAX) and then fold_binary_loc reassociates it in int type into (int) sizeof (int) + [-2(overflow)], thus introducing overflow where there hasn't been originally. maybe_const_value then refuses to fold it into constant due to that. I've fixed that by the moving the TYPE_OVERFLOW check from before the operation to a check whether the whole reassociation doesn't introduce overflows (while previously it was testing solely for overflow introduced on fold_converted lit0/lit1 being combined together, not e.g. when overflow was introduced by fold_converting lit0 resp. lit1, or for minus_lit{0,1} constants). Unfortunately that patch lead to regression on loop-15.c: +FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "\\\\+" 0 +FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "n_. \\\\* n_." 1 as we were no longer reassociating an expression used in number of iterations calculation. Looking at that lead me to the second testcase below, which I hope is valid C, the overflow happens there in unsigned type, thus with defined wrapping, then there is implementation defined? conversion to signed type (but all our targets are two's complement), and associate_trees: was reassociating it in signed type, thus introducing signed overflow where there wasn't before. Fixed by doing the reassociation in the unsigned type if (at least) one of the operands is of unsigned type. This fixes the loop-15.c testcase as well as the new testcase. CCing Eric as the author of the last changes in that area. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2012-11-07 Jakub Jelinek PR c++/55137 * fold-const.c (fold_binary_loc) : Don't introduce TREE_OVERFLOW through reassociation. If type doesn't have defined overflow, but one or both of the operands do, use the wrapping type for reassociation and only convert to type at the end. * g++.dg/opt/pr55137.C: New test. * gcc.c-torture/execute/pr55137.c: New test. Jakub --- gcc/fold-const.c.jj 2012-11-07 09:16:41.929494183 +0100 +++ gcc/fold-const.c 2012-11-07 09:47:12.227710542 +0100 @@ -10337,6 +10337,7 @@ fold_binary_loc (location_t loc, { tree var0, con0, lit0, minus_lit0; tree var1, con1, lit1, minus_lit1; + tree atype = type; bool ok = true; /* Split both trees into variables, constants, and literals. Then @@ -10352,11 +10353,25 @@ fold_binary_loc (location_t loc, if (code == MINUS_EXPR) code = PLUS_EXPR; - /* With undefined overflow we can only associate constants with one - variable, and constants whose association doesn't overflow. */ + /* With undefined overflow prefer doing association in a type + which wraps on overflow, if that is one of the operand types. */ if ((POINTER_TYPE_P (type) && POINTER_TYPE_OVERFLOW_UNDEFINED) || (INTEGRAL_TYPE_P (type) && !TYPE_OVERFLOW_WRAPS (type))) { + if (INTEGRAL_TYPE_P (TREE_TYPE (arg0)) + && TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0))) + atype = TREE_TYPE (arg0); + else if (INTEGRAL_TYPE_P (TREE_TYPE (arg1)) + && TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1))) + atype = TREE_TYPE (arg1); + gcc_assert (TYPE_PRECISION (atype) == TYPE_PRECISION (type)); + } + + /* With undefined overflow we can only associate constants with one + variable, and constants whose association doesn't overflow. */ + if ((POINTER_TYPE_P (atype) && POINTER_TYPE_OVERFLOW_UNDEFINED) + || (INTEGRAL_TYPE_P (atype) && !TYPE_OVERFLOW_WRAPS (atype))) + { if (var0 && var1) { tree tmp0 = var0; @@ -10367,14 +10382,14 @@ fold_binary_loc (location_t loc, if (CONVERT_EXPR_P (tmp0) && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (tmp0, 0))) && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (tmp0, 0))) - <= TYPE_PRECISION (type))) + <= TYPE_PRECISION (atype))) tmp0 = TREE_OPERAND (tmp0, 0); if (TREE_CODE (tmp1) == NEGATE_EXPR) tmp1 = TREE_OPERAND (tmp1, 0); if (CONVERT_EXPR_P (tmp1) && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (tmp1, 0))) && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (tmp1, 0))) - <= TYPE_PRECISION (type))) + <= TYPE_PRECISION (atype))) tmp1 = TREE_OPERAND (tmp1, 0); /* The only case we can still associate with two variables is if they are the same, modulo negation and bit-pattern @@ -10382,16 +10397,6 @@ fold_binary_loc (location_t loc, if (!operand_equal_p (tmp0, tmp1, 0)) ok = false; } - - if (ok && lit0 && lit1) - { - tree tmp0 = fold_convert (type, lit0); - tree tmp1 = fold_convert (type, lit1); - - if (!TREE_OVERFLOW (tmp0) && !TREE_OVERFLOW (tmp1) - && TREE_OVERFLOW (fold_build2 (code, type, tmp0, tmp1))) - ok = false; - } } /* Only do something if we found more than two objects. Otherwise, @@ -10402,10 +10407,16 @@ fold_binary_loc (location_t loc, + (lit0 != 0) + (lit1 != 0) + (minus_lit0 != 0) + (minus_lit1 != 0)))) { - var0 = associate_trees (loc, var0, var1, code, type); - con0 = associate_trees (loc, con0, con1, code, type); - lit0 = associate_trees (loc, lit0, lit1, code, type); - minus_lit0 = associate_trees (loc, minus_lit0, minus_lit1, code, type); + bool any_overflows = false; + if (lit0) any_overflows |= TREE_OVERFLOW (lit0); + if (lit1) any_overflows |= TREE_OVERFLOW (lit1); + if (minus_lit0) any_overflows |= TREE_OVERFLOW (minus_lit0); + if (minus_lit1) any_overflows |= TREE_OVERFLOW (minus_lit1); + var0 = associate_trees (loc, var0, var1, code, atype); + con0 = associate_trees (loc, con0, con1, code, atype); + lit0 = associate_trees (loc, lit0, lit1, code, atype); + minus_lit0 = associate_trees (loc, minus_lit0, minus_lit1, + code, atype); /* Preserve the MINUS_EXPR if the negative part of the literal is greater than the positive part. Otherwise, the multiplicative @@ -10419,38 +10430,45 @@ fold_binary_loc (location_t loc, && tree_int_cst_lt (lit0, minus_lit0)) { minus_lit0 = associate_trees (loc, minus_lit0, lit0, - MINUS_EXPR, type); + MINUS_EXPR, atype); lit0 = 0; } else { lit0 = associate_trees (loc, lit0, minus_lit0, - MINUS_EXPR, type); + MINUS_EXPR, atype); minus_lit0 = 0; } } + + /* Don't introduce overflows through reassociation. */ + if (!any_overflows + && ((lit0 && TREE_OVERFLOW (lit0)) + || (minus_lit0 && TREE_OVERFLOW (minus_lit0)))) + return NULL_TREE; + if (minus_lit0) { if (con0 == 0) return fold_convert_loc (loc, type, associate_trees (loc, var0, minus_lit0, - MINUS_EXPR, type)); + MINUS_EXPR, atype)); else { con0 = associate_trees (loc, con0, minus_lit0, - MINUS_EXPR, type); + MINUS_EXPR, atype); return fold_convert_loc (loc, type, associate_trees (loc, var0, con0, - PLUS_EXPR, type)); + PLUS_EXPR, atype)); } } - con0 = associate_trees (loc, con0, lit0, code, type); + con0 = associate_trees (loc, con0, lit0, code, atype); return fold_convert_loc (loc, type, associate_trees (loc, var0, con0, - code, type)); + code, atype)); } } --- gcc/testsuite/g++.dg/opt/pr55137.C.jj 2012-11-07 09:31:45.027160654 +0100 +++ gcc/testsuite/g++.dg/opt/pr55137.C 2012-11-07 09:31:45.028160649 +0100 @@ -0,0 +1,4 @@ +// PR c++/55137 +// { dg-do compile } + +enum E { F = -1 + (int) (sizeof (int) - 1) }; --- gcc/testsuite/gcc.c-torture/execute/pr55137.c.jj 2012-11-07 09:44:11.226768063 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr55137.c 2012-11-07 09:44:24.380691303 +0100 @@ -0,0 +1,30 @@ +/* PR c++/55137 */ + +extern void abort (void); + +int +foo (unsigned int x) +{ + return ((int) (x + 1U) + 1) < (int) x; +} + +int +bar (unsigned int x) +{ + return (int) (x + 1U) + 1; +} + +int +baz (unsigned int x) +{ + return x + 1U; +} + +int +main () +{ + if (foo (__INT_MAX__) != (bar (__INT_MAX__) < __INT_MAX__) + || foo (__INT_MAX__) != ((int) baz (__INT_MAX__) + 1 < __INT_MAX__)) + abort (); + return 0; +}