From patchwork Fri Sep 25 15:04:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Greenhalgh X-Patchwork-Id: 54167 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f197.google.com (mail-wi0-f197.google.com [209.85.212.197]) by patches.linaro.org (Postfix) with ESMTPS id 71CF2218DB for ; Fri, 25 Sep 2015 15:05:28 +0000 (UTC) Received: by wicmn1 with SMTP id mn1sf9467170wic.1 for ; Fri, 25 Sep 2015 08:05:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-type:x-original-sender :x-original-authentication-results; bh=gKN699LnnpYGJklWYMFYbTS360iatN5Pax0RljayjEk=; b=aXW3gu/awfVMbUhgouVIzDcLz+redgsBQPHp3bif9Y+WqLoF/DOc+OnCyFJ9H1eNRw CKKJ38gpi4v3OjHw/+/WN6Jc0rUT6v3qCy898FJHx/YqYXs1bon7gJMdXujI7t4UBvg6 dFTEjkjvUxziDDUiR/5np8CVttV8ABghNJaqx4d779/bGHfdCd/CFvLAjLh/W4P375WF mliIIsHW+vryRriyFqPXu6t0bM5OEr+zCgK8ol04APuDP2mXKvZGAce906BW9zc4Ehuu 0MS0TbFXiqUxOWt5fgiceMYZOFlS2qcu7kceNGA+F2RdHF6A7ljg8OARCCzoHJYhsgVt QULA== X-Gm-Message-State: ALoCoQlx1tLM6iXIR/4TP9hOHDK/vYpsbVO+Rw+/q15YhrhB3hOPfzwYXmDCjbtDrLZSUNK/sdpm X-Received: by 10.112.149.39 with SMTP id tx7mr1015178lbb.11.1443193527424; Fri, 25 Sep 2015 08:05:27 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.179.8 with SMTP id dc8ls171057lac.12.gmail; Fri, 25 Sep 2015 08:05:27 -0700 (PDT) X-Received: by 10.25.213.8 with SMTP id m8mr1163935lfg.114.1443193527102; Fri, 25 Sep 2015 08:05:27 -0700 (PDT) Received: from mail-la0-x229.google.com (mail-la0-x229.google.com. [2a00:1450:4010:c03::229]) by mx.google.com with ESMTPS id dn8si1900212lac.119.2015.09.25.08.05.26 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 25 Sep 2015 08:05:26 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::229 as permitted sender) client-ip=2a00:1450:4010:c03::229; Received: by laclj5 with SMTP id lj5so6380735lac.3 for ; Fri, 25 Sep 2015 08:05:26 -0700 (PDT) X-Received: by 10.152.23.97 with SMTP id l1mr1873520laf.56.1443193526560; Fri, 25 Sep 2015 08:05:26 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.59.35 with SMTP id w3csp94745lbq; Fri, 25 Sep 2015 08:05:25 -0700 (PDT) X-Received: by 10.68.65.13 with SMTP id t13mr2715658pbs.43.1443193525079; Fri, 25 Sep 2015 08:05:25 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id nq1si6222679pbc.127.2015.09.25.08.05.24 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 25 Sep 2015 08:05:25 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-408351-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 10331 invoked by alias); 25 Sep 2015 15:05:07 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 10165 invoked by uid 89); 25 Sep 2015 15:05:06 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 25 Sep 2015 15:04:58 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-30-wZh_G5HaTmurqBFcyA066w-1; Fri, 25 Sep 2015 16:04:53 +0100 Received: from e107456-lin.cambridge.arm.com ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 25 Sep 2015 16:04:53 +0100 From: James Greenhalgh To: gcc-patches@gcc.gnu.org Cc: ramana.radhakrishnan@arm.com, bernds_cb1@t-online.de, law@redhat.com, ebotcazou@libertysurf.fr, steven@gcc.gnu.org Subject: [Patch ifcvt 1/3] Factor out cost calculations from noce cases Date: Fri, 25 Sep 2015 16:04:36 +0100 Message-Id: <1443193479-10830-2-git-send-email-james.greenhalgh@arm.com> In-Reply-To: <1443193479-10830-1-git-send-email-james.greenhalgh@arm.com> References: <20150911095608.GA2448@arm.com> <1443193479-10830-1-git-send-email-james.greenhalgh@arm.com> MIME-Version: 1.0 X-MC-Unique: wZh_G5HaTmurqBFcyA066w-1 X-IsSubscribed: yes X-Original-Sender: james.greenhalgh@arm.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::229 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 Hi, In this patch we try to pull out the cost calculations used by the no-conditional-execution if-convert functions. We want to replicate the logic of the current cost decisions, but to phrase it in a way which can be pulled out as common. To preserve the current behaviour as best as we can, this means asking the common question, "is a magic_number less than or equal to branch_cost". Clearly this is not the question we want to be asking longer term, but this preserves existing target behaviour. This is imperfect for a few reasons. First, some of the more ambitious noce if-convert functions have a (slightly) more complicated cost-model. This means that we have to jump through hoops to present the cost calculation in the common form. These hoops are not very big, but it does make the logic seem a bit... weird. Second, because our long term goal is to hand the cost calculation off to the target and make it better reflect a meaningful question, we must first build the candidate ifcvt sequence for comparison. This will cause a slight compile time regression as we now generate more sequences before bailing out (each of which needs a cost calculation). On the other hand, it should be clear from this point what we have to do to lift this out to a target hook which can do a smart job, and I think this fits with the overall direction we intend to take. Bootstrapped and checked on x86_64-none-linux-gnu, aarch64-none-linux-gnu and arm-none-linux-gnueabihf without issue. Comparison of Spec2000/Spec2006 code generation for these three targets showed no changes. OK? Thanks, James --- 2015-09-26 James Greenhalgh * ifcvt.c (noce_if_info): Add a magic_number field :-(. (noce_is_profitable_p): New. (noce_try_store_flag_constants): Move cost calculation to after sequence generation, factor it out to noce_is_profitable_p. (noce_try_addcc): Likewise. (noce_try_store_flag_mask): Likewise. (noce_try_cmove): Likewise. (noce_try_cmove_arith): Likewise. (noce_try_sign_mask): Add comment regarding cost calculations. diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 157a716..e89d567 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -820,6 +820,14 @@ struct noce_if_info /* Estimated cost of the particular branch instruction. */ unsigned int branch_cost; + + /* For if-convert transformations, the legacy way to decide whether + the transformation should be applied is a comparison of a magic + number against BRANCH_COST. Ultimately, this should go away, but + to avoid regressing targets this field encodes that number so the + profitability analysis can remain unchanged. */ + unsigned int magic_number; + }; static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int); @@ -836,6 +844,19 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); static int noce_try_minmax (struct noce_if_info *); static int noce_try_abs (struct noce_if_info *); static int noce_try_sign_mask (struct noce_if_info *); +static bool noce_is_profitable_p (rtx_insn *, struct noce_if_info *); + +/* Given SEQ, which is a sequence we might want to generate after + if-conversion, and a basic-block structure in IF_INFO which represents + the code generation before if-conversion, return TRUE if this would + be a profitable transformation. */ + +static bool +noce_is_profitable_p (rtx_insn *seq ATTRIBUTE_UNUSED, + struct noce_if_info *if_info) +{ + return (if_info->branch_cost >= if_info->magic_number); +} /* Helper function for noce_try_store_flag*. */ @@ -1192,8 +1213,13 @@ noce_try_store_flag_constants (struct noce_if_info *if_info) HOST_WIDE_INT itrue, ifalse, diff, tmp; int normalize; bool can_reverse; + bool no_cost_model = false; machine_mode mode = GET_MODE (if_info->x);; rtx common = NULL_RTX; + /* ??? There are paths through this function from which no cost function + is checked before conversion. Maintain that behaviour by setting + the magic number used by noce_is_profitable_p to zero. */ + if_info->magic_number = 0; rtx a = if_info->a; rtx b = if_info->b; @@ -1204,9 +1230,9 @@ noce_try_store_flag_constants (struct noce_if_info *if_info) && CONST_INT_P (XEXP (a, 1)) && CONST_INT_P (XEXP (b, 1)) && rtx_equal_p (XEXP (a, 0), XEXP (b, 0)) - && noce_operand_ok (XEXP (a, 0)) - && if_info->branch_cost >= 2) + && noce_operand_ok (XEXP (a, 0))) { + if_info->magic_number = 2; common = XEXP (a, 0); a = XEXP (a, 1); b = XEXP (b, 1); @@ -1278,23 +1304,33 @@ noce_try_store_flag_constants (struct noce_if_info *if_info) else gcc_unreachable (); } - else if (ifalse == 0 && exact_log2 (itrue) >= 0 - && (STORE_FLAG_VALUE == 1 - || if_info->branch_cost >= 2)) - normalize = 1; - else if (itrue == 0 && exact_log2 (ifalse) >= 0 && can_reverse - && (STORE_FLAG_VALUE == 1 || if_info->branch_cost >= 2)) + else if (ifalse == 0 && exact_log2 (itrue) >= 0) { + if_info->magic_number = 2; + if (STORE_FLAG_VALUE == 1) + no_cost_model = true; + normalize = 1; + } + else if (itrue == 0 && exact_log2 (ifalse) >= 0 && can_reverse) + { + if_info->magic_number = 2; + if (STORE_FLAG_VALUE == 1) + no_cost_model = true; normalize = 1; reversep = true; } - else if (itrue == -1 - && (STORE_FLAG_VALUE == -1 - || if_info->branch_cost >= 2)) - normalize = -1; - else if (ifalse == -1 && can_reverse - && (STORE_FLAG_VALUE == -1 || if_info->branch_cost >= 2)) + else if (itrue == -1) { + if_info->magic_number = 2; + if (STORE_FLAG_VALUE == -1) + no_cost_model = true; + normalize = -1; + } + else if (ifalse == -1 && can_reverse) + { + if_info->magic_number = 2; + if (STORE_FLAG_VALUE == -1) + no_cost_model = true; normalize = -1; reversep = true; } @@ -1385,6 +1421,10 @@ noce_try_store_flag_constants (struct noce_if_info *if_info) if (!seq) return FALSE; + /* Check if this is actually beneficial. */ + if (!no_cost_model && !noce_is_profitable_p (seq, if_info)) + return FALSE; + emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); return TRUE; @@ -1446,8 +1486,7 @@ noce_try_addcc (struct noce_if_info *if_info) /* If that fails, construct conditional increment or decrement using setcc. */ - if (if_info->branch_cost >= 2 - && (XEXP (if_info->a, 1) == const1_rtx + if ((XEXP (if_info->a, 1) == const1_rtx || XEXP (if_info->a, 1) == constm1_rtx)) { start_sequence (); @@ -1477,6 +1516,11 @@ noce_try_addcc (struct noce_if_info *if_info) if (!seq) return FALSE; + /* Check if this is actually beneficial. */ + if_info->magic_number = 2; + if (!noce_is_profitable_p (seq, if_info)) + return FALSE; + emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); return TRUE; @@ -1501,15 +1545,14 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) return FALSE; reversep = 0; - if ((if_info->branch_cost >= 2 - || STORE_FLAG_VALUE == -1) - && ((if_info->a == const0_rtx + + if ((if_info->a == const0_rtx && rtx_equal_p (if_info->b, if_info->x)) || ((reversep = (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN)) && if_info->b == const0_rtx - && rtx_equal_p (if_info->a, if_info->x)))) + && rtx_equal_p (if_info->a, if_info->x))) { start_sequence (); target = noce_emit_store_flag (if_info, @@ -1523,7 +1566,7 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) if (target) { - int old_cost, new_cost, insn_cost; + int new_cost, old_cost; int speed_p; if (target != if_info->x) @@ -1533,12 +1576,26 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) if (!seq) return FALSE; + /* The previous costing code here calculated everything in the + rtx_cost base, and compared it against + COSTS_N_INSNS (if_info->branch_cost). As we don't want to + multiply branch cost, instead divide through by + COSTS_N_INSNS (1) to get our magic_number. We also need to + take account of a possible magic_number of 2 which should + be applied if STORE_FLAG_VALUE != -1. */ speed_p = optimize_bb_for_speed_p (BLOCK_FOR_INSN (if_info->insn_a)); - insn_cost = insn_rtx_cost (PATTERN (if_info->insn_a), speed_p); - old_cost = COSTS_N_INSNS (if_info->branch_cost) + insn_cost; + old_cost = insn_rtx_cost (PATTERN (if_info->insn_a), speed_p); new_cost = seq_cost (seq, speed_p); + if_info->magic_number = (new_cost - old_cost); + if_info->magic_number + = (if_info->magic_number / COSTS_N_INSNS (1)) + + ((if_info->magic_number % COSTS_N_INSNS (1)) ? 1 : 0); - if (new_cost > old_cost) + if_info->magic_number + = MAX (((STORE_FLAG_VALUE != -1) ? 2 : 0), + (if_info->magic_number)); + + if (!noce_is_profitable_p (seq, if_info)) return FALSE; emit_insn_before_setloc (seq, if_info->jump, @@ -1703,9 +1760,7 @@ noce_try_cmove (struct noce_if_info *if_info) we don't know about, so give them a chance before trying this approach. */ else if (!targetm.have_conditional_execution () - && CONST_INT_P (if_info->a) && CONST_INT_P (if_info->b) - && ((if_info->branch_cost >= 2 && STORE_FLAG_VALUE == -1) - || if_info->branch_cost >= 3)) + && CONST_INT_P (if_info->a) && CONST_INT_P (if_info->b)) { machine_mode mode = GET_MODE (if_info->x); HOST_WIDE_INT ifalse = INTVAL (if_info->a); @@ -1744,6 +1799,12 @@ noce_try_cmove (struct noce_if_info *if_info) if (!seq) return FALSE; + /* Check if this is actually beneficial. */ + if_info->magic_number = STORE_FLAG_VALUE == -1 + ? 2 : 3; + if (!noce_is_profitable_p (seq, if_info)) + return FALSE; + emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); return TRUE; @@ -1930,7 +1991,9 @@ noce_try_cmove_arith (struct noce_if_info *if_info) conditional on their addresses followed by a load. Don't do this early because it'll screw alias analysis. Note that we've already checked for no side effects. */ - /* ??? FIXME: Magic number 5. */ + /* ??? FIXME: Magic number 5. We cost this here rather than through + noce_is_profitable_p as the fallback cases below can produce viable + transformations in the case where (if_info->branch_cost < 5). */ if (cse_not_expected && MEM_P (a) && MEM_P (b) && MEM_ADDR_SPACE (a) == MEM_ADDR_SPACE (b) @@ -1973,10 +2036,12 @@ noce_try_cmove_arith (struct noce_if_info *if_info) else else_cost = 0; - /* We're going to execute one of the basic blocks anyway, so - bail out if the most expensive of the two blocks is unacceptable. */ - if (MAX (then_cost, else_cost) > COSTS_N_INSNS (if_info->branch_cost)) - return FALSE; + /* We want the most expensive of the above, divided through by + COSTS_N_INSNS (1). */ + if_info->magic_number = MAX (then_cost, else_cost); + if_info->magic_number + = (if_info->magic_number / COSTS_N_INSNS (1)) + + ((if_info->magic_number % COSTS_N_INSNS (1)) ? 1 : 0); /* Possibly rearrange operands to make things come out more natural. */ if (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN) @@ -2115,6 +2180,9 @@ noce_try_cmove_arith (struct noce_if_info *if_info) if (!ifcvt_seq) return FALSE; + if (!noce_is_profitable_p (ifcvt_seq, if_info)) + return FALSE; + emit_insn_before_setloc (ifcvt_seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); return TRUE; @@ -2571,7 +2639,12 @@ noce_try_sign_mask (struct noce_if_info *if_info) non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no INSN_B which can happen for e.g. conditional stores to memory. For the cost computation use the block TEST_BB where the evaluation will end up - after the transformation. */ + after the transformation. + ??? The underlying calculation is a natural fit for the long term + direction of noce_is_profitable_p, but there is no way to transform + this cost calculation in to a comparison against branch_cost. When + noce_is_profitable_p becomes a proper cost calulcation, this logic + should be cleaned up. */ t_unconditional = (t == if_info->b && (if_info->insn_b == NULL_RTX