From patchwork Tue Aug 2 13:14:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kai Tietz X-Patchwork-Id: 107913 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 CB444B71CE for ; Tue, 2 Aug 2011 23:14:37 +1000 (EST) Received: (qmail 30035 invoked by alias); 2 Aug 2011 13:14:31 -0000 Received: (qmail 29819 invoked by uid 22791); 2 Aug 2011 13:14:28 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_CF, TW_TM X-Spam-Check-By: sourceware.org Received: from mail-qy0-f175.google.com (HELO mail-qy0-f175.google.com) (209.85.216.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 02 Aug 2011 13:14:13 +0000 Received: by qyk30 with SMTP id 30so1654538qyk.20 for ; Tue, 02 Aug 2011 06:14:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.224.205.198 with SMTP id fr6mr1886115qab.162.1312290852629; Tue, 02 Aug 2011 06:14:12 -0700 (PDT) Received: by 10.229.99.137 with HTTP; Tue, 2 Aug 2011 06:14:12 -0700 (PDT) In-Reply-To: References: Date: Tue, 2 Aug 2011 15:14:12 +0200 Message-ID: Subject: Re: [patch tree-optimization]: Avoid !=/== 0/1 comparisons for boolean-typed argument From: Kai Tietz To: Richard Guenther Cc: GCC Patches X-IsSubscribed: yes 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 2011/8/2 Richard Guenther : > On Tue, Aug 2, 2011 at 12:17 PM, Kai Tietz wrote: >> Hello, >> >> this patch removes in forward-propagation useless comparisons X != 0 >> and X != ~0 for boolean-typed X.  For one-bit precision typed X we >> simplifiy X == 0 (and X != ~0) to ~X, and for X != 0 (and X == ~0) to >> X. >> For none one-bit precisione typed X, we simplify here X == 0 -> X ^ 1, >> and for X != 0 -> X.  We can do this as even for Ada - which has only >> boolean-type with none-one-bit precision - the truth-value is one. > > This isn't a simplification but a canonicalization and thus should be > done by fold_stmt instead (we are not propagating anything after all). > In fact, fold_stmt should do parts of this already by means of its > canonicalizations via fold. Well, it simplifies and canonicalizes. But to put this into gimple-fold looks better. >> Additionally this patch changes for function >> forward_propagate_comparison the meaning of true-result.  As this >> result wasn't used and it is benefitial to use this propagation also > > which is a bug - for a true return value we need to set cfg_changed to true. I addressed this in my updated patch (see below) >> in second loop in function ssa_forward_propagate_and_combine, it >> returns true iff statement was altered.  Additionally this function >> handles now the boolean-typed simplifications. > > why call it twice?  How should that be "beneficial"?  I think that > forward_propagate_into_comparison should instead fold the changed > statement. Well, due missing fold_stmt call, there were still none-converted comparisons. I've added here the call to fold_stmt_inplace, and it solved the issue. >> For the hunk in gimple.c for function canonicalize_cond_expr_cond: >> This change seems to show no real effect, but IMHO it makes sense to >> add here the check for cast from boolean-type to be consitant. > > Probably yes. > > Thanks, > Richard. 2011-08-02 Kai Tietz * gimple.c (canonicalize_cond_expr_cond): Handle cast from boolean-type. (ssa_forward_propagate_and_combine): Interprete result of forward_propagate_comparison. * gcc/gimple-fold.c (fold_gimple_assign): Add canonicalization for boolean-typed operands for comparisons. 2011-08-02 Kai Tietz * gcc.dg/tree-ssa/forwprop-15.c: New testcase. Regression tested and bootstrapped for all languages (including Ada and Obj-C++). Ok for apply? Regards, Kai Index: gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-forwprop1" } */ + +_Bool +foo (_Bool a, _Bool b, _Bool c +{ + _Bool r1 = a == 0 & b != 0; + _Bool r2 = b != 0 & c == 0; + return (r1 == 0 & r2 == 0); +} + +/* { dg-final { scan-tree-dump-times " == " 0 "forwprop1" } } */ +/* { dg-final { scan-tree-dump-times " != " 0 "forwprop1" } } */ +/* { dg-final { cleanup-tree-dump "forwprop1" } } */ Index: gcc/gcc/gimple-fold.c =================================================================== --- gcc.orig/gcc/gimple-fold.c +++ gcc/gcc/gimple-fold.c @@ -814,6 +814,34 @@ fold_gimple_assign (gimple_stmt_iterator gimple_assign_rhs1 (stmt), gimple_assign_rhs2 (stmt)); } + else if (gimple_assign_rhs_code (stmt) == EQ_EXPR + || gimple_assign_rhs_code (stmt) == NE_EXPR) + { + tree op1 = gimple_assign_rhs1 (stmt); + tree op2 = gimple_assign_rhs2 (stmt); + tree type = TREE_TYPE (op1); + if (useless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)), + type) + && TREE_CODE (op2) == INTEGER_CST) + { + gimple s; + bool inverted = (gimple_assign_rhs_code (stmt) == EQ_EXPR); + if (!integer_zerop (op2)) + inverted = !inverted; + + if (inverted == false) + result = op1; + else if (TREE_CODE (op1) == SSA_NAME + && (s = SSA_NAME_DEF_STMT (op1)) != NULL + && is_gimple_assign (s) + && gimple_assign_rhs_code (s) == BIT_NOT_EXPR) + result = gimple_assign_rhs1 (s); + else + result = build1_loc (gimple_location (stmt), BIT_NOT_EXPR, type, op1); + + } + + } if (!result) result = fold_binary_loc (loc, subcode, Index: gcc/gcc/tree-ssa-forwprop.c =================================================================== --- gcc.orig/gcc/tree-ssa-forwprop.c +++ gcc/gcc/tree-ssa-forwprop.c @@ -469,6 +469,9 @@ forward_propagate_into_comparison (gimpl { gimple_assign_set_rhs_from_tree (gsi, tmp); update_stmt (stmt); + if (fold_stmt_inplace (stmt)) + update_stmt (stmt); + if (TREE_CODE (rhs1) == SSA_NAME) cfg_changed |= remove_prop_source_from_use (rhs1); if (TREE_CODE (rhs2) == SSA_NAME) @@ -2407,7 +2410,8 @@ ssa_forward_propagate_and_combine (void) } else if (TREE_CODE_CLASS (code) == tcc_comparison) { - forward_propagate_comparison (stmt); + if (forward_propagate_comparison (stmt)) + cfg_changed = true; gsi_next (&gsi); } else