From patchwork Sat Aug 17 02:13:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Pinski X-Patchwork-Id: 1973413 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=quicinc.com header.i=@quicinc.com header.a=rsa-sha256 header.s=qcppdkim1 header.b=ooIfOFt/; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Wm2Tr3x2Zz1yXf for ; Sat, 17 Aug 2024 12:13:50 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DAA333860743 for ; Sat, 17 Aug 2024 02:13:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by sourceware.org (Postfix) with ESMTPS id 047CE3858C66 for ; Sat, 17 Aug 2024 02:13:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 047CE3858C66 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=quicinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=quicinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 047CE3858C66 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=205.220.180.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723860796; cv=none; b=GeaNRvmQSIoZ2VbAhGYe6ieJnw4hLmD3Gk0Cv0dHrhr830mBYTCS7fpE+rmKQZIYT8+ZG6MKnbgnVRnct95C01yPrJHtZ1/AG+x+jpChoLb+xoYvKVTtRzop2TyA9r13+KlE8owYGck34P7Ht60AB7QgVjSDSOeoyOAj8au4WT0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723860796; c=relaxed/simple; bh=7/AxoLxuJ2tfanNQlENRl7y00BipDa0Y0lC94Ca71ks=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=RA+/kw4ljiO9+IglsvdWxWD67qzLlvy5NqCTn+AcPISIMaGMGPnQUzs9pWvuF3WQDEdUhNofMp5CwwbIeWZb1ytLEhFmVNvxtSc5ipIN0UPIYAQRj+FDcxVWtHpWMWe1ncYtB2t0iILMVd7hMP2BpMFH+FcAO1M4LD5vwwDgvmI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0279872.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 47H222XB026486 for ; Sat, 17 Aug 2024 02:13:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= cc:content-transfer-encoding:content-type:date:from:message-id :mime-version:subject:to; s=qcppdkim1; bh=7nCQZJzwU5g72qMq5xD1Ay y3qMjPGs3r8kNa0pOYWtI=; b=ooIfOFt/CQ6xHJ1eRlel60TqgBdiWHe+CdR9qg hOGUKgWbAL9th1MZAqz1Dukq27lBSC2F4Z+pfVur2oIuDcqb1kcRXhvCMDiY5AnT rmphZ50QxGB1C16/SzBejn+yyaF03tjaCwyDkOna9U7OAwY2V9j48yAv2m3aA6iB 5jpR7nVogwXWMqkpeEBXBuWO5BHPKBa/8dzj9wlUtm7Pg3QhemGNuXByEu2Hdndy /j0LaGXKIdDpH3ndgdz3YUsIWnl6DieKJR9wBEXYgums022qJHgV+koaEBjG2kxw 9NE8MaiUvN0VMi/v3Tf//ZDd+nb0B3w/08EpUR5IYJHus1XA== Received: from nasanppmta02.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 411957wpx6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 17 Aug 2024 02:13:13 +0000 (GMT) Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA02.qualcomm.com (8.18.1.2/8.18.1.2) with ESMTPS id 47H2DCAP013898 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 17 Aug 2024 02:13:12 GMT Received: from hu-apinski-lv.qualcomm.com (10.49.16.6) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Fri, 16 Aug 2024 19:13:11 -0700 From: Andrew Pinski To: CC: Andrew Pinski Subject: [PATCH] PHIOPT: move factor_out_conditional_operation over to use gimple_match_op Date: Fri, 16 Aug 2024 19:13:00 -0700 Message-ID: <20240817021300.1590935-1-quic_apinski@quicinc.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Originating-IP: [10.49.16.6] X-ClientProxiedBy: nalasex01c.na.qualcomm.com (10.47.97.35) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: PUUx4hcQcl5Bc4OmBqNiSMi-ZH4Ln05L X-Proofpoint-ORIG-GUID: PUUx4hcQcl5Bc4OmBqNiSMi-ZH4Ln05L X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16 definitions=2024-08-16_18,2024-08-16_01,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 phishscore=0 lowpriorityscore=0 mlxlogscore=999 clxscore=1015 suspectscore=0 impostorscore=0 bulkscore=0 malwarescore=0 spamscore=0 mlxscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2407110000 definitions=main-2408170013 X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org To start working on more with expressions with more than one operand, converting over to use gimple_match_op is needed. The added side-effect here is factor_out_conditional_operation can now support builtins/internal calls that has one operand without any extra code added. Note on the changed testcases: * pr87007-5.c: the test was testing testing for avoiding partial register stalls for the sqrt and making sure there is only one zero of the register before the branch, the phiopt would now merge the sqrt's so disable phiopt. Bootstrapped and tested on x86_64-linux-gnu with no regressions. gcc/ChangeLog: * gimple-match-exports.cc (gimple_match_op::operands_occurs_in_abnormal_phi): New function. * gimple-match.h (gimple_match_op): Add operands_occurs_in_abnormal_phi. * tree-ssa-phiopt.cc (factor_out_conditional_operation): Use gimple_match_op instead of manually extracting from/creating the gimple. gcc/testsuite/ChangeLog: * gcc.target/i386/pr87007-5.c: Disable phi-opt. Signed-off-by: Andrew Pinski --- gcc/gimple-match-exports.cc | 14 +++++ gcc/gimple-match.h | 2 + gcc/testsuite/gcc.target/i386/pr87007-5.c | 5 +- gcc/tree-ssa-phiopt.cc | 66 ++++++++++------------- 4 files changed, 49 insertions(+), 38 deletions(-) diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc index aacf3ff0414..15d54b7d843 100644 --- a/gcc/gimple-match-exports.cc +++ b/gcc/gimple-match-exports.cc @@ -126,6 +126,20 @@ gimple_match_op::resimplify (gimple_seq *seq, tree (*valueize)(tree)) } } +/* Returns true if any of the operands of THIS occurs + in abnormal phis. */ +bool +gimple_match_op::operands_occurs_in_abnormal_phi() const +{ + for (unsigned int i = 0; i < num_ops; i++) + { + if (TREE_CODE (ops[i]) == SSA_NAME + && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i])) + return true; + } + return false; +} + /* Return whether T is a constant that we'll dispatch to fold to evaluate fully constant expressions. */ diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h index d710fcbace2..8edff578ba9 100644 --- a/gcc/gimple-match.h +++ b/gcc/gimple-match.h @@ -136,6 +136,8 @@ public: /* The operands to CODE. Only the first NUM_OPS entries are meaningful. */ tree ops[MAX_NUM_OPS]; + + bool operands_occurs_in_abnormal_phi() const; }; inline diff --git a/gcc/testsuite/gcc.target/i386/pr87007-5.c b/gcc/testsuite/gcc.target/i386/pr87007-5.c index 8f2dc947f6c..1a240adef63 100644 --- a/gcc/testsuite/gcc.target/i386/pr87007-5.c +++ b/gcc/testsuite/gcc.target/i386/pr87007-5.c @@ -1,8 +1,11 @@ /* { dg-do compile } */ -/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized" } */ +/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized -fno-ssa-phiopt" } */ /* Load of d2/d3 is hoisted out, the loop is split, store of d1 and sqrt are sunk out of the loop and the loop is elided. One vsqrtsd with memory operand needs a xor to avoid partial dependence. */ +/* Phi-OPT needs to ne disabled otherwise, sqrt calls are merged which is better + but we are testing to make sure the partial register stall for SSE is still avoided + for sqrts. */ #include diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index aacccc414f6..2d4aba5b087 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -220,13 +220,12 @@ static gphi * factor_out_conditional_operation (edge e0, edge e1, gphi *phi, tree arg0, tree arg1, gimple *cond_stmt) { - gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL, *new_stmt; - tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; + gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL; tree temp, result; gphi *newphi; gimple_stmt_iterator gsi, gsi_for_def; location_t locus = gimple_location (phi); - enum tree_code op_code; + gimple_match_op arg0_op, arg1_op; /* Handle only PHI statements with two arguments. TODO: If all other arguments to PHI are INTEGER_CST or if their defining @@ -250,31 +249,31 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, /* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is an unary operation. */ arg0_def_stmt = SSA_NAME_DEF_STMT (arg0); - if (!is_gimple_assign (arg0_def_stmt) - || (gimple_assign_rhs_class (arg0_def_stmt) != GIMPLE_UNARY_RHS - && gimple_assign_rhs_code (arg0_def_stmt) != VIEW_CONVERT_EXPR)) + if (!gimple_extract_op (arg0_def_stmt, &arg0_op)) return NULL; - /* Use the RHS as new_arg0. */ - op_code = gimple_assign_rhs_code (arg0_def_stmt); - new_arg0 = gimple_assign_rhs1 (arg0_def_stmt); - if (op_code == VIEW_CONVERT_EXPR) - { - new_arg0 = TREE_OPERAND (new_arg0, 0); - if (!is_gimple_reg_type (TREE_TYPE (new_arg0))) - return NULL; - } - if (TREE_CODE (new_arg0) == SSA_NAME - && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg0)) + /* Check to make sure none of the operands are in abnormal phis. */ + if (arg0_op.operands_occurs_in_abnormal_phi ()) + return NULL; + + /* Currently just support one operand expressions. */ + if (arg0_op.num_ops != 1) return NULL; + tree new_arg0 = arg0_op.ops[0]; + tree new_arg1; + if (TREE_CODE (arg1) == SSA_NAME) { - /* Check if arg1 is an SSA_NAME and the stmt which defines arg1 - is an unary operation. */ + /* Check if arg1 is an SSA_NAME. */ arg1_def_stmt = SSA_NAME_DEF_STMT (arg1); - if (!is_gimple_assign (arg1_def_stmt) - || gimple_assign_rhs_code (arg1_def_stmt) != op_code) + if (!gimple_extract_op (arg1_def_stmt, &arg1_op)) + return NULL; + if (arg1_op.code != arg0_op.code) + return NULL; + if (arg1_op.num_ops != arg0_op.num_ops) + return NULL; + if (arg1_op.operands_occurs_in_abnormal_phi ()) return NULL; /* Either arg1_def_stmt or arg0_def_stmt should be conditional. */ @@ -282,14 +281,7 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, && dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb (arg1_def_stmt))) return NULL; - - /* Use the RHS as new_arg1. */ - new_arg1 = gimple_assign_rhs1 (arg1_def_stmt); - if (op_code == VIEW_CONVERT_EXPR) - new_arg1 = TREE_OPERAND (new_arg1, 0); - if (TREE_CODE (new_arg1) == SSA_NAME - && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg1)) - return NULL; + new_arg1 = arg1_op.ops[0]; } else { @@ -300,6 +292,7 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, /* arg0_def_stmt should be conditional. */ if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb (arg0_def_stmt))) return NULL; + /* If arg1 is an INTEGER_CST, fold it to new type. */ if (INTEGRAL_TYPE_P (TREE_TYPE (new_arg0)) && (int_fits_type_p (arg1, TREE_TYPE (new_arg0)) @@ -405,16 +398,15 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, add_phi_arg (newphi, new_arg0, e0, locus); add_phi_arg (newphi, new_arg1, e1, locus); + gimple_match_op new_op = arg0_op; + /* Create the operation stmt and insert it. */ - if (op_code == VIEW_CONVERT_EXPR) - { - temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp); - new_stmt = gimple_build_assign (result, temp); - } - else - new_stmt = gimple_build_assign (result, op_code, temp); + new_op.ops[0] = temp; + gimple_seq seq = NULL; + result = maybe_push_res_to_seq (&new_op, &seq, result); + gcc_assert (result); gsi = gsi_after_labels (gimple_bb (phi)); - gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); + gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING); /* Remove the original PHI stmt. */ gsi = gsi_for_stmt (phi);