From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8678 invoked by alias); 29 Oct 2005 04:16:10 -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 8576 invoked by uid 22791); 29 Oct 2005 04:16:04 -0000 Received: from yosemite.airs.com (HELO yosemite.airs.com) (205.217.158.180) by sourceware.org (qpsmtpd/0.30-dev) with SMTP; Sat, 29 Oct 2005 04:16:04 +0000 Received: (qmail 2634 invoked by uid 10); 29 Oct 2005 04:16:03 -0000 Received: (qmail 24805 invoked by uid 500); 29 Oct 2005 04:15:56 -0000 To: Andrew Pinski Cc: gcc-patches@gcc.gnu.org Subject: Re: patch ping References: <200510290118.j9T1Idir028535@earth.phy.uc.edu> From: Ian Lance Taylor Date: Sat, 29 Oct 2005 04:16:00 -0000 In-Reply-To: <200510290118.j9T1Idir028535@earth.phy.uc.edu> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.4 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2005-10/txt/msg01640.txt.bz2 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. Ian --- fold-const.c.~1.629.~ 2005-10-21 10:15:14.000000000 -0700 +++ fold-const.c 2005-10-28 20:57:15.000000000 -0700 @@ -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; @@ -4038,6 +4052,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 +4061,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),