From patchwork Thu May 5 00:41:18 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 67190 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp480136qge; Wed, 4 May 2016 17:42:11 -0700 (PDT) X-Received: by 10.66.218.161 with SMTP id ph1mr16046981pac.83.1462408931572; Wed, 04 May 2016 17:42:11 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id to9si7787029pab.69.2016.05.04.17.42.11 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 04 May 2016 17:42:11 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-426618-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-426618-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-426618-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=OdPltvalECNrhJvCe tl0ylZ025iUV0MLBMtUj64s6ny5mrI3Kp8WLv+aCfED8pEbjt0Do7/xrwG+CCXOF VX/tvxMJHljBVvh2DYf0XDN2oWEq13K8YnV0wHibbGT63mAsvdD69L8zXqoW/LUC o+qmPCFPBW9qz1BvbxGp7Msdc8= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=rriI/Tb+5yWiJJ7fxmqfKDv ZIEM=; b=xSleuUPZUKUnWX1TVMVoXxI4S861fC3PZ/vvi3nZNms1yTSSZ36TniS 5BO9ihQ05tcQFQcFqJFV7u05qiRv8VtWjzSk0aMixVbJS5AjREpqwitMNqRW2+8P PcCjS7Q2O/K70UUme8GR8lajbOAVbHtF8VX74qUZG/rXlZKv7Kwo= Received: (qmail 125254 invoked by alias); 5 May 2016 00:41:55 -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 125238 invoked by uid 89); 5 May 2016 00:41:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=UD:tree-ssa-reassoc.c, unsuccessful, Hx-languages-length:7558 X-HELO: mail-pa0-f47.google.com Received: from mail-pa0-f47.google.com (HELO mail-pa0-f47.google.com) (209.85.220.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 05 May 2016 00:41:44 +0000 Received: by mail-pa0-f47.google.com with SMTP id bt5so29397103pac.3 for ; Wed, 04 May 2016 17:41:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=JkuaCV7zD49YO5aN2HCO2G910bZ95Mfsx3yS05sH++c=; b=j6Aq3zpIigyhbQKWA+6kefNoeUY0GOuvkgfOJtE+wT8goJQ29/BWtUqzQSigZVS+U/ xZtN+jZiPVGHNmNxpVYbHHL+lvFwat9EXlVg3Ipx7WHVyk23Jjvsr8EdxH3l8utwKJuv iuAGfbFEcSCqVC19VzqwEY6VoGaE2cFPzrs5N0ZlMey27CcMhtOxIwyiOOa0rkIuT/v2 pDhOrCOr6+tX0UOFcfa0FK3YHEY5YvIUhzc1LFCeZXUam25i86MtWcwDKaA1LQSd3W1w Q4NZopAE1g7H47XKM00AkHKaLiYU7IFdOY0Oas9C8kTpsmmLmqOgTzIWsuyr1d/uNK2b 2xmw== X-Gm-Message-State: AOPr4FUWZrYyh5FG2ayS1SnOhPilcgCa8u/eox4fWALX/mXy9JFyrlivtqwJGnzVr8zUc0Mr X-Received: by 10.66.194.230 with SMTP id hz6mr16450005pac.132.1462408902950; Wed, 04 May 2016 17:41:42 -0700 (PDT) Received: from [10.1.1.8] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.googlemail.com with ESMTPSA id h5sm8761634pat.0.2016.05.04.17.41.40 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 04 May 2016 17:41:42 -0700 (PDT) Subject: Re: [RFC][PATCH][PR40921] Convert x + (-y * z * z) into x - y * z * z To: Richard Biener References: <56CFC02F.2070801@linaro.org> <56D42341.5090607@linaro.org> <5718B5B1.8030809@linaro.org> <571B7448.4080005@linaro.org> Cc: "gcc-patches@gcc.gnu.org" From: kugan Message-ID: <572A96AE.10701@linaro.org> Date: Thu, 5 May 2016 10:41:18 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hi Richard, > > + int last = ops.length () - 1; > + bool negate_result = false; > > Do > > oe &last = ops.last (); > Done. > > + if (rhs_code == MULT_EXPR > + && ops.length () > 1 > + && ((TREE_CODE (ops[last]->op) == INTEGER_CST > > and last.op here and below > > + && integer_minus_onep (ops[last]->op)) > + || ((TREE_CODE (ops[last]->op) == REAL_CST) > + && real_equal (&TREE_REAL_CST > (ops[last]->op), &dconstm1)))) > Done. > Here the checks !HONOR_SNANS () && (!HONOS_SIGNED_ZEROS || > !COMPLEX_FLOAT_TYPE_P) > are missing. The * -1 might appear literally and you are only allowed > to turn it into a negate > under the above conditions. Done. > > + { > + ops.unordered_remove (last); > > use ops.pop (); > Done. > + negate_result = true; > > Please move the whole thing under the else { } case of the ops.length > == 0, ops.length == 1 test chain > as you did for the actual emit of the negate. > I see your point. However, when we remove the (-1) from the ops list, that intern can result in ops.length becoming 1. Therefore, I moved the the following if (negate_result), outside the condition. > > + if (negate_result) > + { > + tree tmp = make_ssa_name (TREE_TYPE (lhs)); > + gimple_set_lhs (stmt, tmp); > + gassign *neg_stmt = gimple_build_assign (lhs, NEGATE_EXPR, > + tmp); > + gimple_stmt_iterator gsi = gsi_for_stmt (stmt); > + gsi_insert_after (&gsi, neg_stmt, GSI_NEW_STMT); > + update_stmt (stmt); > > I think that if powi_result is also built you end up using the wrong > stmt so you miss a > > stmt = SSA_NAME_DEF_STMT (lhs); Yes, indeed. This can happen and I have added this. > > here. Also see the new_lhs handling of the powi_result case - again > you need sth > similar here (it's handling looks a bit fishy as well - this all needs > some comments > and possibly a (lot more) testcases). > > So, please do the above requested changes and verify the 'lhs' issues I pointed > out by trying to add a few more testcase that also cover the case where a powi > is detected in addition to a negation. Please also add a testcase that catches > (-y) * x * (-z). > Added this to the testcase. Does this look better now? Thanks, Kugan >> 2016-04-23 Kugan Vivekanandarajah >> >> PR middle-end/40921 >> * gcc.dg/tree-ssa/pr40921.c: New test. >> >> gcc/ChangeLog: >> >> 2016-04-23 Kugan Vivekanandarajah >> >> PR middle-end/40921 >> * tree-ssa-reassoc.c (try_special_add_to_ops): New. >> (linearize_expr_tree): Call try_special_add_to_ops. >> (reassociate_bb): Convert MULT_EXPR by (-1) to NEGATE_EXPR. >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr40921.c b/gcc/testsuite/gcc.dg/tree-ssa/pr40921.c index e69de29..3a5a23a 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr40921.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr40921.c @@ -0,0 +1,26 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized -ffast-math" } */ + +unsigned int foo (unsigned int x, unsigned int y, unsigned int z) +{ + return x + (-y * z * z); +} + +float bar (float x, float y, float z) +{ + return x + (-y * z * z); +} + +float bar2 (float x, float y, float z) +{ + return x + (-y * z * z * 5.0f); +} + +float bar3 (float x, float y, float z) +{ + return x + (-y * x * -z); +} + + +/* { dg-final { scan-tree-dump-times "_* = -y_" 0 "optimized" } } */ diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index 4e1251b..1df6681 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -4112,6 +4112,45 @@ acceptable_pow_call (gimple *stmt, tree *base, HOST_WIDE_INT *exponent) return true; } +/* Try to derive and add operand entry for OP to *OPS. Return false if + unsuccessful. */ + +static bool +try_special_add_to_ops (vec *ops, + enum tree_code code, + tree op, gimple* def_stmt) +{ + tree base = NULL_TREE; + HOST_WIDE_INT exponent = 0; + + if (TREE_CODE (op) != SSA_NAME) + return false; + + if (code == MULT_EXPR + && acceptable_pow_call (def_stmt, &base, &exponent)) + { + add_repeat_to_ops_vec (ops, base, exponent); + gimple_set_visited (def_stmt, true); + return true; + } + else if (code == MULT_EXPR + && is_gimple_assign (def_stmt) + && gimple_assign_rhs_code (def_stmt) == NEGATE_EXPR + && !HONOR_SNANS (TREE_TYPE (op)) + && (!HONOR_SIGNED_ZEROS (TREE_TYPE (op)) + || !COMPLEX_FLOAT_TYPE_P (TREE_TYPE (op)))) + { + tree rhs1 = gimple_assign_rhs1 (def_stmt); + tree cst = build_minus_one_cst (TREE_TYPE (op)); + add_to_ops_vec (ops, rhs1); + add_to_ops_vec (ops, cst); + gimple_set_visited (def_stmt, true); + return true; + } + + return false; +} + /* Recursively linearize a binary expression that is the RHS of STMT. Place the operands of the expression tree in the vector named OPS. */ @@ -4126,8 +4165,6 @@ linearize_expr_tree (vec *ops, gimple *stmt, bool binrhsisreassoc = false; enum tree_code rhscode = gimple_assign_rhs_code (stmt); struct loop *loop = loop_containing_stmt (stmt); - tree base = NULL_TREE; - HOST_WIDE_INT exponent = 0; if (set_visited) gimple_set_visited (stmt, true); @@ -4163,24 +4200,10 @@ linearize_expr_tree (vec *ops, gimple *stmt, if (!binrhsisreassoc) { - if (rhscode == MULT_EXPR - && TREE_CODE (binrhs) == SSA_NAME - && acceptable_pow_call (binrhsdef, &base, &exponent)) - { - add_repeat_to_ops_vec (ops, base, exponent); - gimple_set_visited (binrhsdef, true); - } - else + if (!try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef)) add_to_ops_vec (ops, binrhs); - if (rhscode == MULT_EXPR - && TREE_CODE (binlhs) == SSA_NAME - && acceptable_pow_call (binlhsdef, &base, &exponent)) - { - add_repeat_to_ops_vec (ops, base, exponent); - gimple_set_visited (binlhsdef, true); - } - else + if (!try_special_add_to_ops (ops, rhscode, binlhs, binlhsdef)) add_to_ops_vec (ops, binlhs); return; @@ -4220,14 +4243,7 @@ linearize_expr_tree (vec *ops, gimple *stmt, linearize_expr_tree (ops, SSA_NAME_DEF_STMT (binlhs), is_associative, set_visited); - if (rhscode == MULT_EXPR - && TREE_CODE (binrhs) == SSA_NAME - && acceptable_pow_call (SSA_NAME_DEF_STMT (binrhs), &base, &exponent)) - { - add_repeat_to_ops_vec (ops, base, exponent); - gimple_set_visited (SSA_NAME_DEF_STMT (binrhs), true); - } - else + if (!try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef)) add_to_ops_vec (ops, binrhs); } @@ -4980,6 +4996,22 @@ reassociate_bb (basic_block bb) && flag_unsafe_math_optimizations) powi_result = attempt_builtin_powi (stmt, &ops); + operand_entry *last = ops.last (); + bool negate_result = false; + if (rhs_code == MULT_EXPR + && ops.length () > 1 + && ((TREE_CODE (last->op) == INTEGER_CST + && integer_minus_onep (last->op)) + || ((TREE_CODE (last->op) == REAL_CST) + && real_equal (&TREE_REAL_CST (last->op), &dconstm1))) + && !HONOR_SNANS (TREE_TYPE (lhs)) + && (!HONOR_SIGNED_ZEROS (TREE_TYPE (lhs)) + || !COMPLEX_FLOAT_TYPE_P (TREE_TYPE (lhs)))) + { + ops.pop (); + negate_result = true; + } + /* If the operand vector is now empty, all operands were consumed by the __builtin_powi optimization. */ if (ops.length () == 0) @@ -5042,6 +5074,18 @@ reassociate_bb (basic_block bb) gsi_insert_after (&gsi, mul_stmt, GSI_NEW_STMT); } } + + if (negate_result) + { + stmt = SSA_NAME_DEF_STMT (lhs); + tree tmp = make_ssa_name (TREE_TYPE (lhs)); + gimple_set_lhs (stmt, tmp); + gassign *neg_stmt = gimple_build_assign (lhs, NEGATE_EXPR, + tmp); + gimple_stmt_iterator gsi = gsi_for_stmt (stmt); + gsi_insert_after (&gsi, neg_stmt, GSI_NEW_STMT); + update_stmt (stmt); + } } } }