From patchwork Thu Oct 15 05:49:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 530504 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 624C41402B6 for ; Thu, 15 Oct 2015 16:49:39 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Z45v4NkH; dkim-atps=neutral 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=tWZw2OWIq5Xx34RBF F1yWkNq2UoMMBWFL7tfkDj0VFBk3KZdMVv5Jp2DyXKGY2iuD8YPcthoCkELvkHIv BV1DF9uKuMrNQoYglSLoooH9YNCWxs7vF/ePCadSLROCxx/hNIq7y26ZsBJJC7He y9vLwQbsMADIr/ccHWqEqqM4Ds= 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=ibCkTokofYfM+hJ1PhX9top 929U=; b=Z45v4NkHLOC6LtL7omPeXkxsOG7SCYH0I0aPV5aSYQcmhlvOmUu1PeZ 4+K2JYqzs5jy5s80EFId8el0w7h1Q56iYGVRXD3VA6+473j8aEoB5GSWqf1lxkCW elZSCrTshoM9Y7Gymh8BsE3rpEA7u5jKd9XnowWzDztde5pn3tqs= Received: (qmail 120383 invoked by alias); 15 Oct 2015 05:49:32 -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 120330 invoked by uid 89); 15 Oct 2015 05:49: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; Thu, 15 Oct 2015 05:49:28 +0000 Received: by pabrc13 with SMTP id rc13so76866632pab.0 for ; Wed, 14 Oct 2015 22:49:26 -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:content-type; bh=BQY4TVghdEjvPOV+4ZTE5w4nLeVUOWJt9ylA24ImZmU=; b=mZ7gKiHEhELAfxFxsFV80H0fWU7gFAsKFLjxZ2oRatcq/KqYmGE1cimwdMpz0M4gG1 fyLHofAaVossiMrBIGNAaVjjQauZ7ljZepK0tERTSX4PtF2soI+Y9AaTUNLqGO+DcE1G WG1uja/oYWCJRhfhgZoURKW02H1US+J9GTHTzeCMQ+cN+kEwzByvgzEAO1zSZgZQoqxC raCMB7RCNORw3i3bdTjVnsfnu+o1bikBXDI0HKW1ZLJdWfnAaOGJNoUKk4hNOv/SBXyX IgSNVBjaO6EnAnTnIfxhmt4RgWRRHntwl+WzkWyRPG6gVxW5C3AEozg68SAw+ogw3N6r Hxiw== X-Gm-Message-State: ALoCoQkKXtDo9l5IWoFq3SESLF6UDFmZqkPvky0xEbfuAjOolzNkLdP4cWkt0Ap2PqLNuiWtOqYB X-Received: by 10.66.145.97 with SMTP id st1mr7870198pab.145.1444888166524; Wed, 14 Oct 2015 22:49:26 -0700 (PDT) Received: from [10.1.1.5] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.googlemail.com with ESMTPSA id qk7sm7267900pbb.80.2015.10.14.22.49.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Oct 2015 22:49:25 -0700 (PDT) Subject: Re: [1/7] Add new tree code SEXT_EXPR To: Richard Biener References: <55ECFC2A.7050908@linaro.org> <55ECFC9C.8050204@linaro.org> <561A3B7E.2060405@linaro.org> Cc: "gcc-patches@gcc.gnu.org" From: Kugan Message-ID: <561F3E61.7060906@linaro.org> Date: Thu, 15 Oct 2015 16:49:21 +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 On 12/10/15 23:21, Richard Biener wrote: > On Sun, Oct 11, 2015 at 12:35 PM, Kugan > wrote: >> >> >> On 15/09/15 23:18, Richard Biener wrote: >>> On Mon, Sep 7, 2015 at 4:55 AM, Kugan wrote: >>>> >>>> This patch adds support for new tree code SEXT_EXPR. >>> >>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c >>> index d567a87..bbc3c10 100644 >>> --- a/gcc/cfgexpand.c >>> +++ b/gcc/cfgexpand.c >>> @@ -5071,6 +5071,10 @@ expand_debug_expr (tree exp) >>> case FMA_EXPR: >>> return simplify_gen_ternary (FMA, mode, inner_mode, op0, op1, op2); >>> >>> + case SEXT_EXPR: >>> + return op0; >>> >>> that looks wrong. Generate (sext:... ) here? >>> >>> + case SEXT_EXPR: >>> + { >>> + rtx op0 = expand_normal (treeop0); >>> + rtx temp; >>> + if (!target) >>> + target = gen_reg_rtx (TYPE_MODE (TREE_TYPE (treeop0))); >>> + >>> + machine_mode inner_mode >>> + = smallest_mode_for_size (tree_to_shwi (treeop1), >>> + MODE_INT); >>> + temp = convert_modes (inner_mode, >>> + TYPE_MODE (TREE_TYPE (treeop0)), op0, 0); >>> + convert_move (target, temp, 0); >>> + return target; >>> + } >>> >>> Humm - is that really how we expand sign extensions right now? No helper >>> that would generate (sext ...) directly? I wouldn't try using 'target' btw but >>> simply return (sext:mode op0 op1) or so. But I am no way an RTL expert. >>> >>> Note that if we don't disallow arbitrary precision SEXT_EXPRs we have to >>> fall back to using shifts (and smallest_mode_for_size is simply wrong). >>> >>> + case SEXT_EXPR: >>> + { >>> + if (!INTEGRAL_TYPE_P (lhs_type) >>> + || !INTEGRAL_TYPE_P (rhs1_type) >>> + || TREE_CODE (rhs2) != INTEGER_CST) >>> >>> please constrain this some more, with >>> >>> || !useless_type_conversion_p (lhs_type, rhs1_type) >>> >>> + { >>> + error ("invalid operands in sext expr"); >>> + return true; >>> + } >>> + return false; >>> + } >>> >>> @@ -3414,6 +3422,9 @@ op_symbol_code (enum tree_code code) >>> case MIN_EXPR: >>> return "min"; >>> >>> + case SEXT_EXPR: >>> + return "sext from bit"; >>> + >>> >>> just "sext" please. >>> >>> +/* Sign-extend operation. It will sign extend first operand from >>> + the sign bit specified by the second operand. */ >>> +DEFTREECODE (SEXT_EXPR, "sext_expr", tcc_binary, 2) >>> >>> "from the INTEGER_CST sign bit specified" >>> >>> Also add "The type of the result is that of the first operand." >>> >> >> >> >> Thanks for the review. Attached patch attempts to address the above >> comments. Does this look better? > > + case SEXT_EXPR: > + gcc_assert (CONST_INT_P (op1)); > + inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0); > > We should add > > 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); > > as we're otherwise silently dropping things like SEXT (short-typed-var, 13) > > + case SEXT_EXPR: > + { > + machine_mode inner_mode = mode_for_size (tree_to_shwi (treeop1), > + MODE_INT, 0); > > Likewise. Also treeop1 should be unsigned, thus tree_to_uhwi? > > + rtx temp, result; > + rtx op0 = expand_normal (treeop0); > + op0 = force_reg (mode, op0); > + if (mode != inner_mode) > + { > > Again, for the RTL bits I'm not sure they are correct. For example I don't > see why we need a lowpart SUBREG, isn't a "regular" SUBREG enough? > > + case SEXT_EXPR: > + { > + if (!INTEGRAL_TYPE_P (lhs_type) > + || !useless_type_conversion_p (lhs_type, rhs1_type) > + || !INTEGRAL_TYPE_P (rhs1_type) > + || TREE_CODE (rhs2) != INTEGER_CST) > > the INTEGRAL_TYPE_P (rhs1_type) check is redundant with > the useless_type_Conversion_p one. Please check > tree_fits_uhwi (rhs2) instead of != INTEGER_CST. > Thanks for the review, Please find the updated patch based on the review comments. Thanks, Kugan From e600c266ac7932ffe4cb36830e8d62c90f6e26ee Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah Date: Mon, 17 Aug 2015 13:37:15 +1000 Subject: [PATCH 1/7] 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 58e55d2..357710b 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5057,6 +5057,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 0bbfccd..63bd1b6 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -9296,6 +9296,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 7231fd6..d693b42 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -984,6 +984,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 712d8cc..03ae758 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -3756,6 +3756,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 ac9586e..0975730 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -3884,6 +3884,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 7cd1fe7..efd8d5b 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -1794,6 +1794,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, @@ -3414,6 +3422,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 56580af..48e7413 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -752,6 +752,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