From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31487 invoked by alias); 29 Oct 2005 21:08:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 31468 invoked by uid 22791); 29 Oct 2005 21:08:18 -0000 Received: from bethe.phy.uc.edu (HELO bethe.phy.uc.edu) (129.137.4.14) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sat, 29 Oct 2005 21:08:18 +0000 Received: from earth.geop.uc.edu (earth.phy.uc.edu [10.44.11.234]) by bethe.phy.uc.edu (8.12.11/8.12.11) with ESMTP id j9TL8ALu015047; Sat, 29 Oct 2005 17:08:10 -0400 Received: from earth.phy.uc.edu (localhost.localdomain [127.0.0.1]) by earth.geop.uc.edu (8.12.11/8.9.3) with ESMTP id j9TL8AqV024719; Sat, 29 Oct 2005 17:08:10 -0400 Received: (from pinskia@localhost) by earth.phy.uc.edu (8.12.11/8.12.11/Submit) id j9TL8A40024718; Sat, 29 Oct 2005 17:08:10 -0400 From: Andrew Pinski Message-Id: <200510292108.j9TL8A40024718@earth.phy.uc.edu> Subject: Re: patch ping To: pinskia@physics.uc.edu (Andrew Pinski) Date: Sat, 29 Oct 2005 21:08:00 -0000 Cc: pinskia@physics.uc.edu (Andrew Pinski), ian@airs.com (Ian Lance Taylor), gcc-patches@gcc.gnu.org In-Reply-To: <200510292026.j9TKQQsg023880@earth.phy.uc.edu> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="%--multipart-mixed-boundary-1.23649.1130620090--%" X-Spam-Score: -100 () USER_IN_WHITELIST X-SW-Source: 2005-10/txt/msg01660.txt.bz2 --%--multipart-mixed-boundary-1.23649.1130620090--% Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 1888 > > > > > > > > > Andrew Pinski writes: > > > > > > > I thought that I would not have to ping patches any more > > > > with the patch queue but I am wrong, oh well. > > > > > > > > http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00879.html > > > > [PATCH] Fix PR middle-end/22429, fold building tree which depends on signed overflow > > > > > > > > This fixes one of the wrong-code regressions in 4.1, basicially > > > > fold was building a tree which depends on signed overflowing > > > > being defined as wrapping which is only true with -fwrapv. > > > > > > Setting etype to TREE_TYPE (etype) looks wrong to me. And I suspect > > > that the reason you have to do it is that your patch doesn't set value > > > back to zero. The old code would work because it checks TREE_OVERFLOW > > > again. But your patch effectively does not--it is possible for value > > > to fall through without TREE_OVERFLOW being set. > > > > > > Also, I think the patch makes the code more confusing. In the case of > > > flag_wrapv && !TYPE_UNSIGNED (type), the first computation of value is > > > of no importance. > > > > > > How about something like this? I haven't tested it. > > This works except it introduces two Ada test failures, I am going to look > > into them. They are both ICEs. > > Both of the failure is due to range_binop returning NULL in: > 4047 if (integer_zerop (range_binop (NE_EXPR, integer_type_node, > 4048 minv, 1, maxv, 1))) > > This is due to minv being a non-constant value. > > Let me try the obvious patch to test for that case and see if it works. Here is the patch which works for those two tests and also for my testcase still, it adds the testcase also to the patch. I am right now doing a full bootstrap/test on x86_64-pc-linux-gnu with this patch. Thanks, Andrew Pinski --%--multipart-mixed-boundary-1.23649.1130620090--% Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-Description: ASCII C program text Content-Disposition: attachment; filename="temp.diff.txt" Content-length: 2360 Index: fold-const.c =================================================================== --- fold-const.c (revision 105995) +++ fold-const.c (working copy) @@ -4013,8 +4013,22 @@ build_range_check (tree type, tree exp, } } - value = const_binop (MINUS_EXPR, high, low, 0); - if (value != 0 && TREE_OVERFLOW (value) && ! TYPE_UNSIGNED (etype)) + /* Try to build a check for whether EXP - LOW is between zero and + HIGH - LOW. We can only do this safely using an unsigned type, + or when signed values wrap around. Otherwise EXP - LOW might + overflow. And of course it doesn't work if HIGH - LOW + overflows. */ + + value = NULL_TREE; + + if (TYPE_UNSIGNED (type) || flag_wrapv) + { + value = const_binop (MINUS_EXPR, high, low, 0); + if (value && TREE_OVERFLOW (value)) + value = NULL_TREE; + } + + if (value == NULL_TREE && !TYPE_UNSIGNED (type)) { tree utype, minv, maxv; @@ -4025,6 +4039,11 @@ build_range_check (tree type, tree exp, case INTEGER_TYPE: case ENUMERAL_TYPE: case CHAR_TYPE: + /* If we have a type which is a subtype use the + subtype instead. */ + if (TREE_TYPE (etype)) + etype = TREE_TYPE (etype); + utype = lang_hooks.types.unsigned_type (etype); maxv = fold_convert (utype, TYPE_MAX_VALUE (etype)); maxv = range_binop (PLUS_EXPR, NULL_TREE, maxv, 1, @@ -4038,6 +4057,8 @@ build_range_check (tree type, tree exp, low = fold_convert (etype, low); exp = fold_convert (etype, exp); value = const_binop (MINUS_EXPR, high, low, 0); + if (TREE_OVERFLOW (value)) + value = NULL_TREE; } break; default: @@ -4045,7 +4066,7 @@ build_range_check (tree type, tree exp, } } - if (value != 0 && ! TREE_OVERFLOW (value)) + if (value) return build_range_check (type, fold_build2 (MINUS_EXPR, etype, exp, low), 1, fold_convert (etype, integer_zero_node), Index: testsuite/gcc.c-torture/execute/pr22429-1.c =================================================================== --- testsuite/gcc.c-torture/execute/pr22429-1.c (revision 0) +++ testsuite/gcc.c-torture/execute/pr22429-1.c (revision 0) @@ -0,0 +1,14 @@ +int f(int n) +{ + if (-1073741824 <= n && n <= 1073741823) + return 1; + return 0; +} + +int main() +{ + if (f(1073741824)) + abort (); + return 0; +} + --%--multipart-mixed-boundary-1.23649.1130620090--%--