From patchwork Sat Jul 6 19:42:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Glisse X-Patchwork-Id: 257299 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]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 1DF932C009A for ; Sun, 7 Jul 2013 05:42:21 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type :content-id; q=dns; s=default; b=eWPxeVpOS8vxkE/4nqWx2pHXAuCkhSm v5qxtYwGy7WI46vbBpdrQ2GjQNnDHApZTHSCUObv+JTHAe6bstDRGiOEMiuMvbHX G5++2fbW1FApWdQe9Tt0cu4RP1koUirUO/sYA3lAaIJQW2XA+dSYDV7wge6kXgH+ XmgJaTwTq1Sw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type :content-id; s=default; bh=tVqU4oY6Xi5AdqIz5Ar5coixtM4=; b=W0l34 MeOiRq+6ZQufxkxkUnfS4dX4I2gEB0AjcmtdMxMw4lgrE/o/PC61X/K357DT3BAh 9r7j+xlyelPdfTcbYDziOPq++ckOcN2DGs3oN75iXxY6fh1My7aoVoYMkMlfMBkL vmrOAg+I8PZRbWj2nWc2P9pDEWKVdnPOHwlpP0= Received: (qmail 32728 invoked by alias); 6 Jul 2013 19:42:14 -0000 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 Received: (qmail 32703 invoked by uid 89); 6 Jul 2013 19:42:08 -0000 X-Spam-SWARE-Status: No, score=-6.0 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, TW_CF, TW_JB, TW_TM autolearn=no version=3.3.1 Received: from mail3-relais-sop.national.inria.fr (HELO mail3-relais-sop.national.inria.fr) (192.134.164.104) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sat, 06 Jul 2013 19:42:05 +0000 Received: from stedding.saclay.inria.fr ([193.55.250.194]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES128-SHA; 06 Jul 2013 21:42:02 +0200 Received: from glisse (helo=localhost) by stedding.saclay.inria.fr with local-esmtp (Exim 4.80) (envelope-from ) id 1UvYMs-0001EC-7n for gcc-patches@gcc.gnu.org; Sat, 06 Jul 2013 21:42:02 +0200 Date: Sat, 6 Jul 2013 21:42:02 +0200 (CEST) From: Marc Glisse To: gcc-patches@gcc.gnu.org Subject: folding (vec_)cond_expr in a binary operation Message-ID: User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-ID: X-Virus-Found: No Hello, the first attached patch does not bootstrap and has at least 2 main issues. The second patch does pass bootstrap+testsuite, but I liked the first more... First, the fold-const bit causes an assertion failure (building libjava) in combine_cond_expr_cond, which calls: t = fold_binary_loc (gimple_location (stmt), code, type, op0, op1); and then checks: /* Require that we got a boolean type out if we put one in. */ gcc_assert (TREE_CODE (TREE_TYPE (t)) == TREE_CODE (type)); which makes sense... Note that the 2 relevant types are: (gdb) call debug_tree((tree)0x7ffff5e78930) constant 8> unit size constant 1> align 8 symtab -169408800 alias set -1 canonical type 0x7ffff6635c78 precision 1 min max pointer_to_this chain > (gdb) call debug_tree(type) constant 8> unit size constant 1> align 8 symtab -170348640 alias set 19 canonical type 0x7ffff64f69d8 precision 1 min max pointer_to_this > I need to relax the conditions on the vec?-1:0 optimization because with vectors we often end up with several types that are equivalent. Can you think of a good way to do it? In the second patch I limit the transformation to vectors and hope it doesn't cause as much trouble there... Second, the way the forwprop transformation is done, it can be necessary to run the forwprop pass several times in a row, which is not nice. It takes: stmt_cond stmt_binop and produces: stmt_folded stmt_cond2 But one of the arguments of stmt_folded could be an ssa_name defined by a cond_expr, which could now be propagated into stmt_folded (there may be other new possible transformations). However, that cond_expr has already been visited and won't be again. The combine part of the pass uses a PLF to revisit statements, but the forwprop part doesn't have any specific mechanism. The workarounds I can think of are: - manually check if some of the arguments of stmt_folded are cond_expr and recursively call the function on them; - move the transformation to the combine part of the pass; - setup some system to revisit all earlier statements? I went with the second option in the second patch, but this limitation of forward propagation seems strange to me. (s/combine_binop_with_condition/forward_propagate_condition/ for the first patch) 2013-07-06 Marc Glisse PR tree-optimization/57755 gcc/ * tree-ssa-forwprop.c (combine_binop_with_condition): New function. (ssa_forward_propagate_and_combine): Call it. * fold-const.c (fold_ternary_loc): In gimple form, don't check for type equality. gcc/testsuite/ * g++.dg/tree-ssa/pr57755.C: New testcase. (this message does not supersede http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01624.html ) Index: fold-const.c =================================================================== --- fold-const.c (revision 200736) +++ fold-const.c (working copy) @@ -14124,21 +14124,23 @@ fold_ternary_loc (location_t loc, enum t /* Convert A ? 1 : 0 to simply A. */ if ((code == VEC_COND_EXPR ? integer_all_onesp (op1) : (integer_onep (op1) && !VECTOR_TYPE_P (type))) && integer_zerop (op2) /* If we try to convert OP0 to our type, the call to fold will try to move the conversion inside a COND, which will recurse. In that case, the COND_EXPR is probably the best choice, so leave it alone. */ - && type == TREE_TYPE (arg0)) + && (type == TREE_TYPE (arg0) + || (in_gimple_form + && useless_type_conversion_p (type, TREE_TYPE (arg0))))) return pedantic_non_lvalue_loc (loc, arg0); /* Convert A ? 0 : 1 to !A. This prefers the use of NOT_EXPR over COND_EXPR in cases such as floating point comparisons. */ if (integer_zerop (op1) && (code == VEC_COND_EXPR ? integer_all_onesp (op2) : (integer_onep (op2) && !VECTOR_TYPE_P (type))) && truth_value_p (TREE_CODE (arg0))) return pedantic_non_lvalue_loc (loc, Index: tree-ssa-forwprop.c =================================================================== --- tree-ssa-forwprop.c (revision 200736) +++ tree-ssa-forwprop.c (working copy) @@ -1135,20 +1135,135 @@ forward_propagate_comparison (gimple_stm /* Remove defining statements. */ return remove_prop_source_from_use (name); bailout: gsi_next (defgsi); return false; } +/* Forward propagate the condition defined in *DEFGSI to uses in + binary operations. + Returns true if stmt is now unused. Advance DEFGSI to the next + statement. */ + +static bool +forward_propagate_condition (gimple_stmt_iterator *defgsi) +{ + gimple stmt = gsi_stmt (*defgsi); + tree name = gimple_assign_lhs (stmt); + enum tree_code defcode = gimple_assign_rhs_code (stmt); + gimple_stmt_iterator gsi; + enum tree_code code; + tree lhs, type; + gimple use_stmt; + + /* Don't propagate ssa names that occur in abnormal phis. */ + if ((TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME + && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs1 (stmt))) + || (TREE_CODE (gimple_assign_rhs2 (stmt)) == SSA_NAME + && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs2 (stmt))) + || (TREE_CODE (gimple_assign_rhs3 (stmt)) == SSA_NAME + && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs3 (stmt)))) + goto bailout; + + /* Do not un-cse, but propagate through copies. */ + use_stmt = get_prop_dest_stmt (name, &name); + if (!use_stmt + || !is_gimple_assign (use_stmt) + || gimple_has_side_effects (stmt) + || gimple_has_side_effects (use_stmt)) + goto bailout; + + gsi = gsi_for_stmt (use_stmt); + code = gimple_assign_rhs_code (use_stmt); + lhs = gimple_assign_lhs (use_stmt); + type = TREE_TYPE (lhs); + + if (TREE_CODE_CLASS (code) == tcc_binary + || TREE_CODE_CLASS (code) == tcc_comparison) + { + bool cond_is_left = false; + bool trueok = false; + bool falseok = false; + if (name == gimple_assign_rhs1 (use_stmt)) + cond_is_left = true; + else + gcc_assert (name == gimple_assign_rhs2 (use_stmt)); + tree true_op, false_op; + if (cond_is_left) + { + true_op = fold_build2_loc (gimple_location (stmt), code, type, + gimple_assign_rhs2 (stmt), + gimple_assign_rhs2 (use_stmt)); + false_op = fold_build2_loc (gimple_location (stmt), code, type, + gimple_assign_rhs3 (stmt), + gimple_assign_rhs2 (use_stmt)); + } + else + { + true_op = fold_build2_loc (gimple_location (stmt), code, type, + gimple_assign_rhs1 (use_stmt), + gimple_assign_rhs2 (stmt)); + false_op = fold_build2_loc (gimple_location (stmt), code, type, + gimple_assign_rhs1 (use_stmt), + gimple_assign_rhs3 (stmt)); + } + + if (is_gimple_val (true_op)) + trueok = true; + if (is_gimple_val (false_op)) + falseok = true; + /* At least one of them has to simplify, or it isn't worth it. */ + if (!trueok && !falseok) + goto bailout; + if (!trueok) + { + if (!valid_gimple_rhs_p (true_op)) + goto bailout; + gimple g = gimple_build_assign (make_ssa_name (type, NULL), true_op); + true_op = gimple_assign_lhs (g); + gsi_insert_before (&gsi, g, GSI_SAME_STMT); + } + if (!falseok) + { + if (!valid_gimple_rhs_p (false_op)) + goto bailout; + gimple g = gimple_build_assign (make_ssa_name (type, NULL), false_op); + false_op = gimple_assign_lhs (g); + gsi_insert_before (&gsi, g, GSI_SAME_STMT); + } + + gimple_assign_set_rhs_with_ops_1 (&gsi, defcode, + gimple_assign_rhs1 (stmt), + true_op, false_op); + } + else + goto bailout; + + fold_stmt (&gsi); + update_stmt (gsi_stmt (gsi)); + + /* When we remove stmt now the iterator defgsi goes off it's current + sequence, hence advance it now. */ + gsi_next (defgsi); + + /* Remove defining statements. */ + return remove_prop_source_from_use (name); + +bailout: + gsi_next (defgsi); + return false; +} + + /* GSI_P points to a statement which performs a narrowing integral conversion. Look for cases like: t = x & c; y = (T) t; Turn them into: @@ -3382,20 +3497,25 @@ ssa_forward_propagate_and_combine (void) gsi_next (&gsi); } else gsi_next (&gsi); } else if (TREE_CODE_CLASS (code) == tcc_comparison) { if (forward_propagate_comparison (&gsi)) cfg_changed = true; } + else if (code == COND_EXPR || code == VEC_COND_EXPR) + { + if (forward_propagate_condition (&gsi)) + cfg_changed = true; + } else gsi_next (&gsi); } /* Combine stmts with the stmts defining their operands. Note we update GSI within the loop as necessary. */ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);) { gimple stmt = gsi_stmt (gsi); bool changed = false; Index: testsuite/g++.dg/tree-ssa/pr57755.c =================================================================== --- testsuite/g++.dg/tree-ssa/pr57755.c (revision 0) +++ testsuite/g++.dg/tree-ssa/pr57755.c (revision 0) @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-forwprop1" } */ +typedef int vec __attribute__((vector_size(4*sizeof(int)))); + +vec f(vec a,vec b){ + vec z=a!=a; + vec o=z+1; + vec c=(a>3)?o:z; + vec d=(b>2)?o:z; + vec e=c&d; + return e!=0; +} + +vec g(vec a,vec b){ + vec z=a!=a; + vec c=(a<=42)?b:z; + return b&c; +} + +/* { dg-final { scan-tree-dump-times "VEC_COND_EXPR" 2 "forwprop1" } } */ +/* { dg-final { scan-tree-dump-not "!=" "forwprop1" } } */ +/* { dg-final { scan-tree-dump-not "&" "forwprop1" } } */ +/* { dg-final { cleanup-tree-dump "forwprop1" } } */