From patchwork Tue Feb 1 14:45:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robin Dapp X-Patchwork-Id: 50622 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 354B1386486C for ; Tue, 1 Feb 2022 14:45:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 354B1386486C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1643726745; bh=kcP9ye3Jivci0nzyc+xXyvxZ56Dv/oTsGwasVSn3zFs=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=HLNXfXsN1R+T/4/un3RM6LIOQPQZa/vqBkR6JD7uLf29Keba6fORhYe8ncKzRyQhY Sg+6MNv9HIa/CFD4qF7NHF6YxA4E6VxnwiVVoJzTuYz59b2B4VI81fKgawewN8N1w2 t057m0Xb1XZEIJXsok1ZmmnpDvS0HSQtkiiY7oYU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 41C033858033 for ; Tue, 1 Feb 2022 14:45:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 41C033858033 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 211DIlat015165; Tue, 1 Feb 2022 14:45:11 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 3dxkthpyka-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Feb 2022 14:45:11 +0000 Received: from m0098416.ppops.net (m0098416.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 211E6sDa024871; Tue, 1 Feb 2022 14:45:10 GMT Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0b-001b2d01.pphosted.com with ESMTP id 3dxkthpyjh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Feb 2022 14:45:10 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 211EghNI031140; Tue, 1 Feb 2022 14:45:09 GMT Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by ppma03ams.nl.ibm.com with ESMTP id 3dvw79ney9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Feb 2022 14:45:08 +0000 Received: from d06av23.portsmouth.uk.ibm.com (d06av23.portsmouth.uk.ibm.com [9.149.105.59]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 211Ej5hj36503956 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 1 Feb 2022 14:45:06 GMT Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D9DE8A4055; Tue, 1 Feb 2022 14:45:05 +0000 (GMT) Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9E8EBA4040; Tue, 1 Feb 2022 14:45:05 +0000 (GMT) Received: from [9.171.40.238] (unknown [9.171.40.238]) by d06av23.portsmouth.uk.ibm.com (Postfix) with ESMTPS; Tue, 1 Feb 2022 14:45:05 +0000 (GMT) Message-ID: <35755ec2-0612-0d94-c2d5-d2527c384c3b@linux.ibm.com> Date: Tue, 1 Feb 2022 15:45:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Content-Language: en-US Subject: ifcvt: Fix PR104153 and PR104198 To: GCC Patches , Jeff Law X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: 1t_8pTrzKHQBkIBB73_Uy633Wi3MwIjI X-Proofpoint-GUID: GK2blqlhWraKh6jAIRIfSmR7P3b3uM-r X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-02-01_07,2022-02-01_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 suspectscore=0 mlxlogscore=999 clxscore=1011 phishscore=0 lowpriorityscore=0 spamscore=0 adultscore=0 bulkscore=0 mlxscore=0 impostorscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000 definitions=main-2202010078 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Robin Dapp via Gcc-patches From: Robin Dapp Reply-To: Robin Dapp Cc: Richard Sandiford Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi, this is a bugfix for aa8cfe785953a0e87d2472311e1260cd98c605c0 which broke an or1k test case (PR104153) as well as SPARC bootstrap (PR104198). cond_exec_get_condition () returns the jump condition directly and we now it to the backend. The or1k backend modified the condition in-place but this modification is not reverted when the sequence in question is discarded. Therefore this patch copies the RTX instead of using it directly. The SPARC problem is due to the backend recreating the initial condition when being passed a CC comparison. This causes the sequence to read from an already overwritten condition operand. Generally, this could also happen on other targets. The workaround is to always first emit to a temporary. In a second run of noce_convert_multiple_1 we know which sequences actually require the comparison and use no temporaries if all sequences after the current one do not require it. Before, I used reg_overlap_mentioned_p () to check the generated instructions against the condition. The problem with this is that reg_overlap... only handles a set of rtx_codes while a backend can theoretically emit everything in an expander. Is reg_mentioned_p () the "right thing" to do? Maybe it is overly conservative but as soon as we have more than let's say three insns, we are unlikely to succeed anyway. Bootstrapped and reg-tested on s390x, Power 9, x86 and SPARC. Regards Robin --- PR 104198 PR 104153 gcc/ChangeLog: * ifcvt.cc (noce_convert_multiple_sets_1): Copy rtx instead of using it directly. Rework comparison handling and always perform a second pass. gcc/testsuite/ChangeLog: * gcc.dg/pr104198.c: New test. commit 68489d5729b4879bf2df540753fc7ea8ba1565a5 Author: Robin Dapp Date: Mon Jan 24 10:28:05 2022 +0100 ifcvt: Fix PR104153 and PR104198. This is a bugfix for aa8cfe785953a0e87d2472311e1260cd98c605c0 which broke an or1k test case (PR104153) as well as SPARC bootstrap (PR104198). cond_exec_get_condition () returns the jump condition directly and we now pass it to the backend. The or1k backend modified the condition in-place (other backends do that as well) but this modification is not reverted when the sequence in question is discarded. Therefore we copy the RTX instead of using it directly. The SPARC problem is due to the SPARC backend recreating the initial condition when being passed a CC comparison. This causes the sequence to read from an already overwritten condition operand. Generally, this could also happen on other targets. The workaround is to always first emit to a temporary. In a second run of noce_convert_multiple_1 we know which sequences actually require the comparison and will use no temporaries if all sequences after the current one do not require it. diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index fe250d508e1..92c2b40a45a 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -3391,7 +3391,11 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, rtx cond = noce_get_condition (jump, &cond_earliest, false); rtx cc_cmp = cond_exec_get_condition (jump); + if (cc_cmp) + cc_cmp = copy_rtx (cc_cmp); rtx rev_cc_cmp = cond_exec_get_condition (jump, /* get_reversed */ true); + if (rev_cc_cmp) + rev_cc_cmp = copy_rtx (rev_cc_cmp); rtx_insn *insn; int count = 0; @@ -3515,6 +3519,7 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, unsigned cost1 = 0, cost2 = 0; rtx_insn *seq, *seq1, *seq2; rtx temp_dest = NULL_RTX, temp_dest1 = NULL_RTX, temp_dest2 = NULL_RTX; + bool read_comparison = false; seq1 = try_emit_cmove_seq (if_info, temp, cond, new_val, old_val, need_cmov, @@ -3524,10 +3529,38 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, as well. This allows the backend to emit a cmov directly without creating an additional compare for each. If successful, costing is easier and this sequence is usually preferred. */ - seq2 = try_emit_cmove_seq (if_info, target, cond, + seq2 = try_emit_cmove_seq (if_info, temp, cond, new_val, old_val, need_cmov, &cost2, &temp_dest2, cc_cmp, rev_cc_cmp); + /* The backend might have created a sequence that uses the + condition. Check this. */ + rtx_insn *walk = seq2; + while (walk) + { + rtx set = single_set (walk); + + if (!set || !SET_SRC (set)) { + walk = NEXT_INSN (walk); + continue; + } + + rtx src = SET_SRC (set); + + if (XEXP (set, 1) && GET_CODE (XEXP (set, 1)) == IF_THEN_ELSE) + ; + else + { + if (reg_mentioned_p (XEXP (cond, 0), src) + || reg_mentioned_p (XEXP (cond, 1), src)) + { + read_comparison = true; + break; + } + } + walk = NEXT_INSN (walk); + } + /* Check which version is less expensive. */ if (seq1 != NULL_RTX && (cost1 <= cost2 || seq2 == NULL_RTX)) { @@ -3540,6 +3573,8 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, { seq = seq2; temp_dest = temp_dest2; + if (!second_try && read_comparison) + *last_needs_comparison = count; } else { @@ -3558,6 +3593,12 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, unmodified_insns->safe_push (insn); } + /* Even if we did not actually need the comparison, we want to make sure + to try a second time in order to get rid of the temporaries. */ + if (*last_needs_comparison == -1) + *last_needs_comparison = 0; + + return true; } diff --git a/gcc/testsuite/gcc.dg/pr104198.c b/gcc/testsuite/gcc.dg/pr104198.c new file mode 100644 index 00000000000..bfc7a777184 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr104198.c @@ -0,0 +1,36 @@ +/* Make sure if conversion for two instructions does not break + anything (if it runs). */ + +/* { dg-do run } */ +/* { dg-options "-O2 -std=c99" } */ + +#include +#include + +__attribute__ ((noinline)) +int foo (int *a, int n) +{ + int min = 999999; + int bla = 0; + for (int i = 0; i < n; i++) + { + if (a[i] < min) + { + min = a[i]; + bla = 1; + } + } + + if (bla) + min += 1; + return min; +} + +int main() +{ + int a[] = {2, 1, -13, INT_MAX, INT_MIN, 0}; + + int res = foo (a, sizeof (a) / sizeof (a[0])); + + assert (res == (INT_MIN + 1)); +}