From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16149 invoked by alias); 8 Jul 2011 13:33:57 -0000 Received: (qmail 16141 invoked by uid 22791); 8 Jul 2011 13:33:56 -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; Fri, 08 Jul 2011 13:33:41 +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 B751E8A95F for ; Fri, 8 Jul 2011 15:33:40 +0200 (CEST) Date: Fri, 08 Jul 2011 13:38:00 -0000 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 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-07/txt/msg00639.txt.bz2 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? Bootstrap & regtest in progress. Thanks, Richard. 2011-07-08 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,56 @@ 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; + + /* Do not allow changing a zero- to a sign-extension or vice versa. */ + if (TYPE_UNSIGNED (finaltype) + != TYPE_UNSIGNED (TREE_TYPE (middleop))) 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 (!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, rhs1); + + gimple_assign_set_rhs1 (stmt, innerop); update_stmt (stmt); return true; }