From patchwork Thu Jun 20 06:32:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: HAO CHEN GUI X-Patchwork-Id: 92481 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5517F388883F for ; Thu, 20 Jun 2024 06:33:13 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 69FDB3858C41 for ; Thu, 20 Jun 2024 06:32:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 69FDB3858C41 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 69FDB3858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718865164; cv=none; b=rqTtXPH8+KZgL9fyJ3Ye54oZkDsYPm9u4EuLnu3x8vUEBuaxqWoIj/nxq9v8P0csWOcqG8WI2u1erFG+F0hGqzxpyu2yp+a5EENBk4bJUYmmBcyMd8oq3MTF+SGX9/xeZcx6eBxjONkbV9ONnUuEnDia3/edlh859yFkcZymcBA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718865164; c=relaxed/simple; bh=eF5mXhSI5+MqKBFkk5ywDpzBNBiVh4tLqjmA3z5GGfw=; h=DKIM-Signature:Message-ID:Date:To:From:Subject:MIME-Version; b=JjPnDlosMEgmI48sS/6HRhpuvvVf1qlecyfhZSBIsJzuTCSY2jpxDLrzfyjKbPdDB8yDE/0D8o1550A9Uwkd4WITghbe+S6+tQ2SAHODs3SxUQhnd/rjCegmWlLBBASAbSLPMXeKtKNSjvulg30H7G5i4Zq7VLEwulCdZXUV2f8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353724.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 45K6TT7j010781; Thu, 20 Jun 2024 06:32:39 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h= message-id:date:to:cc:from:subject:content-type :content-transfer-encoding:mime-version; s=pp1; bh=Fh6pa7bIUmEh1 mhlWmQqNIvt46pGmA1daNDKvjU5HCo=; b=XP4eG/L54zV8ie0ktIgAbdg4q7NsJ mvRMOMFNXhdLOfeKBU+gjkqYTdhMDeHiNfMf8lCO7DrtP8GjrroPB/UYGJmseG6p +NSg/lnG85uZyqLJZAwbSyAVbaIybL3b5ARXkfPstcYeaWco7J96orqKRn/4v4ic H217DgTl5R+Ql+PyblyrEFuxUqAT7j7JDlVPNuaLTnM+F1Yke7PrKPg0osS7+50c n5SA/gMWf/3GwkwIFm4jesCylpcqwzq2BP26sVNOcVVBJ34r48XfEbF5kU21WOYR t7Nf9oxgac0HFXeIVcafiI+qvAZ8eYV971/elCJzOUwIozCrpeZtnu0QA== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yvf6pr07t-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jun 2024 06:32:39 +0000 (GMT) Received: from m0353724.ppops.net (m0353724.ppops.net [127.0.0.1]) by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 45K6Wdsh014902; Thu, 20 Jun 2024 06:32:39 GMT Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yvf6pr07r-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jun 2024 06:32:39 +0000 (GMT) Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1]) by ppma13.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 45K4fwWg009422; Thu, 20 Jun 2024 06:32:38 GMT Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227]) by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 3ysqgn2x21-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jun 2024 06:32:38 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 45K6WW6653412338 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 20 Jun 2024 06:32:34 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3809120049; Thu, 20 Jun 2024 06:32:32 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4923A2004B; Thu, 20 Jun 2024 06:32:30 +0000 (GMT) Received: from [9.200.103.244] (unknown [9.200.103.244]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Thu, 20 Jun 2024 06:32:30 +0000 (GMT) Message-ID: <3f693aef-81df-46c5-bde2-83e4d998e65c@linux.ibm.com> Date: Thu, 20 Jun 2024 14:32:28 +0800 User-Agent: Mozilla Thunderbird Content-Language: en-US To: gcc-patches Cc: Segher Boessenkool , David , "Kewen.Lin" , Peter Bergner , Richard Sandiford , Jeff Law From: HAO CHEN GUI Subject: [PATCH-1v5] fwprop: Replace rtx_cost with insn_cost in try_fwprop_subst_pattern [PR113325] X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: BBD1Ce-Ausot12EKrwZnFF28fu0Zcb9z X-Proofpoint-GUID: JV41m4EUqLZtIwXqzSADeBzJVS6HjHoB X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 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-06-20_03,2024-06-19_01,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 mlxscore=0 lowpriorityscore=0 adultscore=0 phishscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999 clxscore=1015 suspectscore=0 priorityscore=1501 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2405170001 definitions=main-2406200045 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, 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+patchwork=sourceware.org@gcc.gnu.org Hi, This patch replaces rtx_cost with insn_cost in forward propagation. In the PR, one constant vector should be propagated and replace a pseudo in a store insn if we know it's a duplicated constant vector. It reduces the insn cost but not rtx cost. In this case, the cost is determined by destination operand (memory or pseudo). Unfortunately, rtx cost can't help. The test case is added in the second rs6000 specific patch. Compared to previous version, the main changes are: 1. Remove !single_set at checking likely_profitable_p. Add is_debug_insn here, so that debug insn still need to be profitable. 2. Remove single_set check for cost comparison. Add !is_debug_insn here, so that debug insn doesn't need to check the insn cost. Previous version https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654964.html The patch causes a regression case on i386 as the pattern cost regulation has a bug. Please refer the patch and discussion here. https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651363.html Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is it OK for the trunk? Thanks Gui Haochen ChangeLog fwprop: invoke change_is_worthwhile to judge if a replacement is worthwhile gcc/ * fwprop.cc (try_fwprop_subst_pattern): Invoke change_is_worthwhile to judge if a replacement is worthwhile. Remove single_set check and add is_debug_insn check. * recog.cc (swap_change): Invalidate recog_data when the cached INSN is swapped out. * rtl-ssa/changes.cc (rtl_ssa::changes_are_worthwhile): Check if the insn cost of new rtl is unknown and fail the replacement. patch.diff diff --git a/gcc/fwprop.cc b/gcc/fwprop.cc index de543923b92..60e3ea31edc 100644 --- a/gcc/fwprop.cc +++ b/gcc/fwprop.cc @@ -453,7 +453,7 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change, && (prop.changed_mem_p () || contains_mem_rtx_p (src) || use_insn->is_asm () - || !single_set (use_rtl))) + || use_insn->is_debug_insn ())) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "cannot propagate from insn %d into" @@ -471,29 +471,18 @@ try_fwprop_subst_pattern (obstack_watermark &attempt, insn_change &use_change, redo_changes (0); } - /* ??? In theory, it should be better to use insn costs rather than - set_src_costs here. That would involve replacing this code with - change_is_worthwhile. */ bool ok = recog (attempt, use_change); - if (ok && !prop.changed_mem_p () && !use_insn->is_asm ()) - if (rtx use_set = single_set (use_rtl)) - { - bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (use_rtl)); - temporarily_undo_changes (0); - auto old_cost = set_src_cost (SET_SRC (use_set), - GET_MODE (SET_DEST (use_set)), speed); - redo_changes (0); - auto new_cost = set_src_cost (SET_SRC (use_set), - GET_MODE (SET_DEST (use_set)), speed); - if (new_cost > old_cost - || (new_cost == old_cost && !prop.likely_profitable_p ())) - { - if (dump_file) - fprintf (dump_file, "change not profitable" - " (cost %d -> cost %d)\n", old_cost, new_cost); - ok = false; - } - } + if (ok && !prop.changed_mem_p () && !use_insn->is_asm () + && !use_insn->is_debug_insn ()) + { + bool strict_p = !prop.likely_profitable_p (); + if (!change_is_worthwhile (use_change, strict_p)) + { + if (dump_file) + fprintf (dump_file, "change not profitable"); + ok = false; + } + } if (!ok) { diff --git a/gcc/recog.cc b/gcc/recog.cc index a6799e3f5e6..56370e40e01 100644 --- a/gcc/recog.cc +++ b/gcc/recog.cc @@ -614,7 +614,11 @@ swap_change (int num) else std::swap (*changes[num].loc, changes[num].old); if (changes[num].object && !MEM_P (changes[num].object)) - std::swap (INSN_CODE (changes[num].object), changes[num].old_code); + { + std::swap (INSN_CODE (changes[num].object), changes[num].old_code); + if (recog_data.insn == changes[num].object) + recog_data.insn = nullptr; + } } /* Temporarily undo all the changes numbered NUM and up, with a view diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc index 11639e81bb7..c5ac4956a19 100644 --- a/gcc/rtl-ssa/changes.cc +++ b/gcc/rtl-ssa/changes.cc @@ -186,6 +186,14 @@ rtl_ssa::changes_are_worthwhile (array_slice changes, if (!change->is_deletion ()) { change->new_cost = insn_cost (change->rtl (), for_speed); + /* If the cost is unknown, replacement is not worthwhile. */ + if (!change->new_cost) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, + "Reject replacement due to unknown insn cost.\n"); + return false; + } new_cost += change->new_cost; if (for_speed) weighted_new_cost += (cfg_bb->count.to_sreal_scale (entry_count)