From patchwork Mon Jul 11 12:12:51 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 104200 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 C29D9B6F83 for ; Mon, 11 Jul 2011 22:13:12 +1000 (EST) Received: (qmail 31995 invoked by alias); 11 Jul 2011 12:13:10 -0000 Received: (qmail 31914 invoked by uid 22791); 11 Jul 2011 12:13:09 -0000 X-SWARE-Spam-Status: No, hits=-3.2 required=5.0 tests=AWL, BAYES_00, TW_RV, TW_TM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Jul 2011 12:12:53 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id D6F9E89994 for ; Mon, 11 Jul 2011 14:12:51 +0200 (CEST) Date: Mon, 11 Jul 2011 14:12:51 +0200 (CEST) From: Richard Guenther To: Michael Matz Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Make VRP optimize useless conversions In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) 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 On Fri, 8 Jul 2011, Richard Guenther wrote: > On Fri, 8 Jul 2011, Michael Matz wrote: > > > Hi, > > > > On Fri, 8 Jul 2011, Richard Guenther wrote: > > > > > It should be indeed safe with the current handling of conversions, but > > > better be safe. So, like the following? > > > > No. The point is that you can't compare the bounds that VRP computes with > > each other when the outcome affects correctness. Think about a very > > trivial and stupid VRP, that assigns the range [WIDEST_INT_MIN .. > > WIDEST_UINT_MAX] to each and every SSA name without looking at types and > > operations at all (assuming that this reflects the largest int type on the > > target). It's useless but correct. Of course we wouldn't implement such > > useless range discovery, but similar situations can arise when some VRP > > algorithms give up for certain reasons, or computation of tight bounds > > merely isn't implemented for some operations. > > > > Your routines need to work also in the presence of such imprecise ranges. > > > > Hence, the check that the intermediate conversion is useless needs to take > > into account the input value range (that's conservatively correct), and > > the precision and signedness of the target type (if it can represent all > > value of the input range the conversion was useless). It must not look at > > the suspected value range of the destination, precisely because it is > > conservative only. > > Ok, indeed conservative is different for what VRP does and for what > a transformation must assess. So the following patch makes > a conservative attempt at checking the transformation (which of > course non-surprisingly matches what the VRP part does). > > So, more like the following? The following actually works. Bootstrapped and tested on x86_64-unknown-linux-gnu. Can you double-check it? Thanks, Richard. 2011-07-11 Richard Guenther * tree-vrp.c (simplify_conversion_using_ranges): Manually translate the source value-range through the conversion chain. Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 176030) +++ gcc/tree-vrp.c (working copy) @@ -7347,30 +7347,55 @@ simplify_switch_using_ranges (gimple stm static bool simplify_conversion_using_ranges (gimple stmt) { - tree rhs1 = gimple_assign_rhs1 (stmt); - gimple def_stmt = SSA_NAME_DEF_STMT (rhs1); - value_range_t *final, *inner; + tree innerop, middleop, finaltype; + gimple def_stmt; + value_range_t *innervr; + double_int innermin, innermax, middlemin, middlemax; - /* Obtain final and inner value-ranges for a conversion - sequence (final-type)(intermediate-type)inner-type. */ - final = get_value_range (gimple_assign_lhs (stmt)); - if (final->type != VR_RANGE) - return false; + finaltype = TREE_TYPE (gimple_assign_lhs (stmt)); + middleop = gimple_assign_rhs1 (stmt); + def_stmt = SSA_NAME_DEF_STMT (middleop); if (!is_gimple_assign (def_stmt) || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt))) return false; - rhs1 = gimple_assign_rhs1 (def_stmt); - if (TREE_CODE (rhs1) != SSA_NAME) + innerop = gimple_assign_rhs1 (def_stmt); + if (TREE_CODE (innerop) != SSA_NAME) return false; - inner = get_value_range (rhs1); - if (inner->type != VR_RANGE) + + /* Get the value-range of the inner operand. */ + innervr = get_value_range (innerop); + if (innervr->type != VR_RANGE + || TREE_CODE (innervr->min) != INTEGER_CST + || TREE_CODE (innervr->max) != INTEGER_CST) return false; - /* If the value-range is preserved by the conversion sequence strip - the intermediate conversion. */ - if (!tree_int_cst_equal (final->min, inner->min) - || !tree_int_cst_equal (final->max, inner->max)) + + /* Simulate the conversion chain to check if the result is equal if + the middle conversion is removed. */ + innermin = tree_to_double_int (innervr->min); + innermax = tree_to_double_int (innervr->max); + middlemin = double_int_ext (innermin, TYPE_PRECISION (TREE_TYPE (middleop)), + TYPE_UNSIGNED (TREE_TYPE (middleop))); + middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)), + TYPE_UNSIGNED (TREE_TYPE (middleop))); + /* If the middle values do not represent a proper range fail. */ + if (double_int_cmp (middlemin, middlemax, + TYPE_UNSIGNED (TREE_TYPE (middleop))) > 0) return false; - gimple_assign_set_rhs1 (stmt, rhs1); + if (!double_int_equal_p (double_int_ext (middlemin, + TYPE_PRECISION (finaltype), + TYPE_UNSIGNED (finaltype)), + double_int_ext (innermin, + TYPE_PRECISION (finaltype), + TYPE_UNSIGNED (finaltype))) + || !double_int_equal_p (double_int_ext (middlemax, + TYPE_PRECISION (finaltype), + TYPE_UNSIGNED (finaltype)), + double_int_ext (innermax, + TYPE_PRECISION (finaltype), + TYPE_UNSIGNED (finaltype)))) + return false; + + gimple_assign_set_rhs1 (stmt, innerop); update_stmt (stmt); return true; }