From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 87280 invoked by alias); 14 Sep 2015 09:42:08 -0000 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 Received: (qmail 87230 invoked by uid 89); 14 Sep 2015 09:42:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 14 Sep 2015 09:42:06 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 5F06E2A3089E; Mon, 14 Sep 2015 11:42:02 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QZEZ7g00p7jL; Mon, 14 Sep 2015 11:42:02 +0200 (CEST) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 3BCC72A3089C; Mon, 14 Sep 2015 11:42:02 +0200 (CEST) From: Eric Botcazou To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH][20/n] Remove GENERIC stmt combining from SCCVN Date: Mon, 14 Sep 2015 09:47:00 -0000 Message-ID: <2169802.F6q7hcMxNO@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.29-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-SW-Source: 2015-09/txt/msg00919.txt.bz2 > Ok, so it's folding > > x == 127 ? .gnat_rcheck_CE_Overflow_Check ("overflow_sum3.adb", 14);, 0 : > (short_short_integer) x + 1 > > <= 127 > > where op0 (the COND_EXPR) does not have TREE_SIDE_EFFECTS set but > its operand 1 has: > > (gdb) p debug_tree (op0) > type public visited QI > size > unit size > align 8 symtab 0 alias set -1 canonical type 0x7ffff6572dc8 > precision 8 min max 0x7ffff656a6c0 127> context RM > size > chain > > > arg 0 ... > arg 1 short_short_integer> > side-effects > ... > arg 2 short_short_integer> > ... > > that's unexpected to the code generated by genmatch and I don't see > how omit_one_operand would handle that either. The old code was propagating the comparison inside the arms of COND_EXPR (fold_binary_op_with_conditional_arg) before applying the transformation: if ((short_short_integer) x == 127 ? .gnat_rcheck_CE_Overflow_Check ("overflow_sum3.adb", 14);, 1 : 1) The new code does the reverse, but the old behavior can be easily restored: Index: fold-const.c =================================================================== --- fold-const.c (revision 227729) +++ fold-const.c (working copy) @@ -9025,10 +9025,6 @@ fold_binary_loc (location_t loc, && tree_swap_operands_p (arg0, arg1, true)) return fold_build2_loc (loc, swap_tree_comparison (code), type, op1, op0); - tem = generic_simplify (loc, code, type, op0, op1); - if (tem) - return tem; - /* ARG0 is the first operand of EXPR, and ARG1 is the second operand. First check for cases where an arithmetic operation is applied to a @@ -9114,6 +9110,10 @@ fold_binary_loc (location_t loc, } } + tem = generic_simplify (loc, code, type, op0, op1); + if (tem) + return tem; + switch (code) { case MEM_REF: is sufficient to fix the regression. > The COND_EXPR is originally built with TREE_SIDE_EFFECTS set but: > > Hardware watchpoint 7: *$43 > > Old value = 65595 > New value = 59 > emit_check (gnu_cond=, > gnu_expr=, reason=10, gnat_node=2320) > at /space/rguenther/src/svn/trunk/gcc/ada/gcc-interface/trans.c:8823 > 8823 return gnu_result; > $45 = 0 > > so the Ada frontend resets the flag (improperly?): > > emit_check (gnu_cond=, > gnu_expr=, reason=10, gnat_node=2320) > at /space/rguenther/src/svn/trunk/gcc/ada/gcc-interface/trans.c:8823 > 8823 return gnu_result; > $45 = 0 > (gdb) l > 8818 > 8819 /* GNU_RESULT has side effects if and only if GNU_EXPR has: > 8820 we don't need to evaluate it just for the check. */ > 8821 TREE_SIDE_EFFECTS (gnu_result) = TREE_SIDE_EFFECTS (gnu_expr); > 8822 > 8823 return gnu_result; > 8824 } That's old code and the comment makes it quite clear why this is done though. -- Eric Botcazou