From patchwork Sat Nov 14 01:15:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 56556 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp99613lbb; Fri, 13 Nov 2015 17:15:52 -0800 (PST) X-Received: by 10.66.235.231 with SMTP id up7mr13225524pac.87.1447463751943; Fri, 13 Nov 2015 17:15:51 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id zk6si10410514pbc.252.2015.11.13.17.15.51 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Nov 2015 17:15:51 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-414084-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; spf=pass (google.com: domain of gcc-patches-return-414084-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-414084-patch=linaro.org@gcc.gnu.org; dkim=pass header.i=@gcc.gnu.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=g3XfHDK723rDIBhLX hPvdpZE6X6W+PqBpptQwXK+kZ1ZJ/ygkMQIa5DUoSL+421/fjOtLPnBTb1GR1NI4 RlNKlUWb8Lf5G+ZSIlfIX/uSnG7c4X9m0XqH/Gno3fVYSD+JM4tIRpyjvndOQpIM EqBkeQ4Q5L2C721gB2fEfFpGZw= 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=mc1bC7XGTRezXCfsxfcQ5z6 3A5Q=; b=KBBYTDDMWXHaCBzoYAqyQ0Ks4jh756B3b+ZpSk/z+cy1jCZrjDOneYi mAOXvoJpnKfD34D8ndora0eiEffgTULdUS9aO6jnAFLiEeqdMNST88bJ158VwPks boGq1H4e9hag+nH73WtoWw8a76qYPABQdqR3bmNWV3TZUTWBOMGI= Received: (qmail 63604 invoked by alias); 14 Nov 2015 01:15:31 -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 63594 invoked by uid 89); 14 Nov 2015 01:15:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f42.google.com Received: from mail-pa0-f42.google.com (HELO mail-pa0-f42.google.com) (209.85.220.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 14 Nov 2015 01:15:24 +0000 Received: by pacdm15 with SMTP id dm15so115641459pac.3 for ; Fri, 13 Nov 2015 17:15:22 -0800 (PST) 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:content-type; bh=QgTi+11yp27aCpHth/PMS1chZ3Epr6UgoqiZ7ZKAZLI=; b=ZNEkdDQ9kfaRqCAg95jD/lp9wrV+kxVE9dswMDtrt+4oV80sKFtgP+RH9SRiKXgwfO 3W6f4EK/dql3jXP0rZ0+/gBu7JTOskCwnE6YVImCZGOs4ngC0PxuqCKz/Jikf3SKcaHh Jekp5XPakePnum5/Q1nvjlM3Sa4ikqsIxsV4NZkZM1hmMQ0wzVNsJ/IvMhzasMYs+DKb 9gtgPKiFyUoG2BMfFPld4DBs1U3D3rASmefACGdKnSqQjHUosv1rNRilBwku+Qv3x7y+ v0K8Lo4g20bx5ILqsFOB5L8nuLD5tbFIhOeb9ovxFM0S5OdLNKho+W/LbNDu0IO8BYyy ClcQ== X-Gm-Message-State: ALoCoQkX+pUaGhmNz5mjRxYUR8oOH8VClMPXYKivaNZioCEjUotK+tU6soNiSIHkxElbBmB2ppEJ X-Received: by 10.68.234.166 with SMTP id uf6mr36409266pbc.126.1447463721774; Fri, 13 Nov 2015 17:15:21 -0800 (PST) Received: from [10.1.1.10] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.googlemail.com with ESMTPSA id cs7sm22728910pad.35.2015.11.13.17.15.18 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 13 Nov 2015 17:15:20 -0800 (PST) Subject: Re: [0/7] Type promotion pass and elimination of zext/sext To: Richard Biener References: <55ECFC2A.7050908@linaro.org> <56269E01.5010408@linaro.org> <5628BF86.6070503@linaro.org> <562ECA5D.2040008@linaro.org> <56372A31.3080607@linaro.org> <563F192A.7020004@linaro.org> Cc: "gcc-patches@gcc.gnu.org" From: Kugan Message-ID: <56468B17.6020903@linaro.org> Date: Sat, 14 Nov 2015 12:15:03 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Attached is the latest version of the patch. With the patches 0001-Add-new-SEXT_EXPR-tree-code.patch, 0002-Add-type-promotion-pass.patch and 0003-Optimize-ZEXT_EXPR-with-tree-vrp.patch. I did bootstrap on ppc64-linux-gnu, aarch64-linux-gnu and x64-64-linux-gnu and regression testing on ppc64-linux-gnu, aarch64-linux-gnu arm64-linux-gnu and x64-64-linux-gnu. I ran into three issues in ppc64-linux-gnu regression testing. There are some other test cases which needs adjustment for scanning for some patterns that are not valid now. 1. rtl fwprop was going into infinite loop. Works with the following patch: /* forward_propagate_subreg may be operating on an instruction with 2. gcc.dg/torture/ftrapv-1.c fails This is because we are checking for the SImode trapping. With the promotion of the operation to wider mode, this is i think expected. I think the testcase needs updating. 3. gcc.dg/sms-3.c fails It fails with -fmodulo-sched-allow-regmoves and OK when I remove it. I am looking into it. I also have the following issues based on the previous review (as posted in the previous patch). Copying again for the review purpose. 1. > you still call promote_ssa on both DEFs and USEs and promote_ssa looks > at SSA_NAME_DEF_STMT of the passed arg. Please call promote_ssa just > on DEFs and fixup_uses on USEs. I am doing this to promote SSA that are defined with GIMPLE_NOP. Is there anyway to iterate over this. I have added gcc_assert to make sure that promote_ssa is called only once. 2. > Instead of this you should, in promote_all_stmts, walk over all uses doing what > fixup_uses does and then walk over all defs, doing what promote_ssa does. > > + case GIMPLE_NOP: > + { > + if (SSA_NAME_VAR (def) == NULL) > + { > + /* Promote def by fixing its type for anonymous def. */ > + TREE_TYPE (def) = promoted_type; > + } > + else > + { > + /* Create a promoted copy of parameters. */ > + bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); > > I think the uninitialized vars are somewhat tricky and it would be best > to create a new uninit anonymous SSA name for them. You can > have SSA_NAME_VAR != NULL and def _not_ being a parameter > btw. I experimented with get_or_create_default_def. Here we have to have a SSA_NAME_VAR (def) of promoted type. In the attached patch I am doing the following and seems to work. Does this looks OK? + } + else if (TREE_CODE (SSA_NAME_VAR (def)) != PARM_DECL) + { + tree var = copy_node (SSA_NAME_VAR (def)); + TREE_TYPE (var) = promoted_type; + TREE_TYPE (def) = promoted_type; + SET_SSA_NAME_VAR_OR_IDENTIFIER (def, var); + } I prefer to promote def as otherwise iterating over the uses and promoting can look complicated (have to look at all the different types of stmts again and do the right thing as It was in the earlier version of this before we move to this approach) 3) > you can also transparently handle constants for the cases where promoting > is required. At the moment their handling is interwinded with the def promotion > code. That makes the whole thing hard to follow. I have updated the comments with: +/* Promote constants in STMT to TYPE. If PROMOTE_COND_EXPR is true, + promote only the constants in conditions part of the COND_EXPR. + + We promote the constants when the associated operands are promoted. + This usually means that we promote the constants when we promote the + defining stmnts (as part of promote_ssa). However for COND_EXPR, we + can promote only when we promote the other operand. Therefore, this + is done during fixup_use. */ 4) I am handling gimple_debug separately to avoid any code difference with and without -g option. I have updated the comments for this. 5) I also noticed that tree-ssa-uninit sometimes gives false positives due to the assumptions it makes. Is it OK to move this pass before type promotion? I can do the testings and post a separate patch with this if this OK. 6) I also removed the optimization that prevents some of the redundant truncation/extensions from type promotion pass, as it dosent do much as of now. I can send a proper follow up patch. Is that OK? I also did a simple test with coremark for the latest patch. I compared the code size for coremark for linux-gcc with -Os. Results are as reported by the "size" utility. I know this doesn't mean much but can give some indication. Base with pass Percentage improvement ============================================================== arm 10476 10372 0.9927453226 aarch64 9545 9521 0.2514405448 ppc64 12236 12052 1.5037593985 After resolving the above issues, I would like propose that we commit the pass as not enabled by default (even though the patch as it stands enabled by default - I am doing it for testing purposes). Thanks, Kugan >From c0ce364e3a422912a08189645efde46c36583753 Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah Date: Thu, 22 Oct 2015 10:51:42 +1100 Subject: [PATCH 1/5] Add new SEXT_EXPR tree code --- gcc/cfgexpand.c | 12 ++++++++++++ gcc/expr.c | 20 ++++++++++++++++++++ gcc/fold-const.c | 4 ++++ gcc/tree-cfg.c | 12 ++++++++++++ gcc/tree-inline.c | 1 + gcc/tree-pretty-print.c | 11 +++++++++++ gcc/tree.def | 5 +++++ 7 files changed, 65 insertions(+) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index eaad859..aeb64bb 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5054,6 +5054,18 @@ expand_debug_expr (tree exp) case FMA_EXPR: return simplify_gen_ternary (FMA, mode, inner_mode, op0, op1, op2); + case SEXT_EXPR: + gcc_assert (CONST_INT_P (op1)); + inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0); + gcc_assert (GET_MODE_BITSIZE (inner_mode) == INTVAL (op1)); + + if (mode != inner_mode) + op0 = simplify_gen_unary (SIGN_EXTEND, + mode, + gen_lowpart_SUBREG (inner_mode, op0), + inner_mode); + return op0; + default: flag_unsupported: #ifdef ENABLE_CHECKING diff --git a/gcc/expr.c b/gcc/expr.c index da68870..c2f535f 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -9318,6 +9318,26 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode, target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target); return target; + case SEXT_EXPR: + { + machine_mode inner_mode = mode_for_size (tree_to_uhwi (treeop1), + MODE_INT, 0); + rtx temp, result; + rtx op0 = expand_normal (treeop0); + op0 = force_reg (mode, op0); + if (mode != inner_mode) + { + result = gen_reg_rtx (mode); + temp = simplify_gen_unary (SIGN_EXTEND, mode, + gen_lowpart_SUBREG (inner_mode, op0), + inner_mode); + convert_move (result, temp, 0); + } + else + result = op0; + return result; + } + default: gcc_unreachable (); } diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 602ea24..a149bad 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -987,6 +987,10 @@ int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree parg2, res = wi::bit_and (arg1, arg2); break; + case SEXT_EXPR: + res = wi::sext (arg1, arg2.to_uhwi ()); + break; + case RSHIFT_EXPR: case LSHIFT_EXPR: if (wi::neg_p (arg2)) diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 8e3e810..d18b3f7 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -3752,6 +3752,18 @@ verify_gimple_assign_binary (gassign *stmt) return false; } + case SEXT_EXPR: + { + if (!INTEGRAL_TYPE_P (lhs_type) + || !useless_type_conversion_p (lhs_type, rhs1_type) + || !tree_fits_uhwi_p (rhs2)) + { + error ("invalid operands in sext expr"); + return true; + } + return false; + } + case VEC_WIDEN_LSHIFT_HI_EXPR: case VEC_WIDEN_LSHIFT_LO_EXPR: { diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index b8269ef..e61c200 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -3893,6 +3893,7 @@ estimate_operator_cost (enum tree_code code, eni_weights *weights, case BIT_XOR_EXPR: case BIT_AND_EXPR: case BIT_NOT_EXPR: + case SEXT_EXPR: case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index 11f90051..bec9082 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -1923,6 +1923,14 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, int flags, } break; + case SEXT_EXPR: + pp_string (pp, "SEXT_EXPR <"); + dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false); + pp_string (pp, ", "); + dump_generic_node (pp, TREE_OPERAND (node, 1), spc, flags, false); + pp_greater (pp); + break; + case MODIFY_EXPR: case INIT_EXPR: dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, @@ -3561,6 +3569,9 @@ op_symbol_code (enum tree_code code) case MIN_EXPR: return "min"; + case SEXT_EXPR: + return "sext"; + default: return "<<< ??? >>>"; } diff --git a/gcc/tree.def b/gcc/tree.def index d0a3bd6..789cfdd 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -760,6 +760,11 @@ DEFTREECODE (BIT_XOR_EXPR, "bit_xor_expr", tcc_binary, 2) DEFTREECODE (BIT_AND_EXPR, "bit_and_expr", tcc_binary, 2) DEFTREECODE (BIT_NOT_EXPR, "bit_not_expr", tcc_unary, 1) +/* Sign-extend operation. It will sign extend first operand from + the sign bit specified by the second operand. The type of the + result is that of the first operand. */ +DEFTREECODE (SEXT_EXPR, "sext_expr", tcc_binary, 2) + /* ANDIF and ORIF allow the second operand not to be computed if the value of the expression is determined from the first operand. AND, OR, and XOR always compute the second operand whether its value is -- 1.9.1