From patchwork Fri Oct 12 20:01:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bill Schmidt X-Patchwork-Id: 983287 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-487488-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="gTW0FBx3"; dkim-atps=neutral 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 42WzHZ58n3z9s3Z for ; Sat, 13 Oct 2018 07:01:27 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to:cc :from:subject:date:mime-version:content-type :content-transfer-encoding:message-id; q=dns; s=default; b=h55Ff xVzrnkJxUcBWiCA1Yjt034Xn/Qw7L+v49Rl9J1G/bNtBIDdgjo85Xyuw3w4eFFtR 8LGMpasLMeniXQlM/fMnYKBXwsPMFCCKtB66AVK8ucRfEyDeKZzbR+mVCxwSpR6L +2J0vYFL3l3Teyo3RNEL9YrHp3JJN7ZCUU9gzQ= 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:to:cc :from:subject:date:mime-version:content-type :content-transfer-encoding:message-id; s=default; bh=rKragvi4B0z PKYf/gb6/rTbQYXI=; b=gTW0FBx3ZpXjXaYW3LrQvjeI13cwvG+97Oc0KRFqU0W OWGxyP7eQMJ2N6kVy6lkXy5z7bri+zhy4ehtqH7c/8zFcQbiOdZ7PSb12f4/+pY2 O2tI+cfZJpqCF/ENx++rqAxsldpBQIAH0XGV/dMDunBMziqosYIKGejoQdAWyl78 = Received: (qmail 5498 invoked by alias); 12 Oct 2018 20:01:20 -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 5484 invoked by uid 89); 12 Oct 2018 20:01:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, KHOP_DYNAMIC, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=greatly, Schmidt, schmidt, consistently X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Oct 2018 20:01:16 +0000 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w9CJxewc051292 for ; Fri, 12 Oct 2018 16:01:14 -0400 Received: from e16.ny.us.ibm.com (e16.ny.us.ibm.com [129.33.205.206]) by mx0a-001b2d01.pphosted.com with ESMTP id 2n2y04fyq8-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 12 Oct 2018 16:01:13 -0400 Received: from localhost by e16.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 12 Oct 2018 16:01:06 -0400 Received: from b01cxnp23033.gho.pok.ibm.com (9.57.198.28) by e16.ny.us.ibm.com (146.89.104.203) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Fri, 12 Oct 2018 16:01:04 -0400 Received: from b01ledav001.gho.pok.ibm.com (b01ledav001.gho.pok.ibm.com [9.57.199.106]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w9CK131q22937822 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 12 Oct 2018 20:01:03 GMT Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7C0B12805C; Fri, 12 Oct 2018 15:58:29 -0400 (EDT) Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 424A528060; Fri, 12 Oct 2018 15:58:29 -0400 (EDT) Received: from bigmac.rchland.ibm.com (unknown [9.10.86.17]) by b01ledav001.gho.pok.ibm.com (Postfix) with ESMTP; Fri, 12 Oct 2018 15:58:29 -0400 (EDT) To: GCC Patches Cc: Richard Biener From: Bill Schmidt Subject: [PATCH] Fix PR87473 (SLSR ICE on hidden basis) Date: Fri, 12 Oct 2018 15:01:02 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 x-cbid: 18101220-0072-0000-0000-000003B4F523 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009867; HX=3.00000242; KW=3.00000007; PH=3.00000004; SC=3.00000268; SDB=6.01101722; UDB=6.00570123; IPR=6.00881767; MB=3.00023732; MTD=3.00000008; XFM=3.00000015; UTC=2018-10-12 20:01:04 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18101220-0073-0000-0000-000049BC6C2D Message-Id: Hi, This patch addresses SLSR bug PR87473. The underlying problem here is that create_add_on_incoming_edge contains code to handle a phi_arg that is equal to the base expression of the PHI candidate, where the increment assigned to the incoming arc should be zero minus the index expression of the hidden basis; but several other places in SLSR processing need to handle the same case, and fail to do so. As a result, the code to replace the PHI basis attempts to use an initializing statement that was never created in the first place, and we ICE. This patch adds the necessary logic in four parts of the code to ensure we handle this consistently throughout. This error survived this long because the typical case when accessing the hidden basis is for the index of the hidden basis to be zero. For such a case we don't need an initializing statement, and the ICE doesn't trigger. The test case provided with the PR is a counter-example where the hidden basis has an index of 2. For the four functions fixed here, each identified the case of interest, but just didn't do anything when that case arose. I've reorganized the code in each case to always execute the relevant logic, but change what's done for the specific situation of the "pass-through" PHI argument. This makes the diffs a little hard to read, unfortunately. During the investigation I noticed that we are dealing with a degenerate PHI, introduced by the loopinit pass, that we would be better off optimizing away sooner: [local count: 14598063]: # qz_1 = PHI # jl_22 = PHI _8 = (unsigned int) jl_22; _13 = _8 * _15; qz_11 = (int) _13; The assignment to _8 should just use jl_6 in place of jl_22. This would greatly simplify SLSR's job, since PHI-free code is handled much more straightforwardly than code that involves conditional updates. We go through at least 30 passes without having this cleaned up, and I expect other passes than SLSR would perhaps be hamstrung by this as well. Any recommendations? Bootstrapped and tested on powerpc64le-linux-gnu with no regressions. I've added the test case from the bugzilla to the torture tests. Is this okay for trunk, and after a suitable period, to GCC 7 and 8 also? Thanks! Bill [gcc] 2018-10-12 Bill Schmidt PR tree-optimization/87473 * gimple-ssa-strength-reduction.c (record_phi_increments_1): For phi arguments identical to the base expression of the phi candidate, record a phi-adjust increment of zero minus the index expression of the hidden basis. (phi_incr_cost_1): For phi arguments identical to the base expression of the phi candidate, the difference to compare against the increment is zero minus the index expression of the hidden basis, and there is no potential savings from replacing the (phi) statement. (ncd_with_phi): For phi arguments identical to the base expression of the phi candidate, the difference to compare against the increment is zero minus the index expression of the hidden basis. (all_phi_incrs_profitable_1): For phi arguments identical to the base expression of the phi candidate, the increment to be checked for profitability is zero minus the index expression of the hidden basis. [gcc/testsuite] 2018-10-12 Bill Schmidt PR tree-optimization/87473 * gcc.c-torture/compile/pr87473.c: New file. Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 265112) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2779,17 +2779,23 @@ record_phi_increments_1 (slsr_cand_t basis, gimple for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); + gimple *arg_def = SSA_NAME_DEF_STMT (arg); - if (!operand_equal_p (arg, phi_cand->base_expr, 0)) + if (gimple_code (arg_def) == GIMPLE_PHI) + record_phi_increments_1 (basis, arg_def); + else { - gimple *arg_def = SSA_NAME_DEF_STMT (arg); + widest_int diff; - if (gimple_code (arg_def) == GIMPLE_PHI) - record_phi_increments_1 (basis, arg_def); + if (operand_equal_p (arg, phi_cand->base_expr, 0)) + { + diff = -basis->index; + record_increment (phi_cand, diff, PHI_ADJUST); + } else { slsr_cand_t arg_cand = base_cand_from_table (arg); - widest_int diff = arg_cand->index - basis->index; + diff = arg_cand->index - basis->index; record_increment (arg_cand, diff, PHI_ADJUST); } } @@ -2864,29 +2870,43 @@ phi_incr_cost_1 (slsr_cand_t c, const widest_int & for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); + gimple *arg_def = SSA_NAME_DEF_STMT (arg); - if (!operand_equal_p (arg, phi_cand->base_expr, 0)) + if (gimple_code (arg_def) == GIMPLE_PHI) { - gimple *arg_def = SSA_NAME_DEF_STMT (arg); - - if (gimple_code (arg_def) == GIMPLE_PHI) + int feeding_savings = 0; + tree feeding_var = gimple_phi_result (arg_def); + cost += phi_incr_cost_1 (c, incr, arg_def, &feeding_savings); + if (uses_consumed_by_stmt (feeding_var, phi)) + *savings += feeding_savings; + } + else + { + widest_int diff; + slsr_cand_t arg_cand; + + /* When the PHI argument is just a pass-through to the base + expression of the hidden basis, the difference is zero minus + the index of the basis. There is no potential savings by + eliminating a statement in this case. */ + if (operand_equal_p (arg, phi_cand->base_expr, 0)) { - int feeding_savings = 0; - tree feeding_var = gimple_phi_result (arg_def); - cost += phi_incr_cost_1 (c, incr, arg_def, &feeding_savings); - if (uses_consumed_by_stmt (feeding_var, phi)) - *savings += feeding_savings; + arg_cand = (slsr_cand_t)NULL; + diff = -basis->index; } else { - slsr_cand_t arg_cand = base_cand_from_table (arg); - widest_int diff = arg_cand->index - basis->index; - - if (incr == diff) + arg_cand = base_cand_from_table (arg); + diff = arg_cand->index - basis->index; + } + + if (incr == diff) + { + tree basis_lhs = gimple_assign_lhs (basis->cand_stmt); + cost += add_cost (true, TYPE_MODE (TREE_TYPE (basis_lhs))); + if (arg_cand) { - tree basis_lhs = gimple_assign_lhs (basis->cand_stmt); tree lhs = gimple_assign_lhs (arg_cand->cand_stmt); - cost += add_cost (true, TYPE_MODE (TREE_TYPE (basis_lhs))); if (uses_consumed_by_stmt (lhs, phi)) *savings += stmt_cost (arg_cand->cand_stmt, true); } @@ -3228,23 +3248,26 @@ ncd_with_phi (slsr_cand_t c, const widest_int &inc for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); + gimple *arg_def = SSA_NAME_DEF_STMT (arg); - if (!operand_equal_p (arg, phi_cand->base_expr, 0)) + if (gimple_code (arg_def) == GIMPLE_PHI) + ncd = ncd_with_phi (c, incr, as_a (arg_def), ncd, where); + else { - gimple *arg_def = SSA_NAME_DEF_STMT (arg); + widest_int diff; - if (gimple_code (arg_def) == GIMPLE_PHI) - ncd = ncd_with_phi (c, incr, as_a (arg_def), ncd, - where); - else + if (operand_equal_p (arg, phi_cand->base_expr, 0)) + diff = -basis->index; + else { slsr_cand_t arg_cand = base_cand_from_table (arg); - widest_int diff = arg_cand->index - basis->index; - basic_block pred = gimple_phi_arg_edge (phi, i)->src; - - if ((incr == diff) || (!address_arithmetic_p && incr == -diff)) - ncd = ncd_for_two_cands (ncd, pred, *where, NULL, where); + diff = arg_cand->index - basis->index; } + + basic_block pred = gimple_phi_arg_edge (phi, i)->src; + + if ((incr == diff) || (!address_arithmetic_p && incr == -diff)) + ncd = ncd_for_two_cands (ncd, pred, *where, NULL, where); } } @@ -3515,51 +3538,53 @@ all_phi_incrs_profitable_1 (slsr_cand_t c, gphi *p return false; tree arg = gimple_phi_arg_def (phi, i); + gimple *arg_def = SSA_NAME_DEF_STMT (arg); - if (!operand_equal_p (arg, phi_cand->base_expr, 0)) + if (gimple_code (arg_def) == GIMPLE_PHI) { - gimple *arg_def = SSA_NAME_DEF_STMT (arg); + if (!all_phi_incrs_profitable_1 (c, as_a (arg_def), spread) + || *spread > MAX_SPREAD) + return false; + } + else + { + int j; + widest_int increment; - if (gimple_code (arg_def) == GIMPLE_PHI) - { - if (!all_phi_incrs_profitable_1 (c, as_a (arg_def), - spread) - || *spread > MAX_SPREAD) - return false; - } + if (operand_equal_p (arg, phi_cand->base_expr, 0)) + increment = -basis->index; else { - int j; slsr_cand_t arg_cand = base_cand_from_table (arg); - widest_int increment = arg_cand->index - basis->index; + increment = arg_cand->index - basis->index; + } - if (!address_arithmetic_p && wi::neg_p (increment)) - increment = -increment; + if (!address_arithmetic_p && wi::neg_p (increment)) + increment = -increment; - j = incr_vec_index (increment); + j = incr_vec_index (increment); - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fprintf (dump_file, " Conditional candidate %d, phi: ", - c->cand_num); - print_gimple_stmt (dump_file, phi, 0); - fputs (" increment: ", dump_file); - print_decs (increment, dump_file); - if (j < 0) - fprintf (dump_file, - "\n Not replaced; incr_vec overflow.\n"); - else { - fprintf (dump_file, "\n cost: %d\n", incr_vec[j].cost); - if (profitable_increment_p (j)) - fputs (" Replacing...\n", dump_file); - else - fputs (" Not replaced.\n", dump_file); - } - } + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, " Conditional candidate %d, phi: ", + c->cand_num); + print_gimple_stmt (dump_file, phi, 0); + fputs (" increment: ", dump_file); + print_decs (increment, dump_file); + if (j < 0) + fprintf (dump_file, + "\n Not replaced; incr_vec overflow.\n"); + else { + fprintf (dump_file, "\n cost: %d\n", incr_vec[j].cost); + if (profitable_increment_p (j)) + fputs (" Replacing...\n", dump_file); + else + fputs (" Not replaced.\n", dump_file); + } + } - if (j < 0 || !profitable_increment_p (j)) - return false; - } + if (j < 0 || !profitable_increment_p (j)) + return false; } } Index: gcc/testsuite/gcc.c-torture/compile/pr87473.c =================================================================== --- gcc/testsuite/gcc.c-torture/compile/pr87473.c (nonexistent) +++ gcc/testsuite/gcc.c-torture/compile/pr87473.c (working copy) @@ -0,0 +1,19 @@ +/* PR87473: SLSR ICE on hidden basis with |increment| > 1. */ +/* { dg-additional-options "-fno-tree-ch" } */ + +void +t6 (int qz, int wh) +{ + int jl = wh; + + while (1.0 / 0 < 1) + { + qz = wh * (wh + 2); + + while (wh < 1) + jl = 0; + } + + while (qz < 1) + qz = jl * wh; +}